lostluck commented on code in PR #26374:
URL: https://github.com/apache/beam/pull/26374#discussion_r1175444534
##########
sdks/go/pkg/beam/core/runtime/exec/plan.go:
##########
@@ -100,29 +109,29 @@ func (p *Plan) SourcePTransformID() string {
// are brought up on the first execution. If a bundle fails, the plan cannot
// be reused for further bundles. Does not panic. Blocking.
func (p *Plan) Execute(ctx context.Context, id string, manager DataContext)
error {
- if p.status == Initializing {
+ if p.getStatus() == Initializing {
Review Comment:
You'll note that we change the status throughout Execute. But this is the
only goroutine that should be making those changes.
The Split and Progress calls only read the status state, hence the data race
on read they experienced.
The goal isn't to make bundle execution state fully threadsafe, because that
would be incredibly heavyweight on a per element basis. Only the parts that
need to be accessed asynchronously need to be threadsafe, and status was missed.
--
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]
