Skip to content

Commit e9dbb7c

Browse files
mziccardmziccard
committed
Limit batch size (storage performs better if the batches are split)
1 parent 6ceb59f commit e9dbb7c

3 files changed

Lines changed: 62 additions & 26 deletions

File tree

‎gcloud-java-storage/src/main/java/com/google/cloud/storage/spi/DefaultStorageRpc.java‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -95,11 +95,15 @@ public DefaultStorageRpc(StorageOptions options) {
9595

9696
private class DefaultRpcBatch implements RpcBatch {
9797

98-
private static final int MAX_BATCH_DELETES = 100;
98+
// Batch size is limited as, due to some current service implementation details, the service
99+
// performs better if the batches are split for better distribution. See
100+
// https://github.com/GoogleCloudPlatform/gcloud-java/pull/952#issuecomment-213466772 for
101+
// background.
102+
private static final int MAX_BATCH_SIZE = 100;
99103

100104
private final Storage storage;
101105
private final LinkedList<BatchRequest> batches;
102-
private int deleteCount;
106+
private int currentBatchSize;
103107

104108
private DefaultRpcBatch(Storage storage) {
105109
this.storage = storage;
@@ -111,12 +115,12 @@ private DefaultRpcBatch(Storage storage) {
111115
public void addDelete(StorageObject storageObject, RpcBatch.Callback<Void> callback,
112116
Map<Option, ?> options) {
113117
try {
114-
if (deleteCount == MAX_BATCH_DELETES) {
118+
if (currentBatchSize == MAX_BATCH_SIZE) {
115119
batches.add(storage.batch());
116-
deleteCount = 0;
120+
currentBatchSize = 0;
117121
}
118122
deleteCall(storageObject, options).queue(batches.getLast(), toJsonCallback(callback));
119-
deleteCount++;
123+
currentBatchSize++;
120124
} catch (IOException ex) {
121125
throw translate(ex);
122126
}
@@ -126,7 +130,12 @@ public void addDelete(StorageObject storageObject, RpcBatch.Callback<Void> callb
126130
public void addPatch(StorageObject storageObject, RpcBatch.Callback<StorageObject> callback,
127131
Map<Option, ?> options) {
128132
try {
129-
patchCall(storageObject, options).queue(batches.getFirst(), toJsonCallback(callback));
133+
if (currentBatchSize == MAX_BATCH_SIZE) {
134+
batches.add(storage.batch());
135+
currentBatchSize = 0;
136+
}
137+
patchCall(storageObject, options).queue(batches.getLast(), toJsonCallback(callback));
138+
currentBatchSize++;
130139
} catch (IOException ex) {
131140
throw translate(ex);
132141
}
@@ -136,7 +145,12 @@ public void addPatch(StorageObject storageObject, RpcBatch.Callback<StorageObjec
136145
public void addGet(StorageObject storageObject, RpcBatch.Callback<StorageObject> callback,
137146
Map<Option, ?> options) {
138147
try {
139-
getCall(storageObject, options).queue(batches.getFirst(), toJsonCallback(callback));
148+
if (currentBatchSize == MAX_BATCH_SIZE) {
149+
batches.add(storage.batch());
150+
currentBatchSize = 0;
151+
}
152+
getCall(storageObject, options).queue(batches.getLast(), toJsonCallback(callback));
153+
currentBatchSize++;
140154
} catch (IOException ex) {
141155
throw translate(ex);
142156
}

‎gcloud-java-storage/src/test/java/com/google/cloud/storage/BucketTest.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
import com.google.common.collect.ImmutableList;
4040
import com.google.common.collect.Lists;
4141

42-
import org.easymock.EasyMock;
4342
import org.junit.After;
4443
import org.junit.Before;
4544
import org.junit.Rule;

‎gcloud-java-storage/src/test/java/com/google/cloud/storage/it/ITStorageTest.java‎

Lines changed: 41 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ public class ITStorageTest {
8181
private static final String CONTENT_TYPE = "text/plain";
8282
private static final byte[] BLOB_BYTE_CONTENT = {0xD, 0xE, 0xA, 0xD};
8383
private static final String BLOB_STRING_CONTENT = "Hello Google Cloud Storage!";
84-
private static final int MAX_BATCH_DELETES = 100;
84+
private static final int MAX_BATCH_SIZE = 100;
8585

8686
@BeforeClass
8787
public static void beforeClass() {
@@ -811,19 +811,31 @@ public void testBatchRequest() {
811811
}
812812

813813
@Test
814-
public void testBatchRequestManyDeletes() {
815-
List<BlobId> blobsToDelete = Lists.newArrayListWithCapacity(2 * MAX_BATCH_DELETES);
814+
public void testBatchRequestManyOperations() {
816815
List<StorageBatchResult<Boolean>> deleteResults =
817-
Lists.newArrayListWithCapacity(2 * MAX_BATCH_DELETES);
818-
for (int i = 0; i < 2 * MAX_BATCH_DELETES; i++) {
819-
blobsToDelete.add(BlobId.of(BUCKET, "test-batch-request-many-deletes-blob-" + i));
820-
}
816+
Lists.newArrayListWithCapacity(MAX_BATCH_SIZE);
817+
List<StorageBatchResult<Blob>> getResults =
818+
Lists.newArrayListWithCapacity(MAX_BATCH_SIZE / 2);
819+
List<StorageBatchResult<Blob>> updateResults =
820+
Lists.newArrayListWithCapacity(MAX_BATCH_SIZE / 2);
821821
StorageBatch batch = storage.batch();
822-
for (BlobId blob : blobsToDelete) {
823-
deleteResults.add(batch.delete(blob));
822+
for (int i = 0; i < MAX_BATCH_SIZE; i++) {
823+
BlobId blobId = BlobId.of(BUCKET, "test-batch-request-many-operations-blob-" + i);
824+
deleteResults.add(batch.delete(blobId));
825+
}
826+
for (int i = 0; i < MAX_BATCH_SIZE / 2; i++) {
827+
BlobId blobId = BlobId.of(BUCKET, "test-batch-request-many-operations-blob-" + i);
828+
getResults.add(batch.get(blobId));
829+
}
830+
for (int i = 0; i < MAX_BATCH_SIZE / 2; i++) {
831+
BlobInfo blob =
832+
BlobInfo.builder(BlobId.of(BUCKET, "test-batch-request-many-operations-blob-" + i))
833+
.build();
834+
updateResults.add(batch.update(blob));
824835
}
825-
String sourceBlobName1 = "test-batch-request-many-deletes-source-blob-1";
826-
String sourceBlobName2 = "test-batch-request-many-deletes-source-blob-2";
836+
837+
String sourceBlobName1 = "test-batch-request-many-operations-source-blob-1";
838+
String sourceBlobName2 = "test-batch-request-many-operations-source-blob-2";
827839
BlobInfo sourceBlob1 = BlobInfo.builder(BUCKET, sourceBlobName1).build();
828840
BlobInfo sourceBlob2 = BlobInfo.builder(BUCKET, sourceBlobName2).build();
829841
assertNotNull(storage.create(sourceBlob1));
@@ -836,21 +848,32 @@ public void testBatchRequestManyDeletes() {
836848
batch.submit();
837849

838850
// Check deletes
839-
for (StorageBatchResult<Boolean> deleteResult : deleteResults) {
840-
assertFalse(deleteResult.get());
851+
for (StorageBatchResult<Boolean> failedDeleteResult : deleteResults) {
852+
assertFalse(failedDeleteResult.get());
841853
}
842854

855+
// Check gets
856+
for (StorageBatchResult<Blob> failedGetResult : getResults) {
857+
assertNull(failedGetResult.get());
858+
}
859+
Blob remoteBlob1 = getResult.get();
860+
assertEquals(sourceBlob1.bucket(), remoteBlob1.bucket());
861+
assertEquals(sourceBlob1.name(), remoteBlob1.name());
862+
843863
// Check updates
864+
for (StorageBatchResult<Blob> failedUpdateResult : updateResults) {
865+
try {
866+
failedUpdateResult.get();
867+
fail("Expected StorageException");
868+
} catch (StorageException ex) {
869+
// expected
870+
}
871+
}
844872
Blob remoteUpdatedBlob2 = updateResult.get();
845873
assertEquals(sourceBlob2.bucket(), remoteUpdatedBlob2.bucket());
846874
assertEquals(sourceBlob2.name(), remoteUpdatedBlob2.name());
847875
assertEquals(updatedBlob2.contentType(), remoteUpdatedBlob2.contentType());
848876

849-
// Check gets
850-
Blob remoteBlob1 = getResult.get();
851-
assertEquals(sourceBlob1.bucket(), remoteBlob1.bucket());
852-
assertEquals(sourceBlob1.name(), remoteBlob1.name());
853-
854877
assertTrue(remoteBlob1.delete());
855878
assertTrue(remoteUpdatedBlob2.delete());
856879
}

0 commit comments

Comments
 (0)