-
Notifications
You must be signed in to change notification settings - Fork 490
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
Evict WAL files from disk #8022
Conversation
2934 tests run: 2817 passed, 0 failed, 117 skipped (full report)Code coverage* (full report)
* collected from Rust tests only The comment gets automatically updated with the latest test results
9767942 at 2024-06-26T18:10:07.159Z :recycle: |
4e6173c
to
c828cde
Compare
Thinking about safekeepers that might re-download while some other safekeeper is uploading partial segments -- after this change, it's important that anything that is in a partial upload is committed data, since some other safekeeper might use it to re-download after eviction -- i.e. they must never upload partial segments that contain data that might not have achieved quorum write. Is that already the case? |
Not really, each safekeeper uploads its own copy of partial segment. Safekeepers don't download partial segments from other safekeeper.
No, currently safekeepers upload all bytes that are present on local disk, including the cases when |
Control file upgrade is splitted to another PR: #8125 |
This is a preparation for #8022, to make the PR both backwards and foward compatible. This commit adds `eviction_state` field to control file. Adds support for reading it, but writes control file in old format where possible, to keep the disk format forward compatible. Note: in `patch_control_file`, new field gets serialized to json like this: - `"eviction_state": "Present"` - `"eviction_state": {"Offloaded": "0/8F"}`
52646b0
to
58f4fb8
Compare
0950055
to
026868e
Compare
992b565
to
6bef78a
Compare
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.
Looked through more than a half, LGTM, continuing, posting some comments accumulated so far.
BTW, TimelinePersistentState.partial_backup offloaded segment LSN always equals EvictionState::Offloaded LSN on evicted timelines, right?
Yes. |
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 except
#8022 (comment)
though this PR shouldn't make the situation worse.
8d9e72d
to
9767942
Compare
This is a preparation for #8022, to make the PR both backwards and foward compatible. This commit adds `eviction_state` field to control file. Adds support for reading it, but writes control file in old format where possible, to keep the disk format forward compatible. Note: in `patch_control_file`, new field gets serialized to json like this: - `"eviction_state": "Present"` - `"eviction_state": {"Offloaded": "0/8F"}`
Fixes #6337 Add safekeeper support to switch between `Present` and `Offloaded(flush_lsn)` states. The offloading is disabled by default, but can be controlled using new cmdline arguments: ``` --enable-offload Enable automatic switching to offloaded state --delete-offloaded-wal Delete local WAL files after offloading. When disabled, they will be left on disk --control-file-save-interval <CONTROL_FILE_SAVE_INTERVAL> Pending updates to control file will be automatically saved after this interval [default: 300s] ``` Manager watches state updates and detects when there are no actvity on the timeline and actual partial backup upload in remote storage. When all conditions are met, the state can be switched to offloaded. In `timeline.rs` there is `StateSK` enum to support switching between states. When offloaded, code can access only control file structure and cannot use `SafeKeeper` to accept new WAL. `FullAccessTimeline` is now renamed to `WalResidentTimeline`. This struct contains guard to notify manager about active tasks requiring on-disk WAL access. All guards are issued by the manager, all requests are sent via channel using `ManagerCtl`. When manager receives request to issue a guard, it unevicts timeline if it's currently evicted. Fixed a bug in partial WAL backup, it used `term` instead of `last_log_term` previously. After this commit is merged, next step is to roll this change out, as in issue #6338.
After #8022 was deployed to staging, I noticed many cases of timeouts. After inspecting the logs, I realized that some operations are taking ~20 seconds and they're doing while holding shared state lock. Usually it happens right after redeploy, because compute reconnections put high load on disks. This commit tries to improve observability around slow operations. Non-observability changes: - `TimelineState::finish_change` now skips update if nothing has changed - `wal_residence_guard()` timeout is set to 30s
## Problem There is an unused safekeeper option `partial_backup_enabled`. `partial_backup_enabled` was implemented in #6530, but this option was always turned into enabled in #8022. If you intended to keep this option for a specific reason, I will close this PR. ## Summary of changes I removed an unused safekeeper option `partial_backup_enabled`.
## Problem There is an unused safekeeper option `partial_backup_enabled`. `partial_backup_enabled` was implemented in #6530, but this option was always turned into enabled in #8022. If you intended to keep this option for a specific reason, I will close this PR. ## Summary of changes I removed an unused safekeeper option `partial_backup_enabled`.
Fixes #6337
Add safekeeper support to switch between
Present
andOffloaded(flush_lsn)
states. The offloading is disabled by default, but can be controlled using new cmdline arguments:Manager watches state updates and detects when there are no actvity on the timeline and actual partial backup upload in remote storage. When all conditions are met, the state can be switched to offloaded.
In
timeline.rs
there isStateSK
enum to support switching between states. When offloaded, code can access only control file structure and cannot useSafeKeeper
to accept new WAL.FullAccessTimeline
is now renamed toWalResidentTimeline
. This struct contains guard to notify manager about active tasks requiring on-disk WAL access. All guards are issued by the manager, all requests are sent via channel usingManagerCtl
. When manager receives request to issue a guard, it unevicts timeline if it's currently evicted.Fixed a bug in partial WAL backup, it used
term
instead oflast_log_term
previously.After this commit is merged, next step is to roll this change out, as in issue #6338.