lostluck commented on code in PR #26782:
URL: https://github.com/apache/beam/pull/26782#discussion_r1213625552
##########
sdks/go/pkg/beam/core/runtime/exec/pardo.go:
##########
@@ -249,6 +253,10 @@ func (n *ParDo) FinishBundle(_ context.Context) error {
if _, err := n.invokeDataFn(n.ctx, typex.NoFiringPane(),
window.SingleGlobalWindow, mtime.ZeroTimestamp, n.Fn.FinishBundleFn(), nil);
err != nil {
return n.fail(err)
}
+ // Flush timers if any.
+ if err := n.TimerTracker.FlushAndReset(n.ctx, n.timerManager); err !=
nil {
Review Comment:
Same thing, it's checked inside FlushAndReset.
The reason we can get away with this is that in Go, the receiver is just
another parameter. So it's valid to call methods on nil. In this case, that
allows folding the nil check into the method itself instead of outside.
The main reason not to do this for performance optimization: That is, you
never check nil anywhere because it's always correct by construction. We can't
do that here, so we'd need to check every time it's called anyway, so I folded
it into the method definition to try to keep pardo.go a little tidier. It's
complicated enough already.
--
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]
