-
Notifications
You must be signed in to change notification settings - Fork 434
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(pageserver): add aux_file_v2 feature flag #7424
Conversation
2754 tests run: 2633 passed, 1 failed, 120 skipped (full report)Failures on Postgres 16
Test coverage report is not availableThe comment gets automatically updated with the latest test results
37e25ab at 2024-04-24T18:30:47.141Z :recycle: |
Signed-off-by: Alex Chi Z <chi@neon.tech> add tests Signed-off-by: Alex Chi Z <chi@neon.tech>
Signed-off-by: Alex Chi Z <chi@neon.tech>
63da363
to
4b23692
Compare
need to fix unit tests, e2e works. |
Signed-off-by: Alex Chi Z <chi@neon.tech>
Signed-off-by: Alex Chi Z <chi@neon.tech>
b21adb0
to
21db1bc
Compare
As said on slack, I think for the rollout we can just add it as a per-tenant config, no need to have it in timeline metadata. That would simplify this I think. |
@arpad-m The timeline metadata is here to prevent the flag from being accidentally reverted, and also handle branching/history correctly. If the control plane / storage controller can guarantee that the flag won't be flipped, then it's fine to set it as a tenant config. (I can simply rename the current tenant config |
I feel the only thing we are uncertain is whether we need to persist the aux_file_v2 state. Maybe I can submit a pull request with only tenant config change to unblock myself, and go back to this discussion later? |
Opened #7505 instead. We can discuss the metadata change later. |
Changing metadata format is not easy. This pull request adds a tenant-level flag on whether to enable aux file v2. As long as we don't roll this out to the user and guarantee our staging projects can persist tenant config correctly, we can test the aux file v2 change with setting this flag. Previous discussion at #7424. Signed-off-by: Alex Chi Z <chi@neon.tech>
Problem
Added
aux_file_v2
to theTimelineMetadata
structure. The structure has not been touched for several months so I'm skeptical whether we can really add it there...The flag cannot be reverted to false once being set. Once the tenant config has
try_enable_aux_file_v2
set to true, the write path of AUX file will read this flag and set the timeline to haveaux_file_v2
enabled. This flag will be persisted into the index part, so even if the flag got lost in the tenant config, the page server will still proceed with the v2 storage.Summary of changes
Checklist before requesting a review
Checklist before merging