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): post event when rpc is ready #14034

Merged
merged 17 commits into from
Dec 20, 2024

Conversation

chronolaw
Copy link
Contributor

@chronolaw chronolaw commented Dec 18, 2024

Summary

KAG-5895

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/clustering cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee labels Dec 18, 2024
@chronolaw chronolaw force-pushed the refactor/events_for_metacall branch from 33bae64 to d28754b Compare December 18, 2024 11:23
@chronolaw chronolaw marked this pull request as ready for review December 18, 2024 13:14
kong/clustering/rpc/manager.lua Outdated Show resolved Hide resolved
@pull-request-size pull-request-size bot added size/L and removed size/M labels Dec 19, 2024
Copy link
Contributor

@StarlightIbuki StarlightIbuki left a comment

Choose a reason for hiding this comment

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

Should we add a disconnection notification? And what about CP's side notification for new DP connection?

@chronolaw
Copy link
Contributor Author

Should we add a disconnection notification? And what about CP's side notification for new DP connection?

I think that CP side does not need notification now.

Copy link
Member

@dndx dndx left a comment

Choose a reason for hiding this comment

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

This is a good start! However, we need more work to be done for [KAG-5891] to be considered done because we also need to receive notifications when connections are broken and stop syncing. But that can be done in another PR.

@dndx dndx merged commit ab888cc into master Dec 20, 2024
28 checks passed
@dndx dndx deleted the refactor/events_for_metacall branch December 20, 2024 22:31
@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-14034-to-master-to-upstream upstream/master
cd .worktree/cherry-pick-14034-to-master-to-upstream
git checkout -b cherry-pick-14034-to-master-to-upstream
ancref=$(git merge-base 33a43c702c81c0ff0e6090f6f95faec30a65a5b7 186e40827fa5b9f0902e6b03738c7a581b661799)
git cherry-pick -x $ancref..186e40827fa5b9f0902e6b03738c7a581b661799

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

chronolaw commented Dec 21, 2024

This is a good start! However, we need more work to be done for [KAG-5891] to be considered done because we also need to receive notifications when connections are broken and stop syncing. But that can be done in another PR.

@StarlightIbuki could you open some new PR for KAG-5891, based on this PR?

-- if rpc is ready we will check then decide how to sync
worker_events.register(function(capabilities_list)
-- we only check once
if self.inited then
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose it's fine to make another sync call when reconnecting?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of cause.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee core/clustering incomplete-cherry-pick A cherry-pick was incomplete and needs manual intervention size/L skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants