-
Notifications
You must be signed in to change notification settings - Fork 333
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
feat: write manifests in background tasks #3709
Conversation
d34f9bd
to
8120097
Compare
188168c
to
a821f7e
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3709 +/- ##
==========================================
- Coverage 85.43% 85.23% -0.20%
==========================================
Files 966 947 -19
Lines 162088 160217 -1871
==========================================
- Hits 138482 136567 -1915
- Misses 23606 23650 +44 |
Haven't gone through the code. It sounds like the write will be rejected if the region is flushing, right? We may need to distinguish different non-writable statuses. Read-only can reject write requests, but holding and retrying later for flush/compact looks more reasonable. |
Co-authored-by: Lei, HUANG <[email protected]>
No, flush/compaction won't change the region state. The region is still in the writable state. |
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
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
What's changed and what's your intention?
This PR spawns a background task to write the manifest file.
It replaces the
writable
flag withRegionState
to represent the state of the region.The region rejects write requests while it is not in the writable state. This avoids the region handling other requests while updating the manifest in the background.
It wraps the manifest manager and the region state into a new struct
ManifestContext
. Before updating the manifest, the context will also check the region state.We need to ensure the region version is updated while holding the write lock of the manifest manager. So the context provides a method
update_manifest()
to update the manifest and apply the update to the region version.Flush and Compaction
Since the region flushes and compacts files in the background job, it can update the manifest in the background directly.
Truncate
It rewrites the implementation of truncate to spawn a background job to write the manifest (See
handle_manifest_truncate_action()
). After the manifest is updated, the background job sends theTruncateResult
back to the worker loop.As the manifest action created by the flush/compaction job before truncation should be ignored, the method
ManifestContext::update_manifest()
needs to handle it correctly.Alter
It sets the state and then spawns a background job to write the manifest.
Regoin Edit
Similar to alteration, the worker spawns a background job to do this.
Drop
Drop doesn't update the manifest so we still write the marker file in the worker loop to avoid this PR getting to large. But it still sets the region state.
Checklist