lostluck commented on a change in pull request #16980:
URL: https://github.com/apache/beam/pull/16980#discussion_r817210898
##########
File path: sdks/go/pkg/beam/core/runtime/exec/pardo.go
##########
@@ -220,6 +226,49 @@ func (n *ParDo) FinishBundle(_ context.Context) error {
return nil
}
+func (n *ParDo) FinalizeBundle(ctx context.Context) error {
+ failedIndices := []int{}
+ for idx, bfc := range n.bf.callbacks {
Review comment:
Is there any reason the callbacks have to live in each ParDo, rather
than an independant structure that's referenced in the ParDos at Plan
construction time, that can be swapped out/cleared when the plan is
re-used/garbage collected?
That would avoid the plumbing creep where every implementation of Unit needs
new methods just to get down to the ParDos that may have callbacks.
You'll note that in Go, that it's not a great idea to simply add new methods
to an interface, as it will break everything that's trying to implement the
interface.
I'll also point out that we have a pattern that does this, like how we
collect metrics from PCollections, by pre-collecting all the PCollections in a
plan rather than plumbing a new path all the way through: See
https://github.com/apache/beam/blob/master/sdks/go/pkg/beam/core/runtime/exec/plan.go#L59
https://github.com/apache/beam/blob/master/sdks/go/pkg/beam/core/runtime/exec/translate.go#L82
--
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]
