Skip to content

Commit 2b4e7a1

Browse files
tlrxtlrx
committed
Use AmazonS3.doesObjectExist() method in S3BlobContainer (#27723)
This pull request changes the S3BlobContainer.blobExists() method implementation to make it use the AmazonS3.doesObjectExist() method instead of AmazonS3.getObjectMetadata(). The AmazonS3 implementation takes care of catching any thrown AmazonS3Exception and compares its response code with 404, returning false (object does not exist) or lets the exception be propagated.
1 parent e767662 commit 2b4e7a1

File tree

5 files changed

+58
-71
lines changed

5 files changed

+58
-71
lines changed

‎core/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java‎

Lines changed: 19 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -368,9 +368,9 @@ public void deleteSnapshot(SnapshotId snapshotId, long repositoryStateId) {
368368
writeIndexGen(updatedRepositoryData, repositoryStateId);
369369

370370
// delete the snapshot file
371-
safeSnapshotBlobDelete(snapshot, snapshotId.getUUID());
371+
deleteSnapshotBlobIgnoringErrors(snapshot, snapshotId.getUUID());
372372
// delete the global metadata file
373-
safeGlobalMetaDataBlobDelete(snapshot, snapshotId.getUUID());
373+
deleteGlobalMetaDataBlobIgnoringErrors(snapshot, snapshotId.getUUID());
374374

375375
// Now delete all indices
376376
for (String index : indices) {
@@ -422,37 +422,27 @@ public void deleteSnapshot(SnapshotId snapshotId, long repositoryStateId) {
422422
}
423423
}
424424

425-
private void safeSnapshotBlobDelete(final SnapshotInfo snapshotInfo, final String blobId) {
426-
if (snapshotInfo != null) {
427-
// we know the version the snapshot was created with
428-
try {
429-
snapshotFormat.delete(snapshotsBlobContainer, blobId);
430-
} catch (IOException e) {
431-
logger.warn((Supplier<?>) () -> new ParameterizedMessage("[{}] Unable to delete snapshot file [{}]", snapshotInfo.snapshotId(), blobId), e);
432-
}
433-
} else {
434-
try {
435-
snapshotFormat.delete(snapshotsBlobContainer, blobId);
436-
} catch (IOException e) {
437-
// snapshot file could not be deleted, log the error
425+
private void deleteSnapshotBlobIgnoringErrors(final SnapshotInfo snapshotInfo, final String blobId) {
426+
try {
427+
snapshotFormat.delete(snapshotsBlobContainer, blobId);
428+
} catch (IOException e) {
429+
if (snapshotInfo != null) {
430+
logger.warn((Supplier<?>) () -> new ParameterizedMessage("[{}] Unable to delete snapshot file [{}]",
431+
snapshotInfo.snapshotId(), blobId), e);
432+
} else {
438433
logger.warn((Supplier<?>) () -> new ParameterizedMessage("Unable to delete snapshot file [{}]", blobId), e);
439434
}
440435
}
441436
}
442437

443-
private void safeGlobalMetaDataBlobDelete(final SnapshotInfo snapshotInfo, final String blobId) {
444-
if (snapshotInfo != null) {
445-
// we know the version the snapshot was created with
446-
try {
447-
globalMetaDataFormat.delete(snapshotsBlobContainer, blobId);
448-
} catch (IOException e) {
449-
logger.warn((Supplier<?>) () -> new ParameterizedMessage("[{}] Unable to delete global metadata file [{}]", snapshotInfo.snapshotId(), blobId), e);
450-
}
451-
} else {
452-
try {
453-
globalMetaDataFormat.delete(snapshotsBlobContainer, blobId);
454-
} catch (IOException e) {
455-
// global metadata file could not be deleted, log the error
438+
private void deleteGlobalMetaDataBlobIgnoringErrors(final SnapshotInfo snapshotInfo, final String blobId) {
439+
try {
440+
globalMetaDataFormat.delete(snapshotsBlobContainer, blobId);
441+
} catch (IOException e) {
442+
if (snapshotInfo != null) {
443+
logger.warn((Supplier<?>) () -> new ParameterizedMessage("[{}] Unable to delete global metadata file [{}]",
444+
snapshotInfo.snapshotId(), blobId), e);
445+
} else {
456446
logger.warn((Supplier<?>) () -> new ParameterizedMessage("Unable to delete global metadata file [{}]", blobId), e);
457447
}
458448
}
@@ -512,9 +502,7 @@ private MetaData readSnapshotMetaData(SnapshotId snapshotId, Version snapshotVer
512502
// When we delete corrupted snapshots we might not know which version we are dealing with
513503
// We can try detecting the version based on the metadata file format
514504
assert ignoreIndexErrors;
515-
if (globalMetaDataFormat.exists(snapshotsBlobContainer, snapshotId.getUUID())) {
516-
snapshotVersion = Version.CURRENT;
517-
} else {
505+
if (globalMetaDataFormat.exists(snapshotsBlobContainer, snapshotId.getUUID()) == false) {
518506
throw new SnapshotMissingException(metadata.name(), snapshotId);
519507
}
520508
}

‎core/src/main/java/org/elasticsearch/snapshots/SnapshotShardsService.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -602,7 +602,6 @@ public ClusterTasksResult<UpdateIndexShardSnapshotStatusRequest> execute(Cluster
602602
entries.add(updatedEntry);
603603
// Finalize snapshot in the repository
604604
snapshotsService.endSnapshot(updatedEntry);
605-
logger.info("snapshot [{}] is done", updatedEntry.snapshot());
606605
}
607606
} else {
608607
entries.add(entry);

‎core/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java‎

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ public ClusterState execute(ClusterState currentState) {
263263
null);
264264
snapshots = new SnapshotsInProgress(newSnapshot);
265265
} else {
266-
throw new ConcurrentSnapshotExecutionException(repositoryName, snapshotName, "a snapshot is already running");
266+
throw new ConcurrentSnapshotExecutionException(repositoryName, snapshotName, " a snapshot is already running");
267267
}
268268
return ClusterState.builder(currentState).putCustom(SnapshotsInProgress.TYPE, snapshots).build();
269269
}
@@ -363,6 +363,8 @@ private void beginSnapshot(final ClusterState clusterState,
363363

364364
repository.initializeSnapshot(snapshot.snapshot().getSnapshotId(), snapshot.indices(), metaData);
365365
snapshotCreated = true;
366+
367+
logger.info("snapshot [{}] started", snapshot.snapshot());
366368
if (snapshot.indices().isEmpty()) {
367369
// No indices in this snapshot - we are done
368370
userCreateSnapshotListener.onResponse();
@@ -947,35 +949,33 @@ void endSnapshot(SnapshotsInProgress.Entry entry) {
947949
* @param failure failure reason or null if snapshot was successful
948950
*/
949951
private void endSnapshot(final SnapshotsInProgress.Entry entry, final String failure) {
950-
threadPool.executor(ThreadPool.Names.SNAPSHOT).execute(new Runnable() {
951-
@Override
952-
public void run() {
953-
final Snapshot snapshot = entry.snapshot();
954-
try {
955-
final Repository repository = repositoriesService.repository(snapshot.getRepository());
956-
logger.trace("[{}] finalizing snapshot in repository, state: [{}], failure[{}]", snapshot, entry.state(), failure);
957-
ArrayList<SnapshotShardFailure> shardFailures = new ArrayList<>();
958-
for (ObjectObjectCursor<ShardId, ShardSnapshotStatus> shardStatus : entry.shards()) {
959-
ShardId shardId = shardStatus.key;
960-
ShardSnapshotStatus status = shardStatus.value;
961-
if (status.state().failed()) {
962-
shardFailures.add(new SnapshotShardFailure(status.nodeId(), shardId, status.reason()));
963-
}
952+
threadPool.executor(ThreadPool.Names.SNAPSHOT).execute(() -> {
953+
final Snapshot snapshot = entry.snapshot();
954+
try {
955+
final Repository repository = repositoriesService.repository(snapshot.getRepository());
956+
logger.trace("[{}] finalizing snapshot in repository, state: [{}], failure[{}]", snapshot, entry.state(), failure);
957+
ArrayList<SnapshotShardFailure> shardFailures = new ArrayList<>();
958+
for (ObjectObjectCursor<ShardId, ShardSnapshotStatus> shardStatus : entry.shards()) {
959+
ShardId shardId = shardStatus.key;
960+
ShardSnapshotStatus status = shardStatus.value;
961+
if (status.state().failed()) {
962+
shardFailures.add(new SnapshotShardFailure(status.nodeId(), shardId, status.reason()));
964963
}
965-
SnapshotInfo snapshotInfo = repository.finalizeSnapshot(
966-
snapshot.getSnapshotId(),
967-
entry.indices(),
968-
entry.startTime(),
969-
failure,
970-
entry.shards().size(),
971-
Collections.unmodifiableList(shardFailures),
972-
entry.getRepositoryStateId(),
973-
entry.includeGlobalState());
974-
removeSnapshotFromClusterState(snapshot, snapshotInfo, null);
975-
} catch (Exception e) {
976-
logger.warn((Supplier<?>) () -> new ParameterizedMessage("[{}] failed to finalize snapshot", snapshot), e);
977-
removeSnapshotFromClusterState(snapshot, null, e);
978964
}
965+
SnapshotInfo snapshotInfo = repository.finalizeSnapshot(
966+
snapshot.getSnapshotId(),
967+
entry.indices(),
968+
entry.startTime(),
969+
failure,
970+
entry.shards().size(),
971+
Collections.unmodifiableList(shardFailures),
972+
entry.getRepositoryStateId(),
973+
entry.includeGlobalState());
974+
removeSnapshotFromClusterState(snapshot, snapshotInfo, null);
975+
logger.info("snapshot [{}] completed with state [{}]", snapshot, snapshotInfo.state());
976+
} catch (Exception e) {
977+
logger.warn((Supplier<?>) () -> new ParameterizedMessage("[{}] failed to finalize snapshot", snapshot), e);
978+
removeSnapshotFromClusterState(snapshot, null, e);
979979
}
980980
});
981981
}

‎plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -73,14 +73,9 @@ class S3BlobContainer extends AbstractBlobContainer {
7373
@Override
7474
public boolean blobExists(String blobName) {
7575
try {
76-
return SocketAccess.doPrivileged(() -> {
77-
blobStore.client().getObjectMetadata(blobStore.bucket(), buildKey(blobName));
78-
return true;
79-
});
80-
} catch (AmazonS3Exception e) {
81-
return false;
76+
return SocketAccess.doPrivileged(() -> blobStore.client().doesObjectExist(blobStore.bucket(), buildKey(blobName)));
8277
} catch (Exception e) {
83-
throw new BlobStoreException("failed to check if blob exists", e);
78+
throw new BlobStoreException("Failed to check if blob [" + blobName +"] exists", e);
8479
}
8580
}
8681

@@ -102,7 +97,7 @@ public InputStream readBlob(String blobName) throws IOException {
10297
@Override
10398
public void writeBlob(String blobName, InputStream inputStream, long blobSize) throws IOException {
10499
if (blobExists(blobName)) {
105-
throw new FileAlreadyExistsException("blob [" + blobName + "] already exists, cannot overwrite");
100+
throw new FileAlreadyExistsException("Blob [" + blobName + "] already exists, cannot overwrite");
106101
}
107102

108103
SocketAccess.doPrivilegedIOException(() -> {
@@ -117,7 +112,7 @@ public void writeBlob(String blobName, InputStream inputStream, long blobSize) t
117112

118113
@Override
119114
public void deleteBlob(String blobName) throws IOException {
120-
if (!blobExists(blobName)) {
115+
if (blobExists(blobName) == false) {
121116
throw new NoSuchFileException("Blob [" + blobName + "] does not exist");
122117
}
123118

‎plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/MockAmazonS3.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import com.amazonaws.AmazonClientException;
2323
import com.amazonaws.AmazonServiceException;
24+
import com.amazonaws.SdkClientException;
2425
import com.amazonaws.services.s3.AbstractAmazonS3;
2526
import com.amazonaws.services.s3.model.AmazonS3Exception;
2627
import com.amazonaws.services.s3.model.CopyObjectRequest;
@@ -36,14 +37,12 @@
3637
import com.amazonaws.services.s3.model.S3Object;
3738
import com.amazonaws.services.s3.model.S3ObjectInputStream;
3839
import com.amazonaws.services.s3.model.S3ObjectSummary;
39-
import com.amazonaws.util.Base64;
4040

4141
import java.io.IOException;
4242
import java.io.InputStream;
4343
import java.io.UncheckedIOException;
4444
import java.net.InetAddress;
4545
import java.net.Socket;
46-
import java.security.DigestInputStream;
4746
import java.util.ArrayList;
4847
import java.util.List;
4948
import java.util.Map;
@@ -88,6 +87,12 @@ public boolean doesBucketExist(String bucket) {
8887
return true;
8988
}
9089

90+
@Override
91+
public boolean doesObjectExist(String bucketName, String objectName) throws AmazonServiceException, SdkClientException {
92+
simulateS3SocketConnection();
93+
return blobs.containsKey(objectName);
94+
}
95+
9196
@Override
9297
public ObjectMetadata getObjectMetadata(
9398
GetObjectMetadataRequest getObjectMetadataRequest)

0 commit comments

Comments
 (0)