Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(dbless): construct right pk string for entity with multiple primary keys #13826

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

chobits
Copy link
Contributor

@chobits chobits commented Nov 4, 2024

Summary

For the entity with multiple primary keys, like consumer_group_consumers, the pk_string() function construct incorrect pk string like table: 0x028a49eda0:table: 0x0289c257b8. Because it does not extract the internal id value. Here, we fix it with original lmdb logic get_cache_key_value(). Note this method directly gets id field name.

Checklist

  • The Pull Request has tests
  • A changelog file has been created under changelog/unreleased/kong or skip-changelog label added on PR if changelog is unnecessary. README.md
  • There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Issue reference

Fix KAG-5732

@github-actions github-actions bot added core/db schema-change-noteworthy cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee labels Nov 4, 2024
@team-gateway-bot team-gateway-bot added the author/community PRs from the open-source community (not Kong Inc) label Nov 4, 2024
@chronolaw chronolaw changed the title fix(sync/lmdb): construct right pk string for entity with multiple primary keys fix(clustering/sync): construct right pk string for entity with multiple primary keys Nov 4, 2024
@chronolaw chronolaw changed the title fix(clustering/sync): construct right pk string for entity with multiple primary keys fix(dbless): construct right pk string for entity with multiple primary keys Nov 4, 2024
@team-eng-enablement team-eng-enablement removed the author/community PRs from the open-source community (not Kong Inc) label Nov 4, 2024
@dndx dndx marked this pull request as draft November 5, 2024 02:31
@dndx
Copy link
Member

dndx commented Nov 5, 2024

@chobits is working on fixing this PR, do not merge yet.

@chobits chobits marked this pull request as ready for review November 5, 2024 06:04
…imary keys

For the entity with multiple primary keys, like consumer_group_consumers, the
pk_string() function construct incorrect pk string like
`table: 0x028a49eda0:table: 0x0289c257b8`. Because it does not extract the
internal id value. Here, we fix it with original lmdb logic
get_cache_key_value(). Note this method directly gets id field name.
…tring (#10623)

* fix(sync): don't use an upvalue request_aware_table to construct pk_string

* fix: use local upvalue {}
@chobits
Copy link
Contributor Author

chobits commented Nov 5, 2024

ready for reviewing,

Still need another fix to iterate the foreign pk strings. TODO kag: https://konghq.atlassian.net/browse/KAG-5750

@chobits chobits marked this pull request as draft November 5, 2024 06:30
@chobits chobits marked this pull request as ready for review November 5, 2024 07:33
@chobits chobits marked this pull request as draft November 5, 2024 07:39
@chobits chobits marked this pull request as ready for review November 5, 2024 07:58
@chobits chobits requested a review from bungle November 5, 2024 08:53
@pull-request-size pull-request-size bot removed the size/S label Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants