feat(pageserver): use json for timeline metadata encoding

[skyzh]

Problem

close #7540

Summary of changes

Change the encoding of timeline metadata to JSON. Note that the index_part still uses a weird representation that:

{
  "metadata_bytes": "<non-ascii header><timeline JSON>"
}

but using JSON at least gives us easy forward compatibility that serde_json ignores unknown fields, and new fields can be defined as Option<X> so that forward/backward compatibility is guaranteed.

Checklist before requesting a review

• I have performed a self-review of my code.
• If it is a core feature, I have added thorough tests.
• Do we need to implement analytics? if so did you add the relevant metrics to the dashboard?
• If this PR requires public announcement, mark it with /release-notes label and add several sentences in this section.

Checklist before merging

• Do not forget to reformat commit message to not include the above checklist

[github-actions]

3024 tests run: 2892 passed, 0 failed, 132 skipped (full report)

---

Flaky tests (4)

Postgres 15

• test_crafted_wal_end[wal_record_crossing_segment_followed_by_small_one]: debug
• test_gc_aggressive: debug
• test_vm_bit_clear_on_heap_lock: debug

Postgres 14

• test_lock_time_tracing: release

Code coverage* (full report)

• functions: 31.4% (6320 of 20146 functions)
• lines: 47.3% (47668 of 100827 lines)

* collected from Rust tests only

---

_{The comment gets automatically updated with the latest test results
2682a32 at 2024-05-09T20:37:21.601Z :recycle:}

[arpad-m]

Note that the index_part still uses a weird representation that

can we somehow avoid that? It would be great if it were a native json object instead of inside a string inside the json.

[jcsp]

For forward compatibility, can we write the old format for timelines that have not switched to aux files v2 yet? That way we can retain the option to rollback the next release, but also not have to wait another week to roll out the write path

[skyzh]

The code logic to implement this is like: if aux file v2 is enabled, decode with JSON; however, we cannot know whether it's enabled before decoding the metadata body. It's kind of egg-and-chicken problem.

The alternative is to maintain two versions of timeline metadata here and do not automatically upgrade the timeline metadata version, WDYT?

[skyzh]

... which means that aux v2 enabled -> metadata v3, aux v2 disabled -> metadata v2

[skyzh]

fixed in 396b9cf

[skyzh]

can we somehow avoid that? It would be great if it were a native json object instead of inside a string inside the json.

@arpad-m My current concern is that index_part.json does not store a checksum header, and therefore, I'm not comfortable with storing the timeline metadata simply with a JSON field (i.e., strip the header and not using string literal for that). Implementation-wise, it also needs a bump for the index part format version, which does not have a clear upgrade path as the TimelineMetadata implementation, and this is something I plan to do in the future.

[arpad-m]

Hmm yeah there is no "normal forms" of json that I know of that one could use for obtaining the reliability one needs to compute checksums, nor is there some other standardized checksumming algorithm. The algorithms that do employ checksums or signatures of json objects usually just compute it on the string representation, but then one has to include it as json string obviously.

Edit: JWT for example

[skyzh]

note that forward compatibility test passed, which means something is going wrong with the test case itself

[skyzh]

maybe related: c20a379?

[skyzh]

forward compat broke as expected after the revert 03038da

[skyzh]

Now we write the old binary format by default and do not automatically upgrade to the new JSON format unless aux file flag is set (aux file flag is part of the next PR in the queue). Ready for review, need to rerun CI after #7684 gets merged to ensure forward compatibility. rerun triggered

[save-buffer]

Should we make an enum for these format versions?

[save-buffer]

(Don't have to do it in this PR, just general question)

[skyzh]

yep, I agree -- with more versions in the future, there should be an enum to map from FORMAT_VERSION_VX to a number.

[koivunej]

I am not sure how this change will be used, but I don't think we should change TimelineMetadata at all -- we have index_part.json where we should also render this as plain json.

We can never let go of reading the old serialization format because of detached tenants, but at least I cannot see a reason from this PR to keep using it in favor of modifying only IndexPart. I'll fire up an alternative for real json format, but in the meantime, I'd like to see some more justification for these changes.

The PR which adds support for interpreting the index_part.json field as the current format (before this PR) or plain json (in future): #7693. The "next weeks" follow-up to stop using Vec<u8> or bincode in json format completely: #7699.

To reiterate from those PR's:

• I don't think we should add any data to TimelineMetadata
  • Instead add all fields to index_part.json where we already have a versioning/migration story
• Current TimelineMetadata will be needed until we've confirmed that there are no longer index_part.json out there using the old format
  • Until then, we shouldn't change TimelineMetadata

[skyzh]

replaced by #7693