Fix flaky build id issue #1338
Fix flaky build id issue #1338
Conversation
Sushisource
force-pushed
the
build-id-fix
branch
from
1b78e8f to
6fa84d1
Compare
Sushisource
force-pushed
the
build-id-fix
branch
from
6fa84d1 to
47f7c57
Compare
internal/internal_task_handlers.go
Outdated
| @@ -1028,10 +1028,16 @@ ProcessEvents: | |||
| } | |||
| if isReplay { | |||
| w.workflowInfo.currentTaskBuildID = currentBuildID | |||
| } else if !isReplay && !isQueryOnlyTask { | |||
| } else if !isQueryOnlyTask { | |||
| // Query only tasks should use the build ID from the workflow task, not the worker's | |||
There was a problem hiding this comment.
This comment seem irrelevant now?
internal/internal_task_handlers.go
Outdated
| // Query only tasks should use the build ID from the workflow task, not the worker's | ||
| // build id, since the user cares about what affected workflow state. | ||
| w.workflowInfo.currentTaskBuildID = w.wth.workerBuildID | ||
| } else { | ||
| // It is a query only task, and we're not replaying somehow - still use the task's build |
There was a problem hiding this comment.
I don't understand this comment, didn't we discuss before that queries are always treated as not replaying?
There was a problem hiding this comment.
Yeah, I can just get rid of the not replaying part
internal/internal_task_handlers.go
Outdated
| } else { | ||
| // It *is* a query only task, use the task's build ID (queries are considered non-replay | ||
| // in go sdk), but only if one was actually discovered. | ||
| if currentBuildID != "" { |
There was a problem hiding this comment.
When is currentBuildID empty?
There was a problem hiding this comment.
It can be empty here if the lookahead didn't find anything, but we wouldn't want to overwrite some previous task which did find something.
There was a problem hiding this comment.
Yeah I am wondering what scenario look ahead can't find anything?
There was a problem hiding this comment.
The problem is sort of specific to queries on the first task I think. No WFT completed event exists yet to be looked-ahead-to (well, it does actually in the query, but we don't read it since we're already up-to-date), but the nextTask code still runs, which then returns this as empty.
There was a problem hiding this comment.
Ah I didn't think of the first WFT thanks for clarifying
This should fix the only case where I could imagine the id not being set. Could not repro locally.