Skip to content

Commit

Permalink
Addressing comments
Browse files Browse the repository at this point in the history
Signed-off-by: Shubh Sahu <[email protected]>
  • Loading branch information
Shubh Sahu committed Jan 7, 2025
1 parent f495cad commit 3947531
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 22 deletions.
16 changes: 5 additions & 11 deletions server/src/main/java/org/opensearch/index/shard/IndexShard.java
Original file line number Diff line number Diff line change
Expand Up @@ -1625,25 +1625,19 @@ public org.apache.lucene.util.Version minimumCompatibleVersion() {
}

/**
* reads the last metadata file from remote store and fetches files present in commit and their sizes.
* @return Tuple(Tuple(primaryTerm, commitGeneration), indexFilesToFileLengthMap)
* Fetches the last remote uploaded segment metadata file
* @return {@link RemoteSegmentMetadata}
* @throws IOException
*/
public Tuple<Tuple<Long, Long>, Map<String, Long>> acquireLastRemoteUploadedIndexCommit() throws IOException {
public RemoteSegmentMetadata fetchLastRemoteUploadedSegmentMetadata() throws IOException {
if (!indexSettings.isAssignedOnRemoteNode()) {
throw new IllegalStateException("Index is not assigned on Remote Node");

Check warning on line 1634 in server/src/main/java/org/opensearch/index/shard/IndexShard.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/shard/IndexShard.java#L1634

Added line #L1634 was not covered by tests
}
RemoteSegmentMetadata lastUploadedMetadata = getRemoteDirectory().readLatestMetadataFile();
if (lastUploadedMetadata == null) {
throw new IllegalStateException("No metadata file found in remote store");
throw new FileNotFoundException("No metadata file found in remote store");

Check warning on line 1638 in server/src/main/java/org/opensearch/index/shard/IndexShard.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/shard/IndexShard.java#L1638

Added line #L1638 was not covered by tests
}
final Map<String, Long> indexFilesToFileLengthMap = lastUploadedMetadata.getMetadata()
.entrySet()
.stream()
.collect(Collectors.toMap(Map.Entry::getKey, entry -> entry.getValue().getLength()));
long primaryTerm = lastUploadedMetadata.getPrimaryTerm();
long commitGeneration = lastUploadedMetadata.getGeneration();
return new Tuple<>(new Tuple<>(primaryTerm, commitGeneration), indexFilesToFileLengthMap);
return lastUploadedMetadata;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ default void snapshotRemoteStoreIndexShard(
* Adds a reference of remote store data for a index commit point.
* <p>
* The index commit point can be obtained by using {@link org.opensearch.index.engine.Engine#acquireLastIndexCommit} method.
* Or for closed index can be obtained by reading last remote uploaded metadata by using {@link org.opensearch.index.shard.IndexShard#acquireLastRemoteUploadedIndexCommit} method.
* Or for closed index can be obtained by reading last remote uploaded metadata by using {@link org.opensearch.index.shard.IndexShard#fetchLastRemoteUploadedSegmentMetadata()} method.
* Repository implementations shouldn't release the snapshot index commit point. It is done by the method caller.
* <p>
* As snapshot process progresses, implementation of this method should update {@link IndexShardSnapshotStatus} object and check
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3744,6 +3744,33 @@ private void writeAtomic(BlobContainer container, final String blobName, final B
}
}

@Override
public void snapshotRemoteStoreIndexShard(
Store store,
SnapshotId snapshotId,
IndexId indexId,
IndexCommit snapshotIndexCommit,
@Nullable String shardStateIdentifier,
IndexShardSnapshotStatus snapshotStatus,
long primaryTerm,
long startTime,
ActionListener<String> listener
) {
snapshotRemoteStoreIndexShard(

Check warning on line 3759 in server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java#L3759

Added line #L3759 was not covered by tests
store,
snapshotId,
indexId,
snapshotIndexCommit,
shardStateIdentifier,
snapshotStatus,
primaryTerm,
snapshotIndexCommit.getGeneration(),

Check warning on line 3767 in server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java#L3767

Added line #L3767 was not covered by tests
startTime,
null,
listener
);
}

Check warning on line 3772 in server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java#L3772

Added line #L3772 was not covered by tests

@Override
public void snapshotRemoteStoreIndexShard(
Store store,
Expand All @@ -3762,10 +3789,6 @@ public void snapshotRemoteStoreIndexShard(
listener.onFailure(new RepositoryException(metadata.name(), "cannot snapshot shard on a readonly repository"));
return;
}
if (snapshotIndexCommit == null && indexFilesToFileLengthMap == null) {
listener.onFailure(new RepositoryException(metadata.name(), "both snapshot index commit and index files map cannot be null"));
return;
}

final ShardId shardId = store.shardId();
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
import org.opensearch.cluster.node.DiscoveryNode;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.Nullable;
import org.opensearch.common.collect.Tuple;
import org.opensearch.common.concurrent.GatedCloseable;
import org.opensearch.common.lifecycle.AbstractLifecycleComponent;
import org.opensearch.common.settings.Settings;
Expand All @@ -65,6 +64,7 @@
import org.opensearch.index.shard.IndexShardState;
import org.opensearch.index.snapshots.IndexShardSnapshotStatus;
import org.opensearch.index.snapshots.IndexShardSnapshotStatus.Stage;
import org.opensearch.index.store.remote.metadata.RemoteSegmentMetadata;
import org.opensearch.indices.IndicesService;
import org.opensearch.repositories.IndexId;
import org.opensearch.repositories.RepositoriesService;
Expand Down Expand Up @@ -407,10 +407,13 @@ private void snapshot(

try {
if (closedIndex) {
final Tuple<Tuple<Long, Long>, Map<String, Long>> tuple = indexShard.acquireLastRemoteUploadedIndexCommit();
primaryTerm = tuple.v1().v1();
commitGeneration = tuple.v1().v2();
indexFilesToFileLengthMap = tuple.v2();
RemoteSegmentMetadata lastRemoteUploadedIndexCommit = indexShard.fetchLastRemoteUploadedSegmentMetadata();
indexFilesToFileLengthMap = lastRemoteUploadedIndexCommit.getMetadata()
.entrySet()
.stream()
.collect(Collectors.toMap(Map.Entry::getKey, entry -> entry.getValue().getLength()));
primaryTerm = lastRemoteUploadedIndexCommit.getPrimaryTerm();
commitGeneration = lastRemoteUploadedIndexCommit.getGeneration();
} else {
wrappedSnapshot = indexShard.acquireLastIndexCommitAndRefresh(true);
snapshotIndexCommit = wrappedSnapshot.get();
Expand All @@ -420,7 +423,7 @@ private void snapshot(
} catch (IOException e) {

Check warning on line 423 in server/src/main/java/org/opensearch/snapshots/SnapshotShardsService.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/snapshots/SnapshotShardsService.java#L423

Added line #L423 was not covered by tests
if (closedIndex) {
logger.warn("Exception while reading latest metadata file from remote store");
throw e;
listener.onFailure(e);

Check warning on line 426 in server/src/main/java/org/opensearch/snapshots/SnapshotShardsService.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/snapshots/SnapshotShardsService.java#L425-L426

Added lines #L425 - L426 were not covered by tests
} else {
wrappedSnapshot.close();
logger.warn(

Check warning on line 429 in server/src/main/java/org/opensearch/snapshots/SnapshotShardsService.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/snapshots/SnapshotShardsService.java#L428-L429

Added lines #L428 - L429 were not covered by tests
Expand Down

0 comments on commit 3947531

Please sign in to comment.