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

[SPARK-47568][SS][3.5] Fix race condition between maintenance thread and load/commit for snapshot files #45881

Closed
wants to merge 1 commit into from

Conversation

sahnib
Copy link
Contributor

@sahnib sahnib commented Apr 4, 2024

Backports #45724 to 3.5

What changes were proposed in this pull request?

This PR fixes a race condition between the maintenance thread and task thread when change-log checkpointing is enabled, and ensure all snapshots are valid.

  1. The maintenance thread currently relies on class variable lastSnapshot to find the latest checkpoint and uploads it to DFS. This checkpoint can be modified at commit time by Task thread if a new snapshot is created.
  2. The task thread was not resetting the lastSnapshot at load time, which can result in newer snapshots (if a old version is loaded) being considered valid and uploaded to DFS. This results in VersionIdMismatch errors.

Why are the changes needed?

These are logical bugs which can cause VersionIdMismatch errors causing user to discard the snapshot and restart the query.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Added unit test cases.

Was this patch authored or co-authored using generative AI tooling?

No

@sahnib sahnib marked this pull request as ready for review April 4, 2024 16:53
@sahnib
Copy link
Contributor Author

sahnib commented Apr 4, 2024

@HeartSaVioR PTAL, thanks.

@HyukjinKwon HyukjinKwon changed the title [SPARK-47568][SS] Fix race condition between maintenance thread and load/commit for snapshot files [SPARK-47568][SS][3.5] Fix race condition between maintenance thread and load/commit for snapshot files Apr 5, 2024
@HyukjinKwon
Copy link
Member

Merged to branch-3.5.

HyukjinKwon pushed a commit that referenced this pull request Apr 5, 2024
…and load/commit for snapshot files

Backports #45724 to 3.5

### What changes were proposed in this pull request?

This PR fixes a race condition between the maintenance thread and task thread when change-log checkpointing is enabled, and ensure all snapshots are valid.

1. The maintenance thread currently relies on class variable lastSnapshot to find the latest checkpoint and uploads it to DFS. This checkpoint can be modified at commit time by Task thread if a new snapshot is created.
2. The task thread was not resetting the lastSnapshot at load time, which can result in newer snapshots (if a old version is loaded) being considered valid and uploaded to DFS. This results in VersionIdMismatch errors.

### Why are the changes needed?

These are logical bugs which can cause `VersionIdMismatch` errors causing user to discard the snapshot and restart the query.

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

Added unit test cases.

### Was this patch authored or co-authored using generative AI tooling?

No

Closes #45881 from sahnib/rocks-db-fix-3.5.

Authored-by: Bhuwan Sahni <bhuwan.sahni@databricks.com>
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
@HyukjinKwon HyukjinKwon closed this Apr 5, 2024
@HeartSaVioR
Copy link
Contributor

Late +1. Thanks everyone!

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