Skip to content

Commit d19b534

Browse files
authored
Fix split thresholds for BQ sink (#36422)
* ensure that we don't exceed split threshold * fix OBO error
1 parent f9feffc commit d19b534

2 files changed

Lines changed: 22 additions & 10 deletions

File tree

sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/SplittingIterable.java

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@
2828
import java.util.function.BiConsumer;
2929
import java.util.function.Function;
3030
import org.apache.beam.sdk.values.TimestampedValue;
31+
import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.collect.Iterators;
3132
import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.collect.Lists;
33+
import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.collect.PeekingIterator;
3234
import org.checkerframework.checker.nullness.qual.Nullable;
3335
import org.joda.time.Instant;
3436

@@ -85,7 +87,8 @@ public SplittingIterable(
8587
@Override
8688
public Iterator<Value> iterator() {
8789
return new Iterator<Value>() {
88-
final Iterator<StorageApiWritePayload> underlyingIterator = underlying.iterator();
90+
final PeekingIterator<StorageApiWritePayload> underlyingIterator =
91+
Iterators.peekingIterator(underlying.iterator());
8992

9093
@Override
9194
public boolean hasNext() {
@@ -103,6 +106,13 @@ public Value next() {
103106
ProtoRows.Builder inserts = ProtoRows.newBuilder();
104107
long bytesSize = 0;
105108
while (underlyingIterator.hasNext()) {
109+
// Make sure that we don't exceed the split-size length over multiple elements. A single
110+
// element can exceed
111+
// the split threshold, but in that case it should be the only element returned.
112+
if ((bytesSize + underlyingIterator.peek().getPayload().length > splitSize)
113+
&& inserts.getSerializedRowsCount() > 0) {
114+
break;
115+
}
106116
StorageApiWritePayload payload = underlyingIterator.next();
107117
ByteString byteString = ByteString.copyFrom(payload.getPayload());
108118
@Nullable TableRow failsafeTableRow = null;
@@ -157,9 +167,6 @@ public Value next() {
157167
timestamps.add(timestamp);
158168
failsafeRows.add(failsafeTableRow);
159169
bytesSize += byteString.size();
160-
if (bytesSize > splitSize) {
161-
break;
162-
}
163170
}
164171
return new AutoValue_SplittingIterable_Value(inserts.build(), timestamps, failsafeRows);
165172
}

sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/StorageApiWriteUnshardedRecords.java

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,15 +1008,18 @@ void postFlush() {
10081008
this.bigLakeConfiguration = bigLakeConfiguration;
10091009
}
10101010

1011-
boolean shouldFlush() {
1012-
return numPendingRecords > flushThresholdCount || numPendingRecordBytes > flushThresholdBytes;
1011+
boolean shouldFlush(int recordBytes) {
1012+
return numPendingRecords > flushThresholdCount
1013+
|| (((numPendingRecordBytes + recordBytes) > flushThresholdBytes)
1014+
&& numPendingRecords > 0);
10131015
}
10141016

10151017
void flushIfNecessary(
10161018
OutputReceiver<BigQueryStorageApiInsertError> failedRowsReceiver,
1017-
@Nullable OutputReceiver<TableRow> successfulRowsReceiver)
1019+
@Nullable OutputReceiver<TableRow> successfulRowsReceiver,
1020+
int recordBytes)
10181021
throws Exception {
1019-
if (shouldFlush()) {
1022+
if (shouldFlush(recordBytes)) {
10201023
forcedFlushes.inc();
10211024
// Too much memory being used. Flush the state and wait for it to drain out.
10221025
// TODO(reuvenlax): Consider waiting for memory usage to drop instead of waiting for all the
@@ -1172,10 +1175,12 @@ public void process(
11721175
@Nullable
11731176
OutputReceiver<TableRow> successfulRowsReceiver =
11741177
(successfulRowsTag != null) ? o.get(successfulRowsTag) : null;
1175-
flushIfNecessary(failedRowsReceiver, successfulRowsReceiver);
1178+
1179+
int recordBytes = element.getValue().getPayload().length;
1180+
flushIfNecessary(failedRowsReceiver, successfulRowsReceiver, recordBytes);
11761181
state.addMessage(element.getValue(), elementTs, failedRowsReceiver);
11771182
++numPendingRecords;
1178-
numPendingRecordBytes += element.getValue().getPayload().length;
1183+
numPendingRecordBytes += recordBytes;
11791184
}
11801185

11811186
private OutputReceiver<TableRow> makeSuccessfulRowsreceiver(

0 commit comments

Comments
 (0)