scwhittle commented on code in PR #30439:
URL: https://github.com/apache/beam/pull/30439#discussion_r1525090238
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/StreamingDataflowWorker.java:
##########
@@ -653,6 +659,9 @@ private static void setUpWorkLoggingContext(String workId,
String computationId)
}
private int chooseMaximumNumberOfThreads() {
+ if (maxThreadCountOverride.get() != 0) {
Review Comment:
since this is possibly modified, it's safer to save the value you observe
instead of observing it again
probably wouldn't ever actually change between calls but might as well be
safe.
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/util/BoundedQueueExecutor.java:
##########
@@ -115,12 +143,16 @@ public boolean executorQueueIsEmpty() {
return executor.getQueue().isEmpty();
}
- public long allThreadsActiveTime() {
+ public synchronized long allThreadsActiveTime() {
return totalTimeMaxActiveThreadsUsed;
}
- public int activeCount() {
- return activeCount.intValue();
+ public synchronized int activeCount() {
+ return activeCount;
+ }
+
+ public synchronized int maximumThreadCount() {
+ return maximumThreadCount;
}
public long bytesOutstanding() {
Review Comment:
doesn't look modified by this PR but bytesOutstanding() and
elementsOutstanding() don't seem safe to read here if they are not beneath the
monitor
can these public methods be removed? or shoudl they be changed to use
monitor?
--
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]
