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

[Server][FC] throws exception during store migration to enforce d2 refresh #1132

Merged
merged 9 commits into from
Sep 7, 2024

Conversation

adamxchen
Copy link
Collaborator

@adamxchen adamxchen commented Aug 21, 2024

Summary

As titled, we need to throw exceptions explicitly so FC can detect the store migration and refresh d2 service as needed. How it works is basically we compare the server cluster with the store cluster set on ZK. If they are different, it means the store migration has completed in the given region, so we can throw exception to refresh the service discovery.

How was this PR tested?

new unit tests and the integ test.

Does this PR introduce any user-facing changes?

  • No. You can skip the rest of this section.
  • Yes. Make sure to explain your proposed changes and call out the behavior change.

@adamxchen adamxchen marked this pull request as ready for review August 22, 2024 17:47
@FelixGV
Copy link
Contributor

FelixGV commented Aug 23, 2024

For a commit which changes the server, please include [server] in the commit message prefix.

@adamxchen adamxchen changed the title [FC] throws exception during store migration to enforce d2 refresh [Server][FC] throws exception during store migration to enforce d2 refresh Aug 23, 2024
# Conflicts:
#	internal/venice-test-common/src/integrationTest/java/com/linkedin/venice/endToEnd/TestStoreMigration.java
Copy link
Contributor

@xunyin8 xunyin8 left a comment

Choose a reason for hiding this comment

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

LGTM just some minor comments

@adamxchen adamxchen enabled auto-merge (squash) September 7, 2024 01:19
@adamxchen adamxchen merged commit 9494b43 into linkedin:main Sep 7, 2024
32 checks passed
@adamxchen
Copy link
Collaborator Author

Enabled the automerge but forgot to squash my commit msgs 😢

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants