-
Notifications
You must be signed in to change notification settings - Fork 1k
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: During ksql startup, avoid recreating deleted command topic when a valid backup exists #7753 #8257
fix: During ksql startup, avoid recreating deleted command topic when a valid backup exists #7753 #8257
Conversation
…anges into its own test file: QuickDegradeAndRestoreCommandTopicIntegrationTest
KsqlRestoreCommandTopic.main( | ||
new String[]{ | ||
"--yes", | ||
"-s", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This flag isn't necessary, this flag is for skipping incompatible commands when restoring the command topic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah ok, will remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
…port in RestoreCommandTopicIntegrationTest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, left one comment
|
||
// Then | ||
assertThat(TEST_HARNESS.topicExists(commandTopic), is(false)); | ||
assertThatEventually("Degraded State", this::isDegradedState, is(true)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we switch the order of these two statements? I'm not sure the check for the command topic will happen before the server has fully started up and skipped over any parts that may create a command topic. (I don't think .start()
is blocking)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Swapped the order of two statements.
…e and topic exists in integration test.
Description
Fixes #7753
During ksql startup, avoid recreating deleted command topic when a valid backup exists.
Earlier, regardless of backup file we used to recreate deleted command topic during ksql startup(which will be out of sync with existing backup). This delays detection of the problem until a new query is issued to ksql server which subsequently will enter DEGRADED status.
With this change, deleted command topics won't be recreated once a valid backup is found and the ksql server will be put on DEGRADED status immediately during the startup process.
Testing done
Unit, integration test and manual verification
Reviewer checklist