arunpandianp commented on code in PR #30270:
URL: https://github.com/apache/beam/pull/30270#discussion_r1484635603
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/DataflowExecutionContext.java:
##########
@@ -341,8 +342,8 @@ public Optional<ActiveMessageMetadata>
getActiveMessageMetadata() {
return Optional.ofNullable(activeMessageMetadata);
}
- public Map<String, IntSummaryStatistics> getProcessingTimesByStepCopy() {
- Map<String, IntSummaryStatistics> processingTimesCopy =
processingTimesByStep;
+ public synchronized Map<String, IntSummaryStatistics>
getProcessingTimesByStepCopy() {
+ Map<String, IntSummaryStatistics> processingTimesCopy = new
HashMap<>(processingTimesByStep);
Review Comment:
> since getActiveMessageMetadata returns an optional, i dont need to copy it
, correct?
We don't need to deep copy there since ActiveMessageMetadata is immutable.
Immutable objects with all immutable members are thread safe after construction.
> Would this work?
looks good to me. I would call it copy instead of clone though to not mixup
with clone().
--
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]
