Do tenant manifest validation with index-part

[arpad-m]

This adds some validation of invariants that we want to uphold wrt the tenant manifest and index_part.json:

• the data the manifest has about a timeline must match with the data in index_part.json. It might actually change, e.g. when we do reparenting during detach ancestor, but that requires the timeline to be unoffloaded, i.e. removed from the manifest.
• any timeline mentioned in index part, must, if present, be archived. If we unarchive, we first update the tenant manifest to unoffload, and only then update index part. And one needs to archive before offloading.
• it is legal for timelines to be mentioned in the manifest but have no index_part: this is a temporary state visible during deletion of the timeline. if the pageserver crashed, an attach of the tenant will clean the state up.
• it is also legal for offloaded timelines to have an ancestor_retain_lsn of None while having an ancestor_timeline_id. This is for the to-be-added flattening functionality: the plan is to set former to None if we have flattened a timeline.

follow-up of #9942
part of #8088

[github-actions]

7077 tests run: 6752 passed, 0 failed, 325 skipped (full report)

---

Flaky tests (6)

Postgres 17

• test_ondemand_wal_download_in_replication_slot_funcs: release-arm64
• test_scrubber_physical_gc_ancestors[2]: release-arm64

Postgres 15

• test_pull_timeline[True]: release-x86-64

Postgres 14

• test_prefetch[4]: release-arm64
• test_pull_timeline[True]: release-arm64, release-x86-64

Code coverage* (full report)

• functions: 31.4% (8329 of 26533 functions)
• lines: 47.7% (65545 of 137448 lines)

* collected from Rust tests only

---

_{The comment gets automatically updated with the latest test results
ba19374 at 2024-12-09T21:38:22.039Z :recycle:}

[problame]

I'm skeptical that the checks in validate_index_part_with_offloaded are race-free because we can't fetch the manifest and the index part atomically.

For example, the following sequence

scrubber: fetches manifest, has timeline as archived
pageserver: timeline gets unarchived, uploads index part with archived_at=None
scrubber: fetches index part
scrubber: runs validate_index_part_with_offloaded
scrubber: logs Timeline offloaded in manifest but not archived in index-part

---

I already complained about the messiness of the tenants.map_ok(|...| { ... } inside pageserver_physical_gc in the previous PR that added tenant manifest validation to the scrubber.

I think it's really getting out of hand - there must be a cleaner way to express what we're doing here -,-.

Not asking you to solve it though, but, maybe raise it in #rust once this PR is merged, asking for prettification from the rust async gurus.

[arpad-m]

I'm skeptical that the checks in validate_index_part_with_offloaded are race-free because we can't fetch the manifest and the index part atomically.

hmm yeah good point. What about a HEAD request? If we discover any errors, we could check if the manifest is still unchanged (comparing etag, last-modified, etc) since the index part has been downloaded. If the manifest has changed, we either demote it to info level, or suppress it entirely.

I think it's really getting out of hand - there must be a cleaner way to express what we're doing here -,-.

Indeed it's quite a mess, I'm sorry. I'm also a fan of imperative programming over functional programming because you get into precisely this situation if you want to move a little bit of data around in a way that is a little bit more custom than simple map.

There is advantages though: stuff like parallel execution is quite easy: we have both timeline and tenant level multi threading here, but it's also bounded by the number of configured threads. So if you invoke the scrubber with a single tenant that has many timelines it still supports parallelism.

I think that's pretty cool and a property I wanted to preserve. getting that right if done manually is pretty hard and there is lots of subtle bugs that can show up, as in you can't just have it spawn to an unbounded channel/executor, potentially growing ram, you need to have backpressure, etc etc. Maybe with a semaphore? idk.

It might be worth a shot to try applying macros from async_stream to this code. I can try that after this PR is merged.

[arpad-m]

If we discover any errors, we could check if the manifest is still unchanged (comparing etag, last-modified, etc) since the index part has been downloaded. If the manifest has changed, we either demote it to info level, or suppress it entirely.

I've pushed a commit to do this. A HEAD request isn't enough. One needs to do a listing of all generations: the "new" manifest could be of a newer generation than the manifest we've had previously.

[problame]

The problem with checking last_modified is its relatively coarse resolution (milliseconds).
I guess it's more of a theoretical problem, but still, kind of dissatisfying.

scrubber: fetches manifest, has timeline as archived_at=T1
pageserver: timeline gets unarchived, uploads index part with archived_at=None
scrubber: fetches index part observes archived_at=None
scrubber: runs validate_index_part_with_offloaded
scrubber: validate fails with logs index-part archived_at={} differs from manifest archived_at={}
scrubber: decides to fetch mainfest again
pageserver: archives timeline at T2, updates both index part and manifest
scrubber: fetches manifest again, observes archived_at=T2
scrubber: second validation would fail again, but last_modified very likely mismatches  

I think a common on the last_modified comparison being imperfect but practically probably sufficient is appropriate.

[arpad-m]

The problem with checking last_modified is its relatively coarse resolution (milliseconds).

Actually it's worse, it's only a resolution of seconds. You are right, we should maybe use etags instead. We have etags in the remote_storage API already, just not for ListingObject. I think it would be good for a followup.