From b0520eb47bf164f8884e316fdc04d0e12e8173ce Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Wed, 22 May 2019 15:46:12 +0200 Subject: [PATCH 1/2] Remove deprecated Repository methods We deprecated `restoreShard` and `snapshotShard` in #42213 This change removes the deprecated methods and their usage and adds a note in the migration docs. --- .../migration/migrate_8_0/java.asciidoc | 8 ++++ .../index/shard/StoreRecovery.java | 2 +- .../repositories/Repository.java | 40 ------------------- .../snapshots/SnapshotShardsService.java | 3 +- 4 files changed, 11 insertions(+), 42 deletions(-) diff --git a/docs/reference/migration/migrate_8_0/java.asciidoc b/docs/reference/migration/migrate_8_0/java.asciidoc index 523e5b463d8bc..bc24f9e81791c 100644 --- a/docs/reference/migration/migrate_8_0/java.asciidoc +++ b/docs/reference/migration/migrate_8_0/java.asciidoc @@ -25,3 +25,11 @@ while silently truncating them to one of the three allowed edit distances 0, 1 or 2. This leniency is now removed and the class will throw errors when trying to construct an instance with another value (e.g. floats like 1.3 used to get accepted but truncated to 1). You should use one of the allowed values. + + +[float] +==== Changes to Repository + +Repository has no dependency to IndexShard anymore. The contract of restoreShard +and snapshotShard has been reduced to Store and MappingService in order to improve +testability. diff --git a/server/src/main/java/org/elasticsearch/index/shard/StoreRecovery.java b/server/src/main/java/org/elasticsearch/index/shard/StoreRecovery.java index aa49f7ecb60ce..fae3703027f9e 100644 --- a/server/src/main/java/org/elasticsearch/index/shard/StoreRecovery.java +++ b/server/src/main/java/org/elasticsearch/index/shard/StoreRecovery.java @@ -469,7 +469,7 @@ private void restore(final IndexShard indexShard, final Repository repository, f } final IndexId indexId = repository.getRepositoryData().resolveIndexId(indexName); assert indexShard.getEngineOrNull() == null; - repository.restoreShard(indexShard, indexShard.store(), restoreSource.snapshot().getSnapshotId(), + repository.restoreShard(indexShard.store(), restoreSource.snapshot().getSnapshotId(), restoreSource.version(), indexId, snapshotShardId, indexShard.recoveryState()); final Store store = indexShard.store(); store.bootstrapNewHistory(); diff --git a/server/src/main/java/org/elasticsearch/repositories/Repository.java b/server/src/main/java/org/elasticsearch/repositories/Repository.java index 3aa19cb130cae..72ebe16a471cc 100644 --- a/server/src/main/java/org/elasticsearch/repositories/Repository.java +++ b/server/src/main/java/org/elasticsearch/repositories/Repository.java @@ -189,27 +189,6 @@ SnapshotInfo finalizeSnapshot(SnapshotId snapshotId, List indices, long */ boolean isReadOnly(); - /** - * Creates a snapshot of the shard based on the index commit point. - *

- * The index commit point can be obtained by using {@link org.elasticsearch.index.engine.Engine#acquireLastIndexCommit} method. - * Repository implementations shouldn't release the snapshot index commit point. It is done by the method caller. - *

- * As snapshot process progresses, implementation of this method should update {@link IndexShardSnapshotStatus} object and check - * {@link IndexShardSnapshotStatus#isAborted()} to see if the snapshot process should be aborted. - * @param indexShard the shard to be snapshotted - * @param snapshotId snapshot id - * @param indexId id for the index being snapshotted - * @param snapshotIndexCommit commit point - * @param snapshotStatus snapshot status - * @deprecated use {@link #snapshotShard(Store, MapperService, SnapshotId, IndexId, IndexCommit, IndexShardSnapshotStatus)} instead - */ - @Deprecated - default void snapshotShard(IndexShard indexShard, SnapshotId snapshotId, IndexId indexId, IndexCommit snapshotIndexCommit, - IndexShardSnapshotStatus snapshotStatus) { - snapshotShard(indexShard.store(), indexShard.mapperService(), snapshotId, indexId, snapshotIndexCommit, snapshotStatus); - } - /** * Creates a snapshot of the shard based on the index commit point. *

@@ -228,25 +207,6 @@ default void snapshotShard(IndexShard indexShard, SnapshotId snapshotId, IndexId void snapshotShard(Store store, MapperService mapperService, SnapshotId snapshotId, IndexId indexId, IndexCommit snapshotIndexCommit, IndexShardSnapshotStatus snapshotStatus); - /** - * Restores snapshot of the shard. - *

- * The index can be renamed on restore, hence different {@code shardId} and {@code snapshotShardId} are supplied. - * @param shard the shard to restore the index into - * @param store the store to restore the index into - * @param snapshotId snapshot id - * @param version version of elasticsearch that created this snapshot - * @param indexId id of the index in the repository from which the restore is occurring - * @param snapshotShardId shard id (in the snapshot) - * @param recoveryState recovery state - * @deprecated use {@link #restoreShard(Store, SnapshotId, Version, IndexId, ShardId, RecoveryState)} instead - */ - @Deprecated - default void restoreShard(IndexShard shard, Store store, SnapshotId snapshotId, Version version, IndexId indexId, - ShardId snapshotShardId, RecoveryState recoveryState) { - restoreShard(store, snapshotId, version, indexId, snapshotShardId, recoveryState); - } - /** * Restores snapshot of the shard. *

diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotShardsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotShardsService.java index f79b6da6ef626..b21df093fadd2 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotShardsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotShardsService.java @@ -367,7 +367,8 @@ private void snapshot(final IndexShard indexShard, final Snapshot snapshot, fina try { // we flush first to make sure we get the latest writes snapshotted try (Engine.IndexCommitRef snapshotRef = indexShard.acquireLastIndexCommit(true)) { - repository.snapshotShard(indexShard, snapshot.getSnapshotId(), indexId, snapshotRef.getIndexCommit(), snapshotStatus); + repository.snapshotShard(indexShard.store(), indexShard.mapperService(), snapshot.getSnapshotId(), indexId, + snapshotRef.getIndexCommit(), snapshotStatus); if (logger.isDebugEnabled()) { final IndexShardSnapshotStatus.Copy lastSnapshotStatus = snapshotStatus.asCopy(); logger.debug("snapshot ({}) completed to {} with {}", snapshot, repository, lastSnapshotStatus); From ed969dcaa7ac1d4cce3554cea06f8ff4230ab228 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Wed, 22 May 2019 16:54:10 +0200 Subject: [PATCH 2/2] Apply feedback --- docs/reference/migration/migrate_8_0/java.asciidoc | 2 +- .../main/java/org/elasticsearch/repositories/Repository.java | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/reference/migration/migrate_8_0/java.asciidoc b/docs/reference/migration/migrate_8_0/java.asciidoc index bc24f9e81791c..21d281acff97f 100644 --- a/docs/reference/migration/migrate_8_0/java.asciidoc +++ b/docs/reference/migration/migrate_8_0/java.asciidoc @@ -30,6 +30,6 @@ accepted but truncated to 1). You should use one of the allowed values. [float] ==== Changes to Repository -Repository has no dependency to IndexShard anymore. The contract of restoreShard +Repository has no dependency on IndexShard anymore. The contract of restoreShard and snapshotShard has been reduced to Store and MappingService in order to improve testability. diff --git a/server/src/main/java/org/elasticsearch/repositories/Repository.java b/server/src/main/java/org/elasticsearch/repositories/Repository.java index 72ebe16a471cc..0eca92039fbf8 100644 --- a/server/src/main/java/org/elasticsearch/repositories/Repository.java +++ b/server/src/main/java/org/elasticsearch/repositories/Repository.java @@ -27,7 +27,6 @@ import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.common.component.LifecycleComponent; import org.elasticsearch.index.mapper.MapperService; -import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.index.snapshots.IndexShardSnapshotStatus; import org.elasticsearch.index.store.Store;