Fixed implementation of WorkflowExecutionUtils#getInstanceCloseEvent that previously could get into infinite loop after a timeout #472
Conversation
temporal-sdk/src/main/java/io/temporal/internal/common/WorkflowExecutionUtils.java
Show resolved
Hide resolved
temporal-sdk/src/main/java/io/temporal/internal/common/WorkflowExecutionUtils.java
Show resolved
Hide resolved
…that previously could get into infinite loop after a timeout Issue temporalio#471
temporal-sdk/src/main/java/io/temporal/internal/common/WorkflowExecutionUtils.java
Show resolved
Hide resolved
temporal-sdk/src/main/java/io/temporal/internal/common/WorkflowExecutionUtils.java
Show resolved
Hide resolved
|
Would be nice to also add a test, but it looks like it's not going to be easy, so LGTM. |
| .setBackoffCoefficient(1) | ||
| .setInitialInterval(Duration.ofMillis(1)) |
There was a problem hiding this comment.
Unrelated to this PR, since you've just copied existing values, but is it a good idea to hammer server without backoff?
There was a problem hiding this comment.
Thank you for raising it!
I agree, it's not. This is already on my radar and I probably will send one more PR shortly after I familiarize myself better with the implementation of GrpcRetryer and BackoffThrottler so I fully understand how these values are used.
This issue was discovered specifically because clients were very aggressively hammering the server. [every 1ms]
|
@vitarb thank you for the review, Vitaly! Yeah, a test would be good here, but a refactoring is needed here to make things mockable first. |
3 participants
What was changed:
Execution flow in
WorkflowExecutionUtils#getInstanceCloseEventhas been fixed to don't allow falling into an infinitecontinuecycle described in Issue #471.Closes issue:
Issue #471