Skip to content

Commit e524c74

Browse files
BenWhiteheadBenWhitehead
committed
fix: update BlobAppendableUploadConfig and FlushPolicy.MinFlushSizeFlushPolicy to default to 4MiB minFlushSize and 16MiB maxPendingBytes
Workload: upload a series of objects as fast as possible using 12 workers to upload concurrently. | | minFlushSize | maxPendingBytes | duration | |--------|-------------:|----------------:|----------:| | Before | 256KiB | 8MiB | 3.646 min | | After | 4MiB | 16MiB | 2.014 min | Add default instances of FlushPolicy.{min,max}FlushSize() to parameters of ITAppendableUploadTest
1 parent 094040c commit e524c74

4 files changed

Lines changed: 42 additions & 19 deletions

File tree

‎google-cloud-storage/src/main/java/com/google/cloud/storage/BlobAppendableUploadConfig.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
package com.google.cloud.storage;
1818

19-
import static com.google.cloud.storage.ByteSizeConstants._256KiB;
2019
import static java.util.Objects.requireNonNull;
2120

2221
import com.google.api.core.ApiFuture;
@@ -54,7 +53,7 @@ public final class BlobAppendableUploadConfig {
5453

5554
private static final BlobAppendableUploadConfig INSTANCE =
5655
new BlobAppendableUploadConfig(
57-
FlushPolicy.minFlushSize(_256KiB), CloseAction.CLOSE_WITHOUT_FINALIZING, 3);
56+
FlushPolicy.minFlushSize(), CloseAction.CLOSE_WITHOUT_FINALIZING, 3);
5857

5958
private final FlushPolicy flushPolicy;
6059
private final CloseAction closeAction;
@@ -71,7 +70,7 @@ private BlobAppendableUploadConfig(
7170
* The {@link FlushPolicy} which will be used to determine when and how many bytes to flush to
7271
* GCS.
7372
*
74-
* <p><i>Default:</i> {@link FlushPolicy#minFlushSize(int) FlushPolicy.minFlushSize(256 * 1024)}
73+
* <p><i>Default:</i> {@link FlushPolicy#minFlushSize()}
7574
*
7675
* @see #withFlushPolicy(FlushPolicy)
7776
* @since 2.51.0 This new api is in preview and is subject to breaking changes.
@@ -84,7 +83,7 @@ public FlushPolicy getFlushPolicy() {
8483
/**
8584
* Return an instance with the {@code FlushPolicy} set to be the specified value.
8685
*
87-
* <p><i>Default:</i> {@link FlushPolicy#minFlushSize(int) FlushPolicy.minFlushSize(256 * 1024)}
86+
* <p><i>Default:</i> {@link FlushPolicy#minFlushSize()}
8887
*
8988
* @see #getFlushPolicy()
9089
* @since 2.51.0 This new api is in preview and is subject to breaking changes.

‎google-cloud-storage/src/main/java/com/google/cloud/storage/ByteSizeConstants.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ final class ByteSizeConstants {
2626
static final int _768KiB = 768 * _1KiB;
2727
static final int _1MiB = 1024 * _1KiB;
2828
static final int _2MiB = 2 * _1MiB;
29+
static final int _4MiB = 4 * _1MiB;
2930
static final int _16MiB = 16 * _1MiB;
3031
static final int _32MiB = 32 * _1MiB;
3132
static final long _1GiB = 1024 * _1MiB;

‎google-cloud-storage/src/main/java/com/google/cloud/storage/FlushPolicy.java‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import static com.google.cloud.storage.ByteSizeConstants._16MiB;
2020
import static com.google.cloud.storage.ByteSizeConstants._2MiB;
21+
import static com.google.cloud.storage.ByteSizeConstants._4MiB;
2122

2223
import com.google.api.core.BetaApi;
2324
import com.google.api.core.InternalExtensionOnly;
@@ -67,6 +68,13 @@ public static MaxFlushSizeFlushPolicy maxFlushSize(int maxFlushSize) {
6768
/**
6869
* Default instance factory method for {@link MinFlushSizeFlushPolicy}.
6970
*
71+
* <p><i>Default:</i> logically equivalent to the following:
72+
*
73+
* <pre>
74+
* {@link #minFlushSize(int) FlushPolicy.minFlushSize}(4 * 1024 * 1024)
75+
* .{@link MinFlushSizeFlushPolicy#withMaxPendingBytes(long) withMaxPendingBytes}(16 * 1024 * 1024)
76+
* </pre>
77+
*
7078
* @since 2.51.0 This new api is in preview and is subject to breaking changes.
7179
*/
7280
@BetaApi
@@ -204,7 +212,7 @@ public String toString() {
204212
@BetaApi
205213
public static final class MinFlushSizeFlushPolicy extends FlushPolicy {
206214
private static final MinFlushSizeFlushPolicy INSTANCE =
207-
new MinFlushSizeFlushPolicy(_2MiB, _16MiB);
215+
new MinFlushSizeFlushPolicy(_4MiB, _16MiB);
208216

209217
private final int minFlushSize;
210218
private final long maxPendingBytes;
@@ -217,7 +225,7 @@ private MinFlushSizeFlushPolicy(int minFlushSize, long maxPendingBytes) {
217225
/**
218226
* The minimum number of bytes to include in each automatic flush
219227
*
220-
* <p><i>Default:</i> {@code 2097152 (2 MiB)}
228+
* <p><i>Default:</i> {@code 4194304 (4 MiB)}
221229
*
222230
* @see #withMinFlushSize(int)
223231
*/
@@ -229,7 +237,7 @@ public int getMinFlushSize() {
229237
/**
230238
* Return an instance with the {@code minFlushSize} set to the specified value.
231239
*
232-
* <p><i>Default:</i> {@code 2097152 (2 MiB)}
240+
* <p><i>Default:</i> {@code 4194304 (4 MiB)}
233241
*
234242
* @param minFlushSize The number of bytes to buffer before flushing.
235243
* @return The new instance

‎google-cloud-storage/src/test/java/com/google/cloud/storage/ITAppendableUploadTest.java‎

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
import com.google.cloud.storage.BlobAppendableUpload.AppendableUploadWriteableByteChannel;
2727
import com.google.cloud.storage.BlobAppendableUploadConfig.CloseAction;
2828
import com.google.cloud.storage.Crc32cValue.Crc32cLengthKnown;
29+
import com.google.cloud.storage.FlushPolicy.MaxFlushSizeFlushPolicy;
30+
import com.google.cloud.storage.FlushPolicy.MinFlushSizeFlushPolicy;
2931
import com.google.cloud.storage.ITAppendableUploadTest.UploadConfigParameters;
3032
import com.google.cloud.storage.MetadataField.PartRange;
3133
import com.google.cloud.storage.TransportCompatibility.Transport;
@@ -103,11 +105,7 @@ public void appendableUpload_emptyObject()
103105
@Test
104106
public void appendableUpload_bytes()
105107
throws IOException, ExecutionException, InterruptedException, TimeoutException {
106-
if (p.uploadConfig.getCloseAction() == CloseAction.FINALIZE_WHEN_CLOSING) {
107-
assumeTrue(
108-
"testbench broken https://github.com/googleapis/storage-testbench/issues/733",
109-
p.content.length() > 1_000);
110-
}
108+
checkTestbenchIssue733();
111109

112110
BlobAppendableUpload upload =
113111
storage.blobAppendableUpload(
@@ -177,11 +175,7 @@ public void appendableBlobUploadTakeover() throws Exception {
177175

178176
@Test
179177
public void testUploadFileUsingAppendable() throws Exception {
180-
if (p.uploadConfig.getCloseAction() == CloseAction.FINALIZE_WHEN_CLOSING) {
181-
assumeTrue(
182-
"testbench broken https://github.com/googleapis/storage-testbench/issues/733",
183-
p.content.length() > 1_000);
184-
}
178+
checkTestbenchIssue733();
185179

186180
String objectName = UUID.randomUUID().toString();
187181
String fileName =
@@ -240,6 +234,26 @@ public void takeoverJustToFinalizeWorks() throws Exception {
240234
() -> assertThat(done2.getCrc32c()).isNotNull());
241235
}
242236

237+
private void checkTestbenchIssue733() {
238+
if (p.uploadConfig.getCloseAction() == CloseAction.FINALIZE_WHEN_CLOSING) {
239+
int estimatedMessageCount = 0;
240+
FlushPolicy flushPolicy = p.uploadConfig.getFlushPolicy();
241+
if (flushPolicy instanceof MinFlushSizeFlushPolicy) {
242+
MinFlushSizeFlushPolicy min = (MinFlushSizeFlushPolicy) flushPolicy;
243+
estimatedMessageCount = p.content.length() / min.getMinFlushSize();
244+
} else if (flushPolicy instanceof MaxFlushSizeFlushPolicy) {
245+
MaxFlushSizeFlushPolicy max = (MaxFlushSizeFlushPolicy) flushPolicy;
246+
estimatedMessageCount = p.content.length() / max.getMaxFlushSize();
247+
}
248+
// if our int division results in a partial message, ensure we are counting at least one
249+
// message. We have a separate test specifically for empty objects.
250+
estimatedMessageCount = Math.max(estimatedMessageCount, 1);
251+
assumeTrue(
252+
"testbench broken https://github.com/googleapis/storage-testbench/issues/733",
253+
estimatedMessageCount > 1);
254+
}
255+
}
256+
243257
private byte[] readAllBytes(BlobInfo actual)
244258
throws IOException, InterruptedException, ExecutionException, TimeoutException {
245259
ApiFuture<BlobReadSession> blobReadSessionFuture = storage.blobReadSession(actual.getBlobId());
@@ -255,8 +269,9 @@ public static final class UploadConfigParameters implements ParametersProvider {
255269
ImmutableList.of(
256270
FlushPolicy.minFlushSize(1_000),
257271
FlushPolicy.minFlushSize(1_000).withMaxPendingBytes(5_000),
258-
FlushPolicy.maxFlushSize(1_000),
259-
FlushPolicy.maxFlushSize(500_000));
272+
FlushPolicy.maxFlushSize(500_000),
273+
FlushPolicy.minFlushSize(),
274+
FlushPolicy.maxFlushSize());
260275
private static final ImmutableList<CloseAction> closeActions =
261276
ImmutableList.copyOf(CloseAction.values());
262277
public static final ImmutableList<Integer> objectSizes =

0 commit comments

Comments
 (0)