Approach 3 is about caching the bundle descriptor forever but tearing down a "live" instance of the DoFns at some SDK chosen arbitrary point in time. This way if a future ProcessBundleRequest comes in, a new "live" instance can be constructed. Approach 2 is still needed so that when the workers are being shutdown all the "live" instances are torn down.
On Fri, Oct 25, 2019 at 12:56 PM Robert Burke <[email protected]> wrote: > Approach 2 isn't incompatible with approach 3. 3 simple sets down > convention/configuration for the conditions when the SDK will do this after > process bundle has completed. > > > > On Fri, Oct 25, 2019, 12:34 PM Robert Bradshaw <[email protected]> > wrote: > >> I think we'll still need approach (2) for when the pipeline finishes >> and a runner is tearing down workers. >> >> On Fri, Oct 25, 2019 at 10:36 AM Maximilian Michels <[email protected]> >> wrote: >> > >> > Hi Jincheng, >> > >> > Thanks for bringing this up and capturing the ideas in the doc. >> > >> > Intuitively, I would have also considered adding a new Proto message for >> > the teardown, but I think the idea to trigger this logic when the SDK >> > Harness evicts process bundle descriptors is more elegant. >> > >> > Thanks, >> > Max >> > >> > On 25.10.19 17:23, Luke Cwik wrote: >> > > I like approach 3 since it doesn't add additional complexity to the >> API >> > > and individual SDKs can choose to implement any clean-up strategy they >> > > want or none at all which is the simplest. >> > > >> > > On Thu, Oct 24, 2019 at 8:46 PM jincheng sun < >> [email protected] >> > > <mailto:[email protected]>> wrote: >> > > >> > > Hi, >> > > >> > > Thanks for your comments in doc, I have add Approach 3 which you >> > > mentioned! @Luke >> > > >> > > For now, we should do a decision for Approach 3 and Approach 1. >> > > Detail can be found in doc [1] >> > > >> > > Welcome anyone's feedback :) >> > > >> > > Regards, >> > > Jincheng >> > > >> > > [1] >> > > >> https://docs.google.com/document/d/1sCgy9VQPf9zVXKRquK8P6N4x7aB62GEO8ozkujRSHZg/edit?usp=sharing >> > > >> > > jincheng sun <[email protected] >> > > <mailto:[email protected]>> 于2019年10月25日周五 上午10:40写道： >> > > >> > > Hi, >> > > >> > > Functionally capable of `abort`, but it will be called at the >> > > end of operator. So, I prefer `dispose` semantics. i.e., all >> > > normal logic has been executed. >> > > >> > > Best, >> > > Jincheng >> > > >> > > Harsh Vardhan <[email protected] <mailto:[email protected] >> >> >> > > 于2019年10月23日周三 上午12:14写道： >> > > >> > > Would approach 1 be akin to abort semantics? >> > > >> > > On Mon, Oct 21, 2019 at 8:01 PM jincheng sun >> > > <[email protected] <mailto: >> [email protected]>> >> > > wrote: >> > > >> > > Hi Luke, >> > > >> > > Thanks a lot for your reply. Since it allows to share >> > > one SDK harness between multiple executable stages, >> the >> > > control service termination may occur much later than >> > > the completion of an executable stage. This is the >> main >> > > reason I prefer runners to control the teardown of >> DoFns. >> > > >> > > Regarding to "SDK harnesses can terminate instances >> any >> > > time they want and start new instances anytime as >> > > well.", personally I think it's not conflict with the >> > > proposed Approach 1 as the SDK harness could decide >> what >> > > to do when receiving the teardown request. It could do >> > > nothing if the DoFns has already been teared down and >> > > could also tear down the DoFns if needed. >> > > >> > > What do you think? >> > > >> > > Best, >> > > Jincheng >> > > >> > > Luke Cwik <[email protected] <mailto:[email protected] >> >> >> > > 于2019年10月22日周二 上午2:05写道： >> > > >> > > Approach 2 is currently the suggested approach[1] >> > > for DoFn's to shutdown. >> > > Note that SDK harnesses can terminate instances >> any >> > > time they want and start new instances anytime as >> well. >> > > >> > > Why do you want to expose this logic so that >> Runners >> > > could control it? >> > > >> > > 1: >> > > >> https://docs.google.com/document/d/1n6s3BOxOPct3uF4UgbbI9O9rpdiKWFH9R6mtVmR7xp0/edit# >> > > >> > > On Mon, Oct 21, 2019 at 4:27 AM jincheng sun >> > > <[email protected] >> > > <mailto:[email protected]>> wrote: >> > > >> > > Hi, >> > > I found that in `SdkHarness` do not stop the >> > > `SdkWorker` when finish. We should add the >> > > logic for stop the `SdkWorker` in >> `SdkHarness`. >> > > More detail can be found [1]. >> > > >> > > There are two approaches to solve this issue: >> > > >> > > Approach 1: We can add a Fn API for teardown >> > > purpose and the runner will teardown a >> specific >> > > bundle descriptor via this teardown Fn API >> > > during disposing. >> > > Approach 2: The control service termination >> > > could be seen as a signal and once SDK harness >> > > receives this signal, the teardown of the >> bundle >> > > descriptor will be performed. >> > > >> > > More detail can be found in [2]. >> > > >> > > As the Approach 2, SDK harness could be shared >> > > between multiple executable stages. The >> control >> > > service termination only occurs when all the >> > > executable stages sharing the same SDK harness >> > > finished. This means that the teardown of >> DoFns >> > > may not be executed immediately after an >> > > executable stage is finished. >> > > >> > > So, I prefer Approach 1. Welcome any feedback >> :) >> > > >> > > Best, >> > > Jincheng >> > > >> > > [1] >> > > >> https://github.com/apache/beam/blob/master/sdks/python/apache_beam/runners/worker/sdk_worker.py >> > > [2] >> > > >> https://docs.google.com/document/d/1sCgy9VQPf9zVXKRquK8P6N4x7aB62GEO8ozkujRSHZg/edit?usp=sharing >> > > >> > > -- >> > > >> > > Got feedback? go/harsh-feedback >> <https://goto.google.com/harsh-feedback> >> > > <https://goto.google.com/harsh-feedback> >> > > >> >
