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(cluster): fix unknown migration error #3899

Conversation

andydunstall
Copy link
Contributor

@andydunstall andydunstall commented Oct 9, 2024

We're getting redundant errors logs of outgoing_slot_migration.cc:184] ERR UNKNOWN_MIGRATION (causing alerts)

Looks like this is expected to be ignored. ClusterFamily::InitMigration returns SendError(OutgoingMigration::kUnknownMigration), and OutgoingMigration::SyncFb ignores CheckRespIsSimpleReply(kUnknownMigration), which doesn't match

Therefore updating ClusterFamily::InitMigration to respond with a simple string SendSimpleString(OutgoingMigration::kUnknownMigration) instead so they match. Alternatively ClusterFamily::InitMigration could check for an error response (I'm not sure which side is right)?

@andydunstall andydunstall requested a review from chakaz October 9, 2024 11:56
Copy link
Collaborator

@chakaz chakaz left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the fix!

@andydunstall
Copy link
Contributor Author

andydunstall commented Oct 10, 2024

test_noreply_pipeline has failed 3 times in a row - doesn't look like thats due to my change (can't reproduce locally)

@kostasrim
Copy link
Contributor

test_noreply_pipeline has failed 3 times in a row - doesn't look like thats due to my change (can't reproduce locally)

no it fails on the CI as well, plz ignore

Copy link
Contributor

@kostasrim kostasrim left a comment

Choose a reason for hiding this comment

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

plz rerun the CI and ignore the failure

@kostasrim
Copy link
Contributor

@andydunstall plz rebase you branch once #3903 is merged

@andydunstall andydunstall force-pushed the fix-redundant-unknown-migration-error branch from d666e40 to 7cc5b06 Compare October 10, 2024 08:41
@andydunstall andydunstall merged commit 1e429c8 into dragonflydb:main Oct 10, 2024
9 checks passed
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.

3 participants