Skip to content

Commit 4f59da3

Browse files
authored
Make GCP Objectstore delete requests retryable (#1768)
Make GCP Objectstore delete requests retryable
1 parent ef849d9 commit 4f59da3

File tree

2 files changed

+20
-15
lines changed

2 files changed

+20
-15
lines changed

multiapps-controller-persistence/src/main/java/org/cloudfoundry/multiapps/controller/persistence/services/GcpObjectStoreFileStorage.java

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public GcpObjectStoreFileStorage(Map<String, Object> credentials) {
5252
protected Storage createObjectStoreStorage(Map<String, Object> credentials) {
5353
return StorageOptions.http()
5454
.setCredentials(getGcpCredentialsSupplier(credentials))
55-
.setStorageRetryStrategy(StorageRetryStrategy.getDefaultStorageRetryStrategy())
55+
.setStorageRetryStrategy(StorageRetryStrategy.getUniformStorageRetryStrategy())
5656
.setRetrySettings(
5757
RetrySettings.newBuilder()
5858
.setMaxAttempts(OBJECT_STORE_MAX_ATTEMPTS_CONFIG)
@@ -110,7 +110,19 @@ public List<FileEntry> getFileEntriesWithoutContent(List<FileEntry> fileEntries)
110110

111111
@Override
112112
public void deleteFile(String id, String space) {
113-
storage.delete(bucketName, id);
113+
deleteFileWithGeneration(id);
114+
}
115+
116+
private boolean deleteFileWithGeneration(String id) {
117+
Blob blob = storage.get(bucketName, id);
118+
if (blob == null) {
119+
return false;
120+
}
121+
if (blob.getGeneration() == null) {
122+
return storage.delete(bucketName, id);
123+
}
124+
//Without generationMatch the delete requests are not retried because a retry can "accidentally delete a newer object version"
125+
return storage.delete(bucketName, id, Storage.BlobSourceOption.generationMatch(blob.getGeneration()));
114126
}
115127

116128
@Override
@@ -212,8 +224,10 @@ private int removeBlobsByFilter(Predicate<? super Blob> filter) {
212224
return deletedBlobsResults.size();
213225
}
214226

215-
protected List<Boolean> deleteBlobs(List<BlobId> blobIds) {
216-
return storage.delete(blobIds);
227+
private List<Boolean> deleteBlobs(List<BlobId> blobIds) {
228+
return blobIds.stream()
229+
.map(blobId -> deleteFileWithGeneration(blobId.getName()))
230+
.toList();
217231
}
218232

219233
protected Set<String> getEntryNames(Predicate<? super Blob> filter) {

multiapps-controller-persistence/src/test/java/org/cloudfoundry/multiapps/controller/persistence/services/GcpObjectStoreFileStorageTest.java

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,9 @@
33
import java.nio.file.Files;
44
import java.nio.file.Path;
55
import java.nio.file.Paths;
6-
import java.util.ArrayList;
7-
import java.util.List;
86
import java.util.Map;
97
import java.util.UUID;
8+
109
import com.google.cloud.storage.Blob;
1110
import com.google.cloud.storage.BlobId;
1211
import com.google.cloud.storage.BlobInfo;
@@ -16,6 +15,7 @@
1615
import org.junit.jupiter.api.AfterEach;
1716
import org.junit.jupiter.api.BeforeEach;
1817
import org.springframework.http.MediaType;
18+
1919
import static org.junit.jupiter.api.Assertions.assertEquals;
2020
import static org.junit.jupiter.api.Assertions.assertNull;
2121

@@ -34,15 +34,6 @@ public void setUp() {
3434
protected Storage createObjectStoreStorage(Map<String, Object> credentials) {
3535
return storage;
3636
}
37-
38-
@Override
39-
protected List<Boolean> deleteBlobs(List<BlobId> blobIds) {
40-
List<Boolean> deletedBlobsResults = new ArrayList<>();
41-
for (BlobId blobId : blobIds) {
42-
deletedBlobsResults.add(storage.delete(blobId));
43-
}
44-
return List.copyOf(deletedBlobsResults);
45-
}
4637
};
4738
spaceId = UUID.randomUUID()
4839
.toString();

0 commit comments

Comments
 (0)