tvalentyn commented on code in PR #36003:
URL: https://github.com/apache/beam/pull/36003#discussion_r2309824566


##########
sdks/python/apache_beam/runners/worker/worker_status.py:
##########
@@ -285,12 +285,12 @@ def _log_lull_sampler_info(self, sampler_info, 
instruction):
     if timeout_exceeded:
       _LOGGER.error(
           (
-              'Operation ongoing in bundle %s%s for at least %.2f seconds'
-              ' without outputting or completing.\n'
+              'Processing of an element in bundle %s%s has exceeded the 
specified'

Review Comment:
   ```suggestion
                 'Processing of an element in bundle %s%s has exceeded the 
specified '
   ```



##########
sdks/go/pkg/beam/core/metrics/sampler.go:
##########
@@ -67,7 +67,7 @@ func (s *StateSampler) Sample(ctx context.Context, t 
time.Duration) error {
                        s.nextLogTime += s.logInterval
                }
                if s.restartLullTimeout > 0 && s.millisSinceLastTransition > 
s.restartLullTimeout {
-                       return errors.Errorf("Operation ongoing in transform %v 
for at least %v ms without outputting or completing in state %v, the SDK 
harness will be terminated and restarted", ps.pid, s.millisSinceLastTransition, 
getState(ps.state))
+                       return errors.Errorf("Processing of an element in 
transform %v has exceeded the specified timeout of %v ms without outputting or 
completing in state %v, SDK harness will be terminated", ps.pid, 
s.restartLullTimeout, getState(ps.state))

Review Comment:
   
   for my information, what does ps.pid stand for? 



##########
sdks/go/pkg/beam/core/metrics/sampler.go:
##########
@@ -67,7 +67,7 @@ func (s *StateSampler) Sample(ctx context.Context, t 
time.Duration) error {
                        s.nextLogTime += s.logInterval
                }
                if s.restartLullTimeout > 0 && s.millisSinceLastTransition > 
s.restartLullTimeout {
-                       return errors.Errorf("Operation ongoing in transform %v 
for at least %v ms without outputting or completing in state %v, the SDK 
harness will be terminated and restarted", ps.pid, s.millisSinceLastTransition, 
getState(ps.state))
+                       return errors.Errorf("Processing of an element in 
transform %v has exceeded the specified timeout of %v ms without outputting or 
completing in state %v, SDK harness will be terminated", ps.pid, 
s.restartLullTimeout, getState(ps.state))

Review Comment:
   is ms the right unit? it sounds like milliseconds. 
   
   use min or minutes for minutes
   



##########
sdks/java/harness/src/main/java/org/apache/beam/fn/harness/control/ExecutionStateSampler.java:
##########
@@ -400,25 +400,26 @@ private Optional<String> takeSample(long 
currentTimeMillis, long millisSinceLast
           if (thread == null) {
             timeoutMessage =
                 String.format(
-                    "Operation ongoing in bundle %s for at least %s without 
outputting "
-                        + "or completing (stack trace unable to be generated). 
The SDK worker will restart.",
+                    "Processing of an element in bundle %s has exceeded the 
specified timeout of %s "
+                        + "(stack trace unable to be generated). The SDK 
worker will be terminated.",
                     processBundleId.get(),
                     DURATION_FORMATTER.print(
                         
Duration.millis(userSpecifiedLullTimeMsForRestart).toPeriod()));
           } else if (currentExecutionState == null) {
             timeoutMessage =
                 String.format(
-                    "Operation ongoing in bundle %s for at least %s without 
outputting "
-                        + "or completing:%n  at %s. The SDK worker will 
restart.",
+                    "Processing of an element in bundle %s has exceeded the 
specified timeout of %s "
+                        + "without outputting or completing:%n  at %s. The SDK 
worker will be terminated.",
                     processBundleId.get(),
                     DURATION_FORMATTER.print(
                         
Duration.millis(userSpecifiedLullTimeMsForRestart).toPeriod()),
                     Joiner.on("\n  at ").join(thread.getStackTrace()));
           } else {
             timeoutMessage =
                 String.format(
-                    "Operation ongoing in bundle %s for PTransform{id=%s, 
name=%s, state=%s} "
-                        + "for at least %s without outputting or completing:%n 
 at %s. The SDK worker will restart.",
+                    "Processing of an element in bundle %s for 
PTransform{id=%s, name=%s, state=%s} "
+                        + "has exceeded the specified timeout of %s without 
outputting or completing:%n  at %s. "

Review Comment:
   let's add units throughout the messages in this file (%s minutes ?) 



-- 
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: github-unsubscr...@beam.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to