lostluck commented on pull request #15911: URL: https://github.com/apache/beam/pull/15911#issuecomment-966668074
That's right. We should however, still add one to the `(*LiftedCombine).FinishBundle()` call at the top of the function (rather than in the loop.). The Lifted Combine isn't calling Combine's FinishBundle until the end. In practice, a LiftedCombine is always immeadiately followed by a DataSink, whose process element is what gets called. I don't know if we need to be separating the DataSource and DataSink into it's their PID metrics just yet, vs folding them into the their adjacent transforms. That's a later change we can make once we determine it's worthwhile. At that point, it's likely moot whether we're attributing anything to the LiftedCombine's FinishBundle since it's doing so little work anyway. Adding it to the top of the FinishBundle will at least attribute all that work to the LiftedCombine properly instead of attributing them to the previous DoFn's FinishBundle call, which is what it's currently doing, which is certainly incorrect. -- 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]
