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

KAFKA-14491: [22/N] Add test for manual upgrade to versioned store #13449

Merged
merged 3 commits into from Apr 7, 2023

Conversation

vcrfxia
Copy link
Collaborator

@vcrfxia vcrfxia commented Mar 24, 2023

This PR adds an integration test for the manual upgrade scenario to upgrade a non-versioned store to a versioned store. The procedure is outlined in KIP-889 and also in the docs added in #13444:

  • stop all app instances
  • delete local state store
  • update changelog topic configs (done beforehand in the test, to ensure no premature compaction of data)
  • restart app with new code for versioned stores.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)


streamsBuilder
.addStateStore(
Stores.timestampedKeyValueStoreBuilder(
Copy link
Member

Choose a reason for hiding this comment

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

The docs (pre PR) might also be affected, but we do support upgrade even from plain KV-stores? If yes, we should test it, too, if no, we should be explicit in the docs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, as long as the changelog topic format is the same it'll work, which means non-timestamped stores are eligible for upgrade too. Added an additional test.

The docs PR just says "non-versioned" right now (no mention of timestamped vs non-timestamped). I can add a line to make it clear that upgrading from both timestamped and non-timestamped stores are supported.

Copy link
Member

Choose a reason for hiding this comment

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

I can add a line to make it clear that upgrading from both timestamped and non-timestamped stores are supported.

Sounds like a good idea. Thx.

@mjsax
Copy link
Member

mjsax commented Mar 30, 2023

FAILURE: Build failed with an exception.
[2023-03-28T21:07:13.365Z] 
[2023-03-28T21:07:13.365Z] * What went wrong:
[2023-03-28T21:07:13.365Z] A problem was found with the configuration of task ':rat' (type 'RatTask').
[2023-03-28T21:07:13.365Z]   - Gradle detected a problem with the following location: '/home/jenkins/workspace/Kafka_kafka-pr_PR-13449'.
[2023-03-28T21:07:13.365Z]     
[2023-03-28T21:07:13.365Z]     Reason: Task ':rat' uses this output of task ':clients:processTestMessages' without declaring an explicit or implicit dependency. This can lead to incorrect results being produced, depending on what order the tasks are executed.
[2023-03-28T21:07:13.365Z]     
[2023-03-28T21:07:13.365Z]     Possible solutions:
[2023-03-28T21:07:13.365Z]       1. Declare task ':clients:processTestMessages' as an input of ':rat'.
[2023-03-28T21:07:13.365Z]       2. Declare an explicit dependency on ':clients:processTestMessages' from ':rat' using Task#dependsOn.
[2023-03-28T21:07:13.365Z]       3. Declare an explicit dependency on ':clients:processTestMessages' from ':rat' using Task#mustRunAfter.

Not 100% sure what this means. Will retrigger Jenkins to see if it goes away.

@mjsax mjsax merged commit 1743548 into apache:trunk Apr 7, 2023
@vcrfxia vcrfxia deleted the kip-889-upgrade-test branch April 10, 2023 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants