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

refactor(clustering/rpc): tunings and fixes for rubustness #13771

Merged
merged 26 commits into from
Nov 1, 2024

Conversation

chronolaw
Copy link
Contributor

@chronolaw chronolaw commented Oct 17, 2024

Summary

Please squash and merge.

KAG-5617

Related tickets:
KAG-5602

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 #[issue number]

@github-actions github-actions bot added core/db cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee labels Oct 17, 2024
@pull-request-size pull-request-size bot added size/M and removed size/S labels Oct 22, 2024
@chronolaw chronolaw changed the title refactor(clustering/rpc): tunings for rubustness refactor(clustering/rpc): tunings and fixes for rubustness Oct 22, 2024
@pull-request-size pull-request-size bot added size/L and removed size/M labels Oct 24, 2024
@chronolaw chronolaw marked this pull request as ready for review October 29, 2024 11:57
@team-gateway-bot team-gateway-bot added the author/community PRs from the open-source community (not Kong Inc) label Oct 29, 2024

-- If the item belongs to a specific workspace,
-- use it directly without using the default one.
local ws_id = item.ws_id or workspace_id(schema, options)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: need to add a test to check the ws_id value if the schema is non-workspaceable. https://konghq.atlassian.net/browse/KAG-5706

This comment is not a block.

Comment on lines 318 to 322
for _, wid in ipairs {ws_id, GLOBAL_WORKSPACE_TAG} do
local key = unique_field_key(entity_name, wid, "cache_key", cache_key)

-- store item_key or nil into lmdb
t:set(key, idx_value)
Copy link
Contributor

@chobits chobits Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a blocker, just a comment for other reviewers.

TODO: we only want to save the entity with key containing specific ws_id, not a GLOBAL workspace id like*, which does not consume too much memory space for lmdb. https://konghq.atlassian.net/browse/KAG-5704

Comment on lines 390 to 395
-- * <entity_name>|*|*|<pk_string> => serialized item
-- * <entity_name>|<ws_id>|*|<pk_string> => actual item key
--
-- * <entity_name>|*|<unique_field_name>|sha256(field_value) => actual item key
-- * <entity_name>|<ws_id>|<unique_field_name>|sha256(field_value) => actual item key
--
-- * <entity_name>|*|<foreign_field_name>|<foreign_key>|<pk_string> => actual item key
-- * <entity_name>|<ws_id>|<foreign_field_name>|<foreign_key>|<pk_string> => actual item key
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chronolaw chronolaw removed the author/community PRs from the open-source community (not Kong Inc) label Oct 30, 2024
@team-gateway-bot team-gateway-bot added the author/community PRs from the open-source community (not Kong Inc) label Oct 30, 2024
@@ -359,7 +359,8 @@ local sync_emitter = {

emit_entity = function(self, entity_name, entity_data)
self.out_n = self.out_n + 1
self.out[self.out_n] = { type = entity_name , row = entity_data, version = self.sync_version, }
self.out[self.out_n] = { type = entity_name , row = entity_data, version = self.sync_version,
ws_id = kong.default_workspace, }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check it later

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here ws_id is a fallback, if row(entity) has the field ws_id, do_sync() will use it but not delta.ws_id.

if not res then
return nil, err
end
end

local res, err = insert_entity_for_txn(t, delta_type, delta_row, nil)
local res, err = insert_entity_for_txn(t, delta_type, delta_row, opts)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

row.ws_id ? delta.ws_id?

Copy link
Contributor Author

@chronolaw chronolaw Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In hooks.lua, delta item will always has the field ws_id, so opts.workspace is a fallback.

  local deltas = {
    {
      type = name,
      pk = pk,
      ws_id = ws_id,
      entity = is_delete and ngx_null or entity,
    },
  }

chronolaw and others added 19 commits October 31, 2024 17:48
* fix(clustering/sync): set ws_id in do_sync()

* comments
* fix(clustering/sync): delta.version may be null

* isempty

* default_ws_changed
* rename db id to pk

* change id to pk in Lua

* change pk to json

* schema:extract_pk_values
Co-authored-by: Datong Sun <datong.sun@konghq.com>
* change db filed name

* rename in rpc.sync

* rename in export
…#13815)

* fix(incremental sync): fix cache_key handling for select_by_cache_key

* fix some comments

* fix: remove unnecessary comment
* refactor(dbless): clean logic of selec_by_field

* check schema_field
@bungle bungle merged commit 0072d7b into master Nov 1, 2024
32 checks passed
@bungle bungle deleted the refactor/tuning_inc_sync branch November 1, 2024 09:12
@team-gateway-bot
Copy link
Collaborator

Cherry-pick failed for master, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git remote add upstream https://github.com/kong/kong-ee
git fetch upstream master
git worktree add -d .worktree/cherry-pick-13771-to-master-to-upstream upstream/master
cd .worktree/cherry-pick-13771-to-master-to-upstream
git checkout -b cherry-pick-13771-to-master-to-upstream
ancref=$(git merge-base 5cab556adc6b0c74c11b83baeb2b81010881e8dc b37bdd165487396379420cef355d16f3fb8a8583)
git cherry-pick -x $ancref..b37bdd165487396379420cef355d16f3fb8a8583

@github-actions github-actions bot added the incomplete-cherry-pick A cherry-pick was incomplete and needs manual intervention label Nov 1, 2024
@chronolaw
Copy link
Contributor Author

Cherry-picked in https://github.com/Kong/kong-ee/pull/10460

@AndyZhang0707 AndyZhang0707 removed the incomplete-cherry-pick A cherry-pick was incomplete and needs manual intervention label Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author/community PRs from the open-source community (not Kong Inc) cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee core/clustering core/db/migrations core/db schema-change-noteworthy size/L skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants