Small cleanup in BlobStoreRepository#finalizeSnapshot #99635
Conversation
Reorders the operations into their logical order and adds a few TODOs for gaps that this cleanup exposes.
DaveCTurner
added
:Distributed Coordination/Snapshot/Restore
|
After going through this during onboarding I think there's room for improvement... |
elasticsearchmachine
added
the
Team:Distributed (Obsolete)
|
Pinging @elastic/es-distributed (Team:Distributed) |
…eRepository-finalizeSnapshot-cleanup
| .<MetadataWriteResult>andThen((l, existingRepositoryData) -> { | ||
| final int existingSnapshotCount = existingRepositoryData.getSnapshotIds().size(); | ||
| if (existingSnapshotCount >= maxSnapshotCount) { | ||
| throw new RepositoryException( |
There was a problem hiding this comment.
NB slight change in exception semantics (see change to BlobStoreSizeLimitIT) - previously this exception would be thrown directly to the outer listener, but now it's wrapped in a SnapshotException first. I claim that the new behaviour is preferable, it's useful to report that this specific snapshot failed, as well as the cause.
| allMetaListeners.acquire(), | ||
| () -> GLOBAL_METADATA_FORMAT.write(clusterMetadata, blobContainer(), snapshotId.getUUID(), compress) | ||
| ) | ||
| // NB failure of writeIndexGen doesn't guarantee the update failed, so we cannot safely clean anything up on failure |
There was a problem hiding this comment.
For my learning: Could you please expand a bit on what this means? If the index-N file fails to update, there will be no root level reference to the new shard generation and data files? If so, why is it not safe to clean up?
There was a problem hiding this comment.
It's a good question. I opened #99645 to answer this in the Javadocs so that others can find this info in future too. Please take a look, and if you need more detail then let's discuss there.
DaveCTurner
deleted the
2023/09/18/BlobStoreRepository-finalizeSnapshot-cleanup
branch
DaveCTurner
restored the
2023/09/18/BlobStoreRepository-finalizeSnapshot-cleanup
branch
Reorders the operations into their logical order and adds a few TODOs
for gaps that this cleanup exposes.