Skip to content

Commit e612a2a

Browse files
aaimeaaime
committed
Avoid infinite number of calls to listBlobs when doing prefix removals (e..g, gridset or layer removals)
1 parent 70b38c6 commit e612a2a

File tree

5 files changed

+51
-28
lines changed

5 files changed

+51
-28
lines changed

‎geowebcache/azureblob/src/main/java/org/geowebcache/azure/AzureBlobStore.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public class AzureBlobStore implements BlobStore {
6767
private final TMSKeyBuilder keyBuilder;
6868
private final BlobStoreListenerList listeners = new BlobStoreListenerList();
6969
private final AzureClient client;
70-
private DeleteManager deleteManager;
70+
DeleteManager deleteManager;
7171

7272
private volatile boolean shutDown = false;
7373

@@ -200,7 +200,7 @@ public boolean delete(TileObject obj) throws StorageException {
200200
@Override
201201
public boolean delete(TileRange tileRange) throws StorageException {
202202
// see if there is anything to delete in that range by computing a prefix
203-
final String coordsPrefix = keyBuilder.coordinatesPrefix(tileRange, true);
203+
final String coordsPrefix = keyBuilder.coordinatesPrefix(tileRange, false);
204204
if (client.listBlobs(coordsPrefix, 1).isEmpty()) {
205205
return false;
206206
}

‎geowebcache/azureblob/src/main/java/org/geowebcache/azure/AzureClient.java‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -94,14 +94,17 @@ public AzureClient(AzureBlobStoreData configuration) throws StorageException {
9494
// no way to see if the containerURL already exists, try to create and see if
9595
// we get a 409 CONFLICT
9696
try {
97-
int status = this.container.create(null, null, null).blockingGet().statusCode();
98-
if (!HttpStatus.valueOf(status).is2xxSuccessful()
99-
&& status != HttpStatus.CONFLICT.value()) {
100-
throw new StorageException(
101-
"Failed to create container "
102-
+ containerName
103-
+ ", REST API returned a "
104-
+ status);
97+
int status = this.container.getProperties().blockingGet().statusCode();
98+
if (status == HttpStatus.NOT_FOUND.value()) {
99+
status = this.container.create(null, null, null).blockingGet().statusCode();
100+
if (!HttpStatus.valueOf(status).is2xxSuccessful()
101+
&& status != HttpStatus.CONFLICT.value()) {
102+
throw new StorageException(
103+
"Failed to create container "
104+
+ containerName
105+
+ ", REST API returned a "
106+
+ status);
107+
}
105108
}
106109
} catch (RestException e) {
107110
if (e.response().statusCode() != HttpStatus.CONFLICT.value()) {

‎geowebcache/azureblob/src/main/java/org/geowebcache/azure/DeleteManager.java‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import static org.geowebcache.azure.AzureBlobStore.log;
1818
import static org.springframework.http.HttpStatus.NOT_FOUND;
1919

20+
import com.google.common.base.Strings;
2021
import com.google.common.util.concurrent.ThreadFactoryBuilder;
2122
import com.microsoft.azure.storage.blob.ContainerURL;
2223
import com.microsoft.azure.storage.blob.ListBlobsOptions;
@@ -26,9 +27,11 @@
2627
import com.microsoft.rest.v2.RestException;
2728
import java.io.Closeable;
2829
import java.util.ArrayList;
30+
import java.util.HashSet;
2931
import java.util.List;
3032
import java.util.Map;
3133
import java.util.Properties;
34+
import java.util.Set;
3235
import java.util.concurrent.Callable;
3336
import java.util.concurrent.ConcurrentHashMap;
3437
import java.util.concurrent.ExecutionException;
@@ -169,14 +172,21 @@ public void issuePendingBulkDeletes() throws StorageException {
169172

170173
try {
171174
Properties deletes = client.getProperties(pendingDeletesKey);
175+
Set<String> deletesToClear = new HashSet<>();
172176
for (Map.Entry<Object, Object> e : deletes.entrySet()) {
173177
final String prefix = e.getKey().toString();
174178
final long timestamp = Long.parseLong(e.getValue().toString());
175179
log.info(
176180
String.format(
177181
"Restarting pending bulk delete on '%s/%s':%d",
178182
client.getContainerName(), prefix, timestamp));
179-
asyncDelete(prefix, timestamp);
183+
if (!asyncDelete(prefix, timestamp)) {
184+
deletesToClear.add(prefix);
185+
}
186+
}
187+
if (!deletesToClear.isEmpty()) {
188+
deletes.keySet().removeAll(deletesToClear);
189+
client.putProperties(pendingDeletesKey, deletes);
180190
}
181191
} finally {
182192
try {
@@ -281,12 +291,10 @@ public Long call() throws Exception {
281291
checkInterrupted();
282292
deleteItems(container, response.body().segment(), filter);
283293
String marker = response.body().nextMarker();
284-
if (marker != null) {
285-
response =
286-
container.listBlobsFlatSegment(marker, options, null).blockingGet();
287-
} else {
288-
break;
289-
}
294+
// marker will be empty if there is no next page
295+
if (Strings.isNullOrEmpty(marker)) break;
296+
// fetch next page
297+
response = container.listBlobsFlatSegment(marker, options, null).blockingGet();
290298
}
291299
} catch (InterruptedException | IllegalStateException e) {
292300
log.info(

‎geowebcache/azureblob/src/test/java/org/geowebcache/azure/AbstractAzureBlobStoreIntegrationTest.java‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,12 @@
1919
import static org.junit.Assert.assertNotNull;
2020
import static org.junit.Assert.assertNull;
2121
import static org.junit.Assert.assertTrue;
22+
import static org.mockito.AdditionalMatchers.or;
2223
import static org.mockito.ArgumentMatchers.anyInt;
2324
import static org.mockito.ArgumentMatchers.anyLong;
2425
import static org.mockito.ArgumentMatchers.anyString;
2526
import static org.mockito.ArgumentMatchers.eq;
27+
import static org.mockito.ArgumentMatchers.isNull;
2628
import static org.mockito.Mockito.mock;
2729
import static org.mockito.Mockito.times;
2830
import static org.mockito.Mockito.verify;
@@ -36,6 +38,7 @@
3638
import java.util.Arrays;
3739
import java.util.Collections;
3840
import java.util.HashMap;
41+
import java.util.List;
3942
import java.util.Map;
4043
import java.util.logging.Logger;
4144
import org.geotools.util.logging.Logging;
@@ -154,7 +157,7 @@ public void testPutWithListener() throws MimeException, StorageException {
154157
eq(tile.getLayerName()),
155158
eq(tile.getGridSetId()),
156159
eq(tile.getBlobFormat()),
157-
anyString(),
160+
anyStringOrNull(),
158161
eq(20L),
159162
eq(30L),
160163
eq(12),
@@ -171,7 +174,7 @@ public void testPutWithListener() throws MimeException, StorageException {
171174
eq(tile.getLayerName()),
172175
eq(tile.getGridSetId()),
173176
eq(tile.getBlobFormat()),
174-
anyString(),
177+
anyStringOrNull(),
175178
eq(20L),
176179
eq(30L),
177180
eq(12),
@@ -212,7 +215,7 @@ public void testDelete() throws MimeException, StorageException {
212215
eq(tile.getLayerName()),
213216
eq(tile.getGridSetId()),
214217
eq(tile.getBlobFormat()),
215-
anyString(),
218+
anyStringOrNull(),
216219
eq(22L),
217220
eq(30L),
218221
eq(12),
@@ -397,7 +400,7 @@ public void testTruncateShortCutsIfNoTilesInParametersPrefix()
397400
anyString(),
398401
anyString(),
399402
anyString(),
400-
anyString(),
403+
anyStringOrNull(),
401404
anyLong(),
402405
anyLong(),
403406
anyInt(),
@@ -493,21 +496,26 @@ public void testTruncateRespectsLevels() throws StorageException, MimeException
493496

494497
verify(listener, times(expectedCount))
495498
.tileDeleted(
496-
anyString(),
497-
anyString(),
498-
anyString(),
499-
anyString(),
499+
anyStringOrNull(),
500+
anyStringOrNull(),
501+
anyStringOrNull(),
502+
anyStringOrNull(),
500503
anyLong(),
501504
anyLong(),
502505
anyInt(),
503506
anyLong());
504507
}
505508

509+
private static String anyStringOrNull() {
510+
return or(isNull(), anyString());
511+
}
512+
506513
/**
507514
* If there are not {@link BlobStoreListener}s, use an optimized code path (not calling delete()
508515
* for each tile)
509516
*/
510517
@Test
518+
@SuppressWarnings("unchecked")
511519
public void testTruncateOptimizationIfNoListeners() throws StorageException, MimeException {
512520

513521
final int zoomStart = 0;
@@ -537,10 +545,12 @@ public void testTruncateOptimizationIfNoListeners() throws StorageException, Mim
537545
mimeType,
538546
parameters);
539547

540-
blobStore = Mockito.spy(blobStore);
548+
@SuppressWarnings("PMD.CloseResource") // closed by the store
549+
DeleteManager deleteManager = Mockito.spy(blobStore.deleteManager);
541550
assertTrue(blobStore.delete(tileRange));
542551

543-
verify(blobStore, times(0)).delete(Mockito.any(TileObject.class));
552+
verify(deleteManager, times(0)).executeParallel(Mockito.any(List.class));
553+
544554
assertFalse(blobStore.get(queryTile(0, 0, 0)));
545555
assertFalse(blobStore.get(queryTile(0, 0, 1)));
546556
assertFalse(blobStore.get(queryTile(0, 1, 1)));

‎geowebcache/core/src/main/java/org/geowebcache/util/TMSKeyBuilder.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ public final class TMSKeyBuilder {
3939
public static final String LAYER_METADATA_OBJECT_NAME = "metadata.properties";
4040
public static final String PARAMETERS_METADATA_OBJECT_PREFIX = "parameters-";
4141
public static final String PARAMETERS_METADATA_OBJECT_SUFFIX = ".properties";
42+
public static final String PENDING_DELETES = "_pending_deletes.properties";
4243

4344
private String prefix;
4445

@@ -220,7 +221,8 @@ public String coordinatesPrefix(TileRange obj, boolean endWithSlash) {
220221
}
221222

222223
public String pendingDeletes() {
223-
return String.format("%s/%s", prefix, "_pending_deletes.properties");
224+
if (!Strings.isNullOrEmpty(prefix)) return String.format("%s/%s", prefix, PENDING_DELETES);
225+
else return PENDING_DELETES;
224226
}
225227

226228
private static String join(boolean closing, Object... elements) {

0 commit comments

Comments
 (0)