From 0036973aaad2baf4f61f47d82226c8153f8ad7ef Mon Sep 17 00:00:00 2001
From: Andrea Aime <andrea.aime@gmail.com>
Date: Thu, 18 Apr 2024 13:03:36 +0200
Subject: [PATCH] Avoid infinite number of calls to listBlobs when doing prefix
removals (e..g, gridset or layer removals)
---
.../org/geowebcache/azure/AzureBlobStore.java | 4 +--
.../org/geowebcache/azure/AzureClient.java | 19 +++++++-----
.../org/geowebcache/azure/DeleteManager.java | 22 +++++++++-----
...AbstractAzureBlobStoreIntegrationTest.java | 30 ++++++++++++-------
4 files changed, 48 insertions(+), 27 deletions(-)
diff --git a/geowebcache/azureblob/src/main/java/org/geowebcache/azure/AzureBlobStore.java b/geowebcache/azureblob/src/main/java/org/geowebcache/azure/AzureBlobStore.java
index 096be5d7eb..3a9eada756 100644
--- a/geowebcache/azureblob/src/main/java/org/geowebcache/azure/AzureBlobStore.java
+++ b/geowebcache/azureblob/src/main/java/org/geowebcache/azure/AzureBlobStore.java
@@ -67,7 +67,7 @@ public class AzureBlobStore implements BlobStore {
private final TMSKeyBuilder keyBuilder;
private final BlobStoreListenerList listeners = new BlobStoreListenerList();
private final AzureClient client;
- private DeleteManager deleteManager;
+ DeleteManager deleteManager;
private volatile boolean shutDown = false;
@@ -200,7 +200,7 @@ public boolean delete(TileObject obj) throws StorageException {
@Override
public boolean delete(TileRange tileRange) throws StorageException {
// see if there is anything to delete in that range by computing a prefix
- final String coordsPrefix = keyBuilder.coordinatesPrefix(tileRange, true);
+ final String coordsPrefix = keyBuilder.coordinatesPrefix(tileRange, false);
if (client.listBlobs(coordsPrefix, 1).isEmpty()) {
return false;
}
diff --git a/geowebcache/azureblob/src/main/java/org/geowebcache/azure/AzureClient.java b/geowebcache/azureblob/src/main/java/org/geowebcache/azure/AzureClient.java
index 2aa9726e53..feaa447c4f 100644
--- a/geowebcache/azureblob/src/main/java/org/geowebcache/azure/AzureClient.java
+++ b/geowebcache/azureblob/src/main/java/org/geowebcache/azure/AzureClient.java
@@ -94,14 +94,17 @@ public AzureClient(AzureBlobStoreData configuration) throws StorageException {
// no way to see if the containerURL already exists, try to create and see if
// we get a 409 CONFLICT
try {
- int status = this.container.create(null, null, null).blockingGet().statusCode();
- if (!HttpStatus.valueOf(status).is2xxSuccessful()
- && status != HttpStatus.CONFLICT.value()) {
- throw new StorageException(
- "Failed to create container "
- + containerName
- + ", REST API returned a "
- + status);
+ int status = this.container.getProperties().blockingGet().statusCode();
+ if (status == HttpStatus.NOT_FOUND.value()) {
+ status = this.container.create(null, null, null).blockingGet().statusCode();
+ if (!HttpStatus.valueOf(status).is2xxSuccessful()
+ && status != HttpStatus.CONFLICT.value()) {
+ throw new StorageException(
+ "Failed to create container "
+ + containerName
+ + ", REST API returned a "
+ + status);
+ }
}
} catch (RestException e) {
if (e.response().statusCode() != HttpStatus.CONFLICT.value()) {
diff --git a/geowebcache/azureblob/src/main/java/org/geowebcache/azure/DeleteManager.java b/geowebcache/azureblob/src/main/java/org/geowebcache/azure/DeleteManager.java
index 2c8cd22133..aaa1e585e5 100644
--- a/geowebcache/azureblob/src/main/java/org/geowebcache/azure/DeleteManager.java
+++ b/geowebcache/azureblob/src/main/java/org/geowebcache/azure/DeleteManager.java
@@ -17,6 +17,7 @@
import static org.geowebcache.azure.AzureBlobStore.log;
import static org.springframework.http.HttpStatus.NOT_FOUND;
+import com.google.common.base.Strings;
import com.google.common.util.concurrent.ThreadFactoryBuilder;
import com.microsoft.azure.storage.blob.ContainerURL;
import com.microsoft.azure.storage.blob.ListBlobsOptions;
@@ -26,9 +27,11 @@
import com.microsoft.rest.v2.RestException;
import java.io.Closeable;
import java.util.ArrayList;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Properties;
+import java.util.Set;
import java.util.concurrent.Callable;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutionException;
@@ -169,6 +172,7 @@ public void issuePendingBulkDeletes() throws StorageException {
try {
Properties deletes = client.getProperties(pendingDeletesKey);
+ Set<String> deletesToClear = new HashSet<>();
for (Map.Entry<Object, Object> e : deletes.entrySet()) {
final String prefix = e.getKey().toString();
final long timestamp = Long.parseLong(e.getValue().toString());
@@ -176,7 +180,13 @@ public void issuePendingBulkDeletes() throws StorageException {
String.format(
"Restarting pending bulk delete on '%s/%s':%d",
client.getContainerName(), prefix, timestamp));
- asyncDelete(prefix, timestamp);
+ if (!asyncDelete(prefix, timestamp)) {
+ deletesToClear.add(prefix);
+ }
+ }
+ if (!deletesToClear.isEmpty()) {
+ deletes.keySet().removeAll(deletesToClear);
+ client.putProperties(pendingDeletesKey, deletes);
}
} finally {
try {
@@ -281,12 +291,10 @@ public Long call() throws Exception {
checkInterrupted();
deleteItems(container, response.body().segment(), filter);
String marker = response.body().nextMarker();
- if (marker != null) {
- response =
- container.listBlobsFlatSegment(marker, options, null).blockingGet();
- } else {
- break;
- }
+ // marker will be empty if there is no next page
+ if (Strings.isNullOrEmpty(marker)) break;
+ // fetch next page
+ response = container.listBlobsFlatSegment(marker, options, null).blockingGet();
}
} catch (InterruptedException | IllegalStateException e) {
log.info(
diff --git a/geowebcache/azureblob/src/test/java/org/geowebcache/azure/AbstractAzureBlobStoreIntegrationTest.java b/geowebcache/azureblob/src/test/java/org/geowebcache/azure/AbstractAzureBlobStoreIntegrationTest.java
index a06df3bcfd..9ddb3adedb 100644
--- a/geowebcache/azureblob/src/test/java/org/geowebcache/azure/AbstractAzureBlobStoreIntegrationTest.java
+++ b/geowebcache/azureblob/src/test/java/org/geowebcache/azure/AbstractAzureBlobStoreIntegrationTest.java
@@ -19,10 +19,12 @@
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
+import static org.mockito.AdditionalMatchers.or;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
@@ -36,6 +38,7 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
import java.util.logging.Logger;
import org.geotools.util.logging.Logging;
@@ -154,7 +157,7 @@ public void testPutWithListener() throws MimeException, StorageException {
eq(tile.getLayerName()),
eq(tile.getGridSetId()),
eq(tile.getBlobFormat()),
- anyString(),
+ anyStringOrNull(),
eq(20L),
eq(30L),
eq(12),
@@ -171,7 +174,7 @@ public void testPutWithListener() throws MimeException, StorageException {
eq(tile.getLayerName()),
eq(tile.getGridSetId()),
eq(tile.getBlobFormat()),
- anyString(),
+ anyStringOrNull(),
eq(20L),
eq(30L),
eq(12),
@@ -212,7 +215,7 @@ public void testDelete() throws MimeException, StorageException {
eq(tile.getLayerName()),
eq(tile.getGridSetId()),
eq(tile.getBlobFormat()),
- anyString(),
+ anyStringOrNull(),
eq(22L),
eq(30L),
eq(12),
@@ -397,7 +400,7 @@ public void testTruncateShortCutsIfNoTilesInParametersPrefix()
anyString(),
anyString(),
anyString(),
- anyString(),
+ anyStringOrNull(),
anyLong(),
anyLong(),
anyInt(),
@@ -493,21 +496,26 @@ public void testTruncateRespectsLevels() throws StorageException, MimeException
verify(listener, times(expectedCount))
.tileDeleted(
- anyString(),
- anyString(),
- anyString(),
- anyString(),
+ anyStringOrNull(),
+ anyStringOrNull(),
+ anyStringOrNull(),
+ anyStringOrNull(),
anyLong(),
anyLong(),
anyInt(),
anyLong());
}
+ private static String anyStringOrNull() {
+ return or(isNull(), anyString());
+ }
+
/**
* If there are not {@link BlobStoreListener}s, use an optimized code path (not calling delete()
* for each tile)
*/
@Test
+ @SuppressWarnings("unchecked")
public void testTruncateOptimizationIfNoListeners() throws StorageException, MimeException {
final int zoomStart = 0;
@@ -537,10 +545,12 @@ public void testTruncateOptimizationIfNoListeners() throws StorageException, Mim
mimeType,
parameters);
- blobStore = Mockito.spy(blobStore);
+ @SuppressWarnings("PMD.CloseResource") // closed by the store
+ DeleteManager deleteManager = Mockito.spy(blobStore.deleteManager);
assertTrue(blobStore.delete(tileRange));
- verify(blobStore, times(0)).delete(Mockito.any(TileObject.class));
+ verify(deleteManager, times(0)).executeParallel(Mockito.any(List.class));
+
assertFalse(blobStore.get(queryTile(0, 0, 0)));
assertFalse(blobStore.get(queryTile(0, 0, 1)));
assertFalse(blobStore.get(queryTile(0, 1, 1)));