mirror of
https://github.com/neondatabase/neon.git
synced 2026-05-24 00:20:37 +00:00
4af7a8253d
At the time of writing, the only logical difference between Attach and Load is that Attach learns the list of timelines by querying the remote storage, whereas Load learns it by listing the timelines directory of the tenant. This patch restructures the code such that Attach 1. prepares the on-disk state and then 2. calls into the same `load()` routine that is used by Load. Further, this patch provides the following fixes & improvements to Attach: 1. Make Attach durable before acknowledging it to the management API client. Before this change, we would acknowledge after just creating the in-memory tenant. In the event of a crash before creating the tenant directory and fsyncing the attaching marker, the pageserver would come up without the tenant present (404), even though we acknowledged success to the client. 2. Simplified resume logic if we crash during Attach. Before this patch, if we crashed during Attach with some timelines downloaded and others not downloaded, we would combine existing metadata files with remote ones one-by-one to figure out what's missing. That was necessary before on-demand download because we were downloading layer files as part of Attach. However, with on-demand download, Attach only downloads & writes the timeline metadata files. After this patch, when we crash during Attach, we blow away the tenant's directory while leaving its attach marker file in place. Then, we start over. IMO this is significantly easier to reason about compared to what we had before. Note that we were losing the work for the downloads even before this change, so that's not a regression (the old reconcile_with_remote would still need to download the `IndexPart`s when resuming Attach after a crash). If we want to improve on this in the future, I think the first order of business will be to avoiding re-downloading the `IndexPart`'s and the initial `list_remote_timelines()`. However, given that crashes should be rare, and attach events also, I don't think the number one priority with Attach code should be to make it as simple as possible. For (2), I changed the location of the attach marker file to be outside the tenant directory, so that we can use standard functions for removing the tenant directory. I even wrote a migration function for it, although in retrospect, I think it's quite unlikely that there are any tenants in attaching state deployed. But oh well, now the code is there, and it even has unit tests. We can delete the migration code once we've successfully rolled it out to all regions. The remaining wrinkle with this change is that Attach needs to hint the downloaded `IndexPart`s to `load()` so that it doesn't download them twice during Attach, which would be wasteful. The mechanism for this is the new `TenantLoadReason` and `TimelineLoadReason`. We could eliminate this particular case by on-demand downloading the metadata. However, that might open up another can of worms which I'd like to avoid. If we ever want to go that route, I suggest we start tracking the attachment state of a timeline more formally, e.g., in a `timelines.json` file. This is PR https://github.com/neondatabase/neon/pull/3466