Store activity result on graceful worker stop #579
Conversation
cretz
requested review from
alexshtin,
mfateev,
Spikhalskiy,
Sushisource and
vitarb
as code owners
…aceful-shutdown # Conflicts: # test/integration_test.go
| @@ -881,11 +881,6 @@ func (atp *activityTaskPoller) ProcessTask(task interface{}) error { | |||
| return nil | |||
| } | |||
|
|
|||
| // if worker is stopping, don't bother reporting activity completion | |||
There was a problem hiding this comment.
Curious to know why this was there.
There was a problem hiding this comment.
Me too, it dates back to the very first creation of integration tests in cadence, see https://github.com/cretz/sdk-go/blame/a115d469e80ed5c9f38f0c3a2183a11d16fb2dd5/internal/internal_task_pollers.go#L890
| if strings.Contains(ts.T().Name(), "GracefulActivityCompletion") { | ||
| options.WorkerStopTimeout = 10 * time.Second | ||
| } |
There was a problem hiding this comment.
I tried and failed to find a more organized way to do this than checking test names which makes me want to 🤮
You know Go much better than I, though, so if there's a reasonably easy way you see to fix that, please do.
There was a problem hiding this comment.
The proper way IMO is to have tests instantiate their harnesses instead of the other way around, but testify (and the whole junit-style of lifecycle management) tends to encourage non-test-specific setup so without inverting control here, this is about as good as you can do I think.
|
@vitarb or @Sushisource - my build is failing only when cache is disabled yet it is unclear why. It can be replicated by running: However, this failure is also occurring for me on master but not Buildkite. Can either of y'all replicate and/or have any ideas? Thanks |
|
The failing test is failing in master and will be fixed in #589. |
What was changed
Allow activity result handling to complete even on worker stop. Also determined flaky test was due to retry throttler not being allowed to exit early, so fixed that (side effect being that all worker stops happen much faster now).
Why?
Activity results were not getting stored on worker stop
Checklist