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

feat: allow detaching from ancestor for timelines without writes #7639

Merged
merged 21 commits into from
May 10, 2024

Conversation

koivunej
Copy link
Member

@koivunej koivunej commented May 7, 2024

The first implementation #7456 did not include index_part.json changes in an attempt to keep amount of changes down. Tracks the historic reparentings and earlier detach in index_part.json.

  • index_part.json receives a new field lineage: Lineage
  • Lineage is queried through RemoteTimelineClient during basebackup, creating PREV LSN: none for the invalid prev record lsn just as it would had been created for a newly created timeline
  • as struct IndexPart grew, it is now boxed in places

Cc: #6994

Base automatically changed from joonas/rtc_cleanup_confusing_clones to main May 7, 2024 16:22
Copy link

github-actions bot commented May 7, 2024

3060 tests run: 2927 passed, 0 failed, 133 skipped (full report)


Flaky tests (3)

Postgres 15

  • test_gc_aggressive: debug
  • test_vm_bit_clear_on_heap_lock: debug

Postgres 14

  • test_gc_aggressive: debug

Code coverage* (full report)

  • functions: 31.4% (6329 of 20161 functions)
  • lines: 47.3% (47710 of 100932 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
f2853dc at 2024-05-10T18:13:33.945Z :recycle:

@koivunej koivunej marked this pull request as ready for review May 7, 2024 19:27
@koivunej koivunej requested a review from a team as a code owner May 7, 2024 19:27
@koivunej koivunej requested a review from arpad-m May 7, 2024 19:27
@koivunej koivunej requested a review from problame May 7, 2024 19:30
Copy link
Contributor

@problame problame left a comment

Choose a reason for hiding this comment

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

Please create a PR against the RFC PR with a section on Lineage. Or, work with John to add it.


This one needs updating:

/// - has prev_lsn in remote storage (temporary restriction)


Didn't have time to review the test and won't have because I'm off tomorrow and Friday.

pageserver/src/tenant/remote_timeline_client/index.rs Outdated Show resolved Hide resolved
pageserver/src/tenant/remote_timeline_client/index.rs Outdated Show resolved Hide resolved
pageserver/src/tenant/timeline.rs Show resolved Hide resolved
@koivunej
Copy link
Member Author

Please create a PR against the RFC PR with a section on Lineage. Or, work with John to add it.

I've already added it, it's for WAL DR (I've pushed comments around the "why").

having a vec of timeline ids does not help, it must be a Vec of branch
points.
@koivunej koivunej merged commit 6351313 into main May 10, 2024
54 checks passed
@koivunej koivunej deleted the joonas/lineage branch May 10, 2024 19:30
@problame
Copy link
Contributor

I've already added it,

Where?

@problame
Copy link
Contributor

Ok. I think it would be useful to somehow link the Rust struct and that section of the RFC.
It wasn't clear to me while reviewing that we need the lineage for WAL-based DR.
When I reviewed, it just read to me like "we're doing some book-keeping to make future debugging / troubleshooting easier".
I don't think there's any mentioning in the struct Lineage doc comment.

@koivunej
Copy link
Member Author

Adding that to the TODO.

a-masterov pushed a commit that referenced this pull request May 20, 2024
The first implementation #7456 did not include `index_part.json` changes
in an attempt to keep amount of changes down. Tracks the historic
reparentings and earlier detach in `index_part.json`.

- `index_part.json` receives a new field `lineage: Lineage`
- `Lineage` is queried through RemoteTimelineClient during basebackup,
creating `PREV LSN: none` for the invalid prev record lsn just as it
would had been created for a newly created timeline
- as `struct IndexPart` grew, it is now boxed in places

Cc: #6994
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.

2 participants