Make SnapshotsInProgress project compatible #125470
Conversation
This PR adds project-id to both SnapshotsInProgress and Snapshot so that they are aware of projects and ready to handle snapshots from multiple projects. Relates: ES-10224
ywangd
added
>non-issue
:Distributed Coordination/Snapshot/Restore
elasticsearchmachine
added
the
Team:Distributed Coordination
|
Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination) |
| @Deprecated(forRemoval = true) | ||
| static ProjectRepo defaultProjectRepo(String repoName) { | ||
| return new ProjectRepo(ProjectId.DEFAULT, repoName); | ||
| } |
There was a problem hiding this comment.
Should we add @FixForMultiProject? Also, wouldn't it make more sense to use Metadata.DEFAULT_PROJECT_ID here as this is a temporary usage?
There was a problem hiding this comment.
I deleted this method as part of the changes to add an extra ProjectId argument. See also #125470 (comment)
server/src/main/java/org/elasticsearch/repositories/RepositoryOperation.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/cluster/SnapshotsInProgress.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/repositories/RepositoryOperation.java
Outdated
Show resolved
Hide resolved
| final var oldVersion = TransportVersionUtils.getPreviousVersion(TransportVersions.PROJECT_ID_IN_SNAPSHOT); | ||
| final BytesStreamOutput out = new BytesStreamOutput(); | ||
| out.setTransportVersion(oldVersion); | ||
| final Custom orig = createTestInstance(() -> randomSnapshot(ProjectId.DEFAULT)); |
There was a problem hiding this comment.
I went with original. see e211f26
| if (isOldCluster()) { | ||
| // create an index to have one shard per node | ||
| createIndex(indexName, indexSettings(3, 0).put("index.routing.allocation.total_shards_per_node", 1).build()); | ||
| ensureGreen(indexName); |
There was a problem hiding this comment.
For the sake of completeness maybe (randomly) index some data?
| flush(indexName, true); | ||
| // Signal shutdown to prevent snapshot from being completed | ||
| putShutdownMetadata(nodeIds); | ||
| registerRepository(repositoryName, "fs", randomBoolean(), Settings.builder().put("location", "backup").build()); |
There was a problem hiding this comment.
nit: maybe doesn't matter, but seems more readable to register the repo a bit earlier?
| assertRunningSnapshot(repositoryName, snapshotName); | ||
| } else { | ||
| if (isUpgradedCluster()) { | ||
| deleteShutdownMetadata(nodeIds); |
There was a problem hiding this comment.
maybe assert that no shutdown metadata exists after deleting it. It might make possible test failures more concrete.
There was a problem hiding this comment.
We have SnapshotShutdownProgressTracker reporting quite detailed information about shard snapshot pausing and resuming due to shutdown. That said, it does not hurt to double check as well. So I added it in e211f26
ywangd
added
the
auto-merge-without-approval
This PR adds project-id to both SnapshotsInProgress and Snapshot so that they are aware of projects and ready to handle snapshots from multiple projects. Relates: ES-10224
This PR adds project-id to both SnapshotsInProgress and Snapshot so that they are aware of projects and ready to handle snapshots from multiple projects.
Relates: ES-10224