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

refactor(walredo): split up walredo.rs #6583

Merged
merged 12 commits into from
Feb 6, 2024

Conversation

problame
Copy link
Contributor

@problame problame commented Feb 2, 2024

The file is massive, and can be broken up neatly into

  • process lifecycle management
  • IPC protocol
  • applying neon-specific walredo records
  • overall walredo manager

This is prep work for #6581

Copy link

github-actions bot commented Feb 2, 2024

2388 tests run: 2273 passed, 0 failed, 115 skipped (full report)


Flaky tests (1)

Postgres 16

  • test_timeline_deletion_with_files_stuck_in_upload_queue: debug

Code coverage (full report)

  • functions: 54.4% (11283 of 20741 functions)
  • lines: 81.5% (63516 of 77969 lines)

The comment gets automatically updated with the latest test results
05f3d70 at 2024-02-05T13:23:40.149Z :recycle:

@problame problame marked this pull request as ready for review February 2, 2024 11:52
@problame problame requested a review from a team as a code owner February 2, 2024 11:52
@problame problame requested review from VladLazar and removed request for a team February 2, 2024 11:52
problame added a commit that referenced this pull request Feb 2, 2024
When we'll later introduce a global pool of pre-spawned walredo
processes (#6581), this
refactoring avoids plumbing through the reference to the pool to all the
places where we create a broken tenant.

Builds atop the refactoring in #6583
Copy link
Contributor

@VladLazar VladLazar left a comment

Choose a reason for hiding this comment

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

LGTM, but clippy has a complaint:

error: unused import: `atomic::AtomicUsize`
    --> pageserver/src/walredo/process.rs:17:12
     |
  17 |     sync::{atomic::AtomicUsize, Mutex, MutexGuard},
     |            ^^^^^^^^^^^^^^^^^^^
     |
     = note: `-D unused-imports` implied by `-D warnings`
     = help: to override `-D warnings` add `#[allow(unused_imports)]`

@problame problame enabled auto-merge (squash) February 5, 2024 10:42
@problame problame changed the title refactor(walredo): split up the massive walredo.rs refactor(walredo): split up walredo.rs Feb 6, 2024
@problame problame merged commit edcde05 into main Feb 6, 2024
46 checks passed
@problame problame deleted the problame/2024-02-walredo-work/prespawn/split-code branch February 6, 2024 09:44
problame added a commit that referenced this pull request Feb 6, 2024
)

When we'll later introduce a global pool of pre-spawned walredo
processes (#6581), this
refactoring avoids plumbing through the reference to the pool to all the
places where we create a broken tenant.

Builds atop the refactoring in #6583
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