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

DGS-10707 Cache checkpoint for cacheInitialized before reader thread starts #3161

Closed
wants to merge 2 commits into from

Conversation

karthikeyanas
Copy link
Member

What

Cache checkpoint for cacheInitialized before reader thread starts. Checkpoint could be updated once the reader thread starts. This could result in a race condition where schemas after the checkpoint during startup would be double counted.

References

https://confluentinc.atlassian.net/browse/DGS-10707

akhileshm1
akhileshm1 previously approved these changes Jun 26, 2024
Copy link
Member

@rayokota rayokota left a comment

Choose a reason for hiding this comment

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

Thanks @karthikeyanas , left some comments

@@ -153,7 +157,7 @@ public void init() throws StoreInitializationException {
throw new StoreInitializationException("Illegal state while initializing store. Store "
+ "was already initialized");
}
this.storeUpdateHandler.cacheInitialized(new HashMap<>(kafkaTopicReader.checkpoints()));
this.storeUpdateHandler.cacheInitialized(new HashMap<>(checkpoints));
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
this.storeUpdateHandler.cacheInitialized(new HashMap<>(checkpoints));
this.storeUpdateHandler.cacheInitialized(checkpoints);

Copy link
Member

@rayokota rayokota left a comment

Choose a reason for hiding this comment

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

The current change doesn't do anything, we need to copy the map earlier

@rayokota
Copy link
Member

Also we should target 7.0.x with this PR

@karthikeyanas karthikeyanas changed the base branch from 7.7.x to 7.0.x June 27, 2024 04:55
@karthikeyanas karthikeyanas dismissed akhileshm1’s stale review June 27, 2024 04:55

The base branch was changed.

@karthikeyanas karthikeyanas changed the base branch from 7.0.x to 7.7.x June 27, 2024 04:55
@karthikeyanas karthikeyanas deleted the schema-count-checkpoint branch June 27, 2024 05: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
Development

Successfully merging this pull request may close these issues.

None yet

3 participants