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

walredo: stderr cleanup & make explicitly cancel safe #6031

Merged
merged 3 commits into from
Dec 4, 2023

Conversation

problame
Copy link
Contributor

@problame problame commented Dec 4, 2023

Problem

I need walredo to be cancellation-safe for #6000 (comment)

Solution

We are only async fn because of wait_for(stderr_logger_task_done).await, added in #5560 .

The stderr_logger_cancel and stderr_logger_task_done were there out of precaution that the stderr logger task might for some reason not stop when the walredo process terminates.
That hasn't been a problem in practice.
So, simplify things:

  • remove stderr_logger_cancel and the wait_for(...stderr_logger_task_done...)
  • use tokio::process::ChildStderr in the stderr logger task
  • add metrics to track number of running stderr logger tasks so in case I'm wrong here, we can use these metrics to identify the issue (not planning to put them into a dashboard or anything)

@problame problame self-assigned this Dec 4, 2023
pageserver/src/walredo.rs Outdated Show resolved Hide resolved
@problame problame marked this pull request as ready for review December 4, 2023 15:21
@problame problame requested a review from a team as a code owner December 4, 2023 15:21
Copy link

github-actions bot commented Dec 4, 2023

2436 tests run: 2340 passed, 0 failed, 96 skipped (full report)


Flaky tests (4)

Postgres 16

  • test_crafted_wal_end[last_wal_record_xlog_switch_ends_on_page_boundary]: release

Postgres 15

  • test_competing_branchings_from_loading_race_to_ok_or_err: release
  • test_branching_with_pgbench[cascade-1-10]: debug

Postgres 14

  • test_ondemand_download_timetravel[real_s3]: debug

Code coverage (full report)

  • functions: 54.6% (9297 of 17027 functions)
  • lines: 82.1% (54016 of 65796 lines)

The comment gets automatically updated with the latest test results
b1c0dd7 at 2023-12-04T16:00:49.188Z :recycle:

@problame problame merged commit 7403d55 into main Dec 4, 2023
41 checks passed
@problame problame deleted the problame/walredo-cancel-safety branch December 4, 2023 16:06
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.

None yet

2 participants