Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not check for object existence when deleting repository index files #31680

Merged
merged 1 commit into from
Jun 29, 2018
Merged

Do not check for object existence when deleting repository index files #31680

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions ...tory-s3/qa/amazon-s3/src/test/java/org/elasticsearch/repositories/s3/AmazonS3Fixture.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,8 @@ private static PathTrie<RequestHandler> defaultHandlers(final Map<String, Bucket
}

final String objectName = objectName(request.getParameters());
if (bucket.objects.remove(objectName) != null) {
return new Response(RestStatus.OK.getStatus(), TEXT_PLAIN_CONTENT_TYPE, EMPTY_BYTE);
}
return newObjectNotFoundError(request.getId(), objectName);
bucket.objects.remove(objectName);
return new Response(RestStatus.NO_CONTENT.getStatus(), TEXT_PLAIN_CONTENT_TYPE, EMPTY_BYTE);
})
);

Expand Down
5 changes: 5 additions & 0 deletions plugins/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,13 @@ public void deleteBlob(String blobName) throws IOException {
if (blobExists(blobName) == false) {
throw new NoSuchFileException("Blob [" + blobName + "] does not exist");
}
deleteBlobIgnoringIfNotExists(blobName);
}

@Override
public void deleteBlobIgnoringIfNotExists(String blobName) throws IOException {
try (AmazonS3Reference clientReference = blobStore.clientReference()) {
// There is no way to know if an non-versioned object existed before the deletion
SocketAccess.doPrivilegedVoid(() -> clientReference.client().deleteObject(blobStore.bucket(), buildKey(blobName)));
} catch (final AmazonClientException e) {
throw new IOException("Exception when deleting blob [" + blobName + "]", e);
Expand Down
10 changes: 2 additions & 8 deletions plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/MockAmazonS3.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,15 +149,9 @@ public ObjectListing listObjects(final ListObjectsRequest request) throws Amazon
@Override
public void deleteObject(final DeleteObjectRequest request) throws AmazonClientException {
assertThat(request.getBucketName(), equalTo(bucket));

final String blobName = request.getKey();
if (blobs.remove(blobName) == null) {
AmazonS3Exception exception = new AmazonS3Exception("[" + blobName + "] does not exist.");
exception.setStatusCode(404);
throw exception;
}
blobs.remove(request.getKey());
}

@Override
public void shutdown() {
// TODO check close
Expand Down
13 changes: 0 additions & 13 deletions ...rc/main/java/org/elasticsearch/index/snapshots/blobstore/BlobStoreIndexShardSnapshot.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,19 +389,6 @@ public BlobStoreIndexShardSnapshot(String snapshot, long indexVersion, List<File
this.incrementalSize = incrementalSize;
}

/**
* Special constructor for the prototype
*/
private BlobStoreIndexShardSnapshot() {
this.snapshot = "";
this.indexVersion = 0;
this.indexFiles = Collections.emptyList();
this.startTime = 0;
this.time = 0;
this.incrementalFileCount = 0;
this.incrementalSize = 0;
}

/**
* Returns index version
*
Expand Down
12 changes: 3 additions & 9 deletions server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -668,9 +668,7 @@ protected void writeIndexGen(final RepositoryData repositoryData, final long rep
// delete the N-2 index file if it exists, keep the previous one around as a backup
if (isReadOnly() == false && newGen - 2 >= 0) {
final String oldSnapshotIndexFile = INDEX_FILE_PREFIX + Long.toString(newGen - 2);
if (snapshotsBlobContainer.blobExists(oldSnapshotIndexFile)) {
snapshotsBlobContainer.deleteBlob(oldSnapshotIndexFile);
}
snapshotsBlobContainer.deleteBlobIgnoringIfNotExists(oldSnapshotIndexFile);
}

// write the current generation to the index-latest file
Expand All @@ -679,9 +677,7 @@ protected void writeIndexGen(final RepositoryData repositoryData, final long rep
bStream.writeLong(newGen);
genBytes = bStream.bytes();
}
if (snapshotsBlobContainer.blobExists(INDEX_LATEST_BLOB)) {
snapshotsBlobContainer.deleteBlob(INDEX_LATEST_BLOB);
}
snapshotsBlobContainer.deleteBlobIgnoringIfNotExists(INDEX_LATEST_BLOB);
logger.debug("Repository [{}] updating index.latest with generation [{}]", metadata.name(), newGen);
writeAtomic(INDEX_LATEST_BLOB, genBytes);
}
Expand All @@ -702,9 +698,7 @@ void writeIncompatibleSnapshots(RepositoryData repositoryData) throws IOExceptio
}
bytes = bStream.bytes();
}
if (snapshotsBlobContainer.blobExists(INCOMPATIBLE_SNAPSHOTS_BLOB)) {
snapshotsBlobContainer.deleteBlob(INCOMPATIBLE_SNAPSHOTS_BLOB);
}
snapshotsBlobContainer.deleteBlobIgnoringIfNotExists(INCOMPATIBLE_SNAPSHOTS_BLOB);
// write the incompatible snapshots blob
writeAtomic(INCOMPATIBLE_SNAPSHOTS_BLOB, bytes);
}
Expand Down