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

Add FullAccessTimeline guard in safekeepers #7887

Merged
merged 14 commits into from
May 31, 2024
Merged

Add FullAccessTimeline guard in safekeepers #7887

merged 14 commits into from
May 31, 2024

Conversation

petuhovskiy
Copy link
Member

@petuhovskiy petuhovskiy commented May 26, 2024

This is a preparation for #6337.

The idea is to add FullAccessTimeline, which will act as a guard for tasks requiring access to WAL files. Eviction will be blocked on these tasks and WAL won't be deleted from disk until there is at least one active FullAccessTimeline.

To get FullAccessTimeline, tasks call tli.full_access_guard().await?. After eviction is implemented, this function will be responsible for downloading missing WAL file and waiting until the download finishes.

This commit also contains other small refactorings:

  • Separate get_tenant_dir and get_timeline_dir functions for building a local path. This is useful for looking at usages and finding tasks requiring access to local filesystem.
  • timeline_manager is now responsible for spawning all background tasks
  • WAL removal task is now spawned instantly after horizon is updated

Copy link

github-actions bot commented May 26, 2024

3156 tests run: 3017 passed, 0 failed, 139 skipped (full report)


Flaky tests (2)

Postgres 16

  • test_vm_bit_clear_on_heap_lock: debug

Postgres 15

  • test_timeline_deletion_with_files_stuck_in_upload_queue: debug

Code coverage* (full report)

  • functions: 31.4% (6494 of 20689 functions)
  • lines: 48.4% (50232 of 103835 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
07a3e9a at 2024-05-31T13:28:38.434Z :recycle:

@petuhovskiy petuhovskiy force-pushed the sk-task-guards branch 2 times, most recently from c4fb322 to 81fcfeb Compare May 29, 2024 14:57
@petuhovskiy petuhovskiy changed the title WIP: Safekeeper tasks refactoring Add FullAccessTimeline guard in safekeepers May 29, 2024
@petuhovskiy petuhovskiy marked this pull request as ready for review May 29, 2024 15:33
@petuhovskiy petuhovskiy requested review from a team as code owners May 29, 2024 15:33
@petuhovskiy petuhovskiy requested a review from arssher May 29, 2024 15:33
safekeeper/src/timeline_manager.rs Show resolved Hide resolved
@petuhovskiy petuhovskiy enabled auto-merge (squash) May 31, 2024 12:41
@petuhovskiy petuhovskiy merged commit 16b2e74 into main May 31, 2024
57 of 58 checks passed
@petuhovskiy petuhovskiy deleted the sk-task-guards branch May 31, 2024 13:19
petuhovskiy added a commit that referenced this pull request Jun 1, 2024
During refactoring in #7887 I
forgot to add "WAL removal" span with ttid. This commit fixes it.
a-masterov pushed a commit that referenced this pull request Jun 3, 2024
This is a preparation for
#6337.

The idea is to add FullAccessTimeline, which will act as a guard for
tasks requiring access to WAL files. Eviction will be blocked on these
tasks and WAL won't be deleted from disk until there is at least one
active FullAccessTimeline.

To get FullAccessTimeline, tasks call `tli.full_access_guard().await?`.
After eviction is implemented, this function will be responsible for
downloading missing WAL file and waiting until the download finishes.

This commit also contains other small refactorings:
- Separate `get_tenant_dir` and `get_timeline_dir` functions for
building a local path. This is useful for looking at usages and finding
tasks requiring access to local filesystem.
- `timeline_manager` is now responsible for spawning all background
tasks
- WAL removal task is now spawned instantly after horizon is updated
a-masterov pushed a commit that referenced this pull request Jun 3, 2024
During refactoring in #7887 I
forgot to add "WAL removal" span with ttid. This commit fixes it.
@shinyaaa shinyaaa mentioned this pull request Jul 19, 2024
5 tasks
skyzh pushed a commit that referenced this pull request Jul 19, 2024
There are unused safekeeper runtimes `WAL_REMOVER_RUNTIME` and
`METRICS_SHIFTER_RUNTIME`.

`WAL_REMOVER_RUNTIME` was implemented in
[#4119](#4119) and removed in
[#7887](#7887).
`METRICS_SHIFTER_RUNTIME` was also implemented in
[#4119](#4119) but has never
been used.

I removed unused safekeeper runtimes `WAL_REMOVER_RUNTIME` and
`METRICS_SHIFTER_RUNTIME`.
problame pushed a commit that referenced this pull request Jul 22, 2024
There are unused safekeeper runtimes `WAL_REMOVER_RUNTIME` and
`METRICS_SHIFTER_RUNTIME`.

`WAL_REMOVER_RUNTIME` was implemented in
[#4119](#4119) and removed in
[#7887](#7887).
`METRICS_SHIFTER_RUNTIME` was also implemented in
[#4119](#4119) but has never
been used.

I removed unused safekeeper runtimes `WAL_REMOVER_RUNTIME` and
`METRICS_SHIFTER_RUNTIME`.
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