scwhittle commented on code in PR #40639:
URL: https://github.com/apache/beam/pull/40639#discussion_r4323275818
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/windmill/work/processing/ComputationWorkExecutorFactory.java:
##########
@@ -166,23 +168,27 @@ private static void trackAutoscalingBytesRead(
MapTask mapTask,
Nodes.ParallelInstructionNode readNode,
Coder<?> readCoder,
- ReadOperation readOperation,
+ Operation operation,
MapTaskExecutor mapTaskExecutor,
String counterName) {
- NameContext nameContext =
- NameContext.create(
- mapTask.getStageName(),
- readNode.getParallelInstruction().getOriginalName(),
- readNode.getParallelInstruction().getSystemName(),
- readNode.getParallelInstruction().getName());
- readOperation.receivers[0].addOutputCounter(
- counterName,
- new OutputObjectAndByteCounter(
- new
IntrinsicMapTaskExecutorFactory.ElementByteSizeObservableCoder<>(readCoder),
- mapTaskExecutor.getOutputCounters(),
- nameContext)
- .setSamplingPeriod(100)
- .countBytes(counterName));
+ if (operation.receivers != null
+ && operation.receivers.length > 0
+ && operation.receivers[0] != null) {
+ NameContext nameContext =
+ NameContext.create(
+ mapTask.getStageName(),
+ readNode.getParallelInstruction().getOriginalName(),
+ readNode.getParallelInstruction().getSystemName(),
+ readNode.getParallelInstruction().getName());
+ operation.receivers[0].addOutputCounter(
+ counterName,
+ new OutputObjectAndByteCounter(
+ new
IntrinsicMapTaskExecutorFactory.ElementByteSizeObservableCoder<>(readCoder),
+ mapTaskExecutor.getOutputCounters(),
+ nameContext)
+ .setSamplingPeriod(100)
Review Comment:
do we want to sample this often? should it be time-based?
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/windmill/work/processing/ComputationWorkExecutorFactory.java:
##########
@@ -166,23 +168,27 @@ private static void trackAutoscalingBytesRead(
MapTask mapTask,
Nodes.ParallelInstructionNode readNode,
Coder<?> readCoder,
- ReadOperation readOperation,
+ Operation operation,
MapTaskExecutor mapTaskExecutor,
String counterName) {
- NameContext nameContext =
- NameContext.create(
- mapTask.getStageName(),
- readNode.getParallelInstruction().getOriginalName(),
- readNode.getParallelInstruction().getSystemName(),
- readNode.getParallelInstruction().getName());
- readOperation.receivers[0].addOutputCounter(
- counterName,
- new OutputObjectAndByteCounter(
- new
IntrinsicMapTaskExecutorFactory.ElementByteSizeObservableCoder<>(readCoder),
- mapTaskExecutor.getOutputCounters(),
- nameContext)
- .setSamplingPeriod(100)
- .countBytes(counterName));
+ if (operation.receivers != null
+ && operation.receivers.length > 0
+ && operation.receivers[0] != null) {
+ NameContext nameContext =
+ NameContext.create(
+ mapTask.getStageName(),
+ readNode.getParallelInstruction().getOriginalName(),
+ readNode.getParallelInstruction().getSystemName(),
+ readNode.getParallelInstruction().getName());
+ operation.receivers[0].addOutputCounter(
Review Comment:
why track just the first receiver? If a operator is multioutput are we
missing tracking some of it?
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/IntrinsicMapTaskExecutorFactory.java:
##########
@@ -272,18 +270,29 @@ private OperationNode createParDoOperation(
}
successorOffset += 1;
}
+ CloudObject cloudUserFn = CloudObject.fromSpec(parDo.getUserFn());
ParDoFn fn =
parDoFnFactory.create(
options,
- CloudObject.fromSpec(parDo.getUserFn()),
+ cloudUserFn,
parDo.getSideInputs(),
mainOutputTag,
outputTagsToReceiverIndicesBuilder.build(),
executionContext,
operationContext);
+ // An SDF's SplittableProcessFn behaves like a source, so we need to
report its bytes read even
+ // though it's not a read operator for autoscaling to work properly.
+ Coder<?> bytesReadCoder = null;
+ if ("SplittableProcessFn".equals(cloudUserFn.getClassName())) {
Review Comment:
this seems brittle. I'm not very familiar with this code but perhaps Kenn
will have a cleaner way of doing this.
--
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]
