reuvenlax commented on code in PR #38783:
URL: https://github.com/apache/beam/pull/38783#discussion_r3385897501
##########
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/bigquery/StorageApiLoads.java:
##########
@@ -379,12 +381,19 @@ public WriteResult expandUntriggered(
PCollection<KV<DestinationT, StorageApiWritePayload>>
successfulConvertedRows =
convertMessagesResult.get(successfulConvertedRowsTag);
- if (numShards > 0) {
+ boolean streaming =
input.getPipeline().getOptions().as(StreamingOptions.class).isStreaming();
+ if (numShards > 0 && streaming) {
successfulConvertedRows =
successfulConvertedRows.apply(
"ResdistibuteNumShards",
Redistribute.<KV<DestinationT,
StorageApiWritePayload>>arbitrarily()
.withNumBuckets(numShards));
+ } else if (numShards > 0 && !streaming) {
+ successfulConvertedRows =
+ successfulConvertedRows
+ .apply("AddKeyWithSideInputs", ParDo.of(new
AddShardKeyFn<>(numShards)))
Review Comment:
Have you tested this?
The number of streams created by the transform fundamentally depends on the
number of bundles, not on locality within a worker. For each bundle and
destination, a stream will be created and destroyed. This PR might be effective
and increasing bundle locality of destinations, but this is something we'd have
to test (e.g. in streaming it would have the opposite effect!)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
