pageserver: reorder upload queue when possible

[erikgrinaker]

Problem

The upload queue currently sees significant head-of-line blocking. For example, index uploads act as upload barriers, and for every layer flush we schedule a layer and index upload, which effectively serializes layer uploads.

Resolves #10096.

Summary of changes

Allow upload queue operations to bypass the queue if they don't conflict with preceding operations, increasing parallelism.

NB: the upload queue currently schedules an explicit barrier after every layer flush as well (see #8550). This must be removed to enable parallelism. This will require a better mechanism for compaction backpressure, see e.g. #8390 or #5415.

[github-actions]

7330 tests run: 6959 passed, 0 failed, 371 skipped (full report)

---

Flaky tests (2)

Postgres 17

• test_scrubber_tenant_snapshot[4]: debug-x86-64

Postgres 14

• test_lr_with_slow_safekeeper: release-arm64

Code coverage* (full report)

• functions: 32.9% (8143 of 24756 functions)
• lines: 48.2% (68254 of 141718 lines)

* collected from Rust tests only

---

_{The comment gets automatically updated with the latest test results
11a217e at 2025-01-14T14:45:52.163Z :recycle:}

[erikgrinaker]

Added a benchmark measuring the next_ready() time for an UploadMetadata with a given number of Delete tasks already in progress. This shows a linear cost of 1 ms per 10,000 in-progress operations (per queued task). 85% of this is due to LayerName::hash for IndexPart::layer_metadata lookups. We can estimate that the common case of layer uploads/deletes only use 1 ms per 100,000 in-progress operations.

It isn't clear that we'll hit this number of in-progress operations under realistic conditions, so this is probably fine.

[VladLazar]

Went over it a few times. Looks correct and the tests are convincing. Perhaps worth a second pair of eyes though.

[erikgrinaker]

@skyzh Want to give this another look-over?

[skyzh]

overall LGTM.

The patch is still bottlenecked by two things: flush deletion queue (as mentioned in #10283) and reorder index part uploads. These could be addressed in future patches.

The patch allows uploads to skip the queue if they are not yet in index_part yet, which means that we can upload more aggressively, and that we would potentially get files not referenced in index_part in the remote storage. I'll update the storage scrubber check at some point.

Moving forward, we can of course modify the bypass algorithm to allow more parallelism (i.e., batch index part upload), but I wonder if things could get easier with the following ideas:

• Having a unique ID for each of the file (i.e., a monotonically increasing ID for each layer file) so that we don't need to deal with upload-delete race at all.
• Handle meta operations in the upload queue instead of atomic primitives like upload/delete. i.e., we only have a few operations in the layer file manager: L0 compaction, freeze/flush, GC. This means that the operations submitted to the upload queue are always in the form of "upload layers x N, deletion, update index part". Hence, upload/delete never needs to get reordered between between index_part uploads because deletes always get appended to the queue after uploads for each of the meta operation.

[skyzh]

Can we also apply the env variable in CI and see if it passes all tests?

[erikgrinaker]

Finally getting back to this. TFTRs!

we would potentially get files not referenced in index_part in the remote storage

This is already the case today -- we always have to upload the file before we upload the index. This won't become more common with this patch, since we don't delay index uploads more than we did before. If anything it might become less common, because index uploads no longer necessarily have to wait for all in-progress operations to complete first.

The patch is still bottlenecked by two things: flush deletion queue (as mentioned in #10283) and reorder index part uploads.

It's not really bottlenecked on the deletion queue race -- that race already exists today, but we can more easily fix that race now. And I think we'll have to do something similar to the current deletion queue flushing even if we do address it.

See #10248 for index upload coalescing.

Having a unique ID for each of the file (i.e., a monotonically increasing ID for each layer file) so that we don't need to deal with upload-delete race at all.

Yeah, immutable files are generally a much better idea. I'd be +1 on doing that instead at some point, and making path conflicts an error condition.

Handle meta operations in the upload queue instead of atomic primitives like upload/delete

I think I'd prefer smaller atomic primitives -- it's a smaller set of cases to deal with, and it's easier to reason about the interactions.

That scheme also wouldn't address the motivating case here: layer flushes currently upload layer/index/layer/index, and we want to upload the layers in parallel.

Can we also apply the env variable in CI and see if it passes all tests?

Good idea, I'll open a draft PR for a CI run.

[erikgrinaker]

Can we also apply the env variable in CI and see if it passes all tests?

Good idea, I'll open a draft PR for a CI run.

#10385

I'm also going to add a limit for the number of in-progress items here, and set it to the remote storage concurrency (100), since it's pointless to schedule more tasks that this. That will put a bound on the complexity in most common cases

Split this out to a follow-up PR: #10384.

[erikgrinaker]

All tests passed on #10385, let's see if this thing flies.