Thanks for updating the Flip. It looks good to me. +1 (binding)
Cheers, Till On Mon, Sep 23, 2019 at 4:12 PM Xintong Song <tonysong...@gmail.com> wrote: > @Till @Andrey > > According to the comments, I just updated the FLIP document [1], with the > following changes: > > - Remove SlotID (in the section Protocol Changes) > - Updated implementation steps to reduce separated code paths. As far as > I can see at the moment, we do not need the feature option. We can add > it > if later we find it necessary in the implementation. > > > Thank you~ > > Xintong Song > > > [1] > > https://cwiki.apache.org/confluence/display/FLINK/FLIP-56%3A+Dynamic+Slot+Allocation > > On Fri, Sep 20, 2019 at 11:01 AM Xintong Song <tonysong...@gmail.com> > wrote: > > > I'm not sure if I understand the implementation plan you suggested > > correctly. To my understanding, it seems that all the steps except for > step > > 5 have to happen in strict order. > > > > - Profiles to be used in step 2 is reported with step 1. > > - SlotProfile in TaskExecutorGateway#requestSlot in step 3 comes from > > profiles used in step 2. > > - Only if RM request slots from TM with profiles (step 3), would TM be > > able to do the proper bookkeeping (step 4) > > - Step 5 can be done as long as we have step 2. > > - Step 6 relies on both step 4 and step 5, for proper bookkeepings on > > both TM and RM sides before enabling non-default profiles. > > > > That means we can only work on the steps in the following order. > > 1-2-3-4-6 > > \-5-/ > > > > What I'm trying to achieve with the current plan, is to have most of the > > implementation steps paralleled, as the following. So that Andrey and I > can > > work concurrently without blocking each other too much. > > 1-2-3-4 > > \5-6-7 > > > > > > I also agree that it would be good to not add too much separate codes. I > > would suggest leave that decision to the implementation time. E.g., if by > > the time we do the TM side bookkeeping, the RM side has already > implemented > > requesting slots with profiles, then we do not need to separate the code > > paths. > > > > > > To that end, I think it makes sense to adjust step 5-7 to first use > > default slot resource profiles for all the bookkeepings, and replace it > > with the requested profiles at the end. > > > > > > What do you think? > > > > > > Thank you~ > > > > Xintong Song > > > > > > > > On Thu, Sep 19, 2019 at 7:59 PM Till Rohrmann <trohrm...@apache.org> > > wrote: > > > >> I think besides of point 1. and 3. there are no dependencies between the > >> RM > >> and TM side changes. Also, I'm not sure whether it makes sense to split > >> the > >> slot manager changes up into the proposed steps 5, 6 and 7. > >> > >> I would highly recommend to not add too much duplicate logic/separate > code > >> paths because it just adds blind spots which are probably not as well > >> tested as the old code paths. > >> > >> Cheers, > >> Till > >> > >> On Thu, Sep 19, 2019 at 11:58 AM Xintong Song <tonysong...@gmail.com> > >> wrote: > >> > >> > Thanks for the comments, Till. > >> > > >> > - Agree on removing SlotID. > >> > > >> > - Regarding the implementation plan, it is true that we can possibly > >> reduce > >> > codes separated by the feature option. But I think to do that we need > to > >> > introduce more dependencies between implementation steps. With the > >> current > >> > plan, we can easily separate steps on the RM side and the TM side, and > >> > start concurrently working on them after quickly updating the > >> interfaces in > >> > between. The feature will come alive when the steps on both RM/TM > sides > >> are > >> > finished. Since we are planning to have two persons (Andrey and I) > >> working > >> > on this FLIP, I think the current plan is probably more convenient. > >> > > >> > Thank you~ > >> > > >> > Xintong Song > >> > > >> > > >> > > >> > On Thu, Sep 19, 2019 at 5:09 PM Till Rohrmann <trohrm...@apache.org> > >> > wrote: > >> > > >> > > Hi Xintong, > >> > > > >> > > thanks for starting the vote. The general plan looks good. Hence +1 > >> from > >> > my > >> > > side. I still have some minor comments one could think about: > >> > > > >> > > * As we no longer have predetermined slots on the TaskExecutor, I > >> think > >> > we > >> > > can get rid of the SlotID. Instead, an allocated slot will be > >> identified > >> > by > >> > > the AllocationID and the TaskManager's ResourceID in order to > >> > differentiate > >> > > duplicate registrations. > >> > > * For the implementation plan, I believe there is only one tiny part > >> on > >> > the > >> > > SlotManager for which we need a separate code path/feature flag > which > >> is > >> > > how we find a matching slot. Everything else should be possible to > >> > > implement in a way that it works with dynamic and static slot > >> allocation: > >> > > 1. Let TMs register with default slot profile at RM > >> > > 2. Change SlotManager to use reported slot profiles instead of > >> > > pre-calculated profiles > >> > > 3. Replace SlotID with SlotProfile in > TaskExecutorGateway#requestSlot > >> > > 4. Extend TM to support dynamic slot allocation (aka proper > >> bookkeeping) > >> > > (can happen concurrently to any of steps 2-3) > >> > > 5. Add bookkeeping to SlotManager (for pending TMs and registered > TMs) > >> > but > >> > > still only use default slot profiles for matching with slot requests > >> > > 6. Allow to match slot requests with reported resources instead of > >> > default > >> > > slot profiles (here we could use a feature flag to switch between > >> dynamic > >> > > and static slot allocation) > >> > > > >> > > Wdyt? > >> > > > >> > > Cheers, > >> > > Till > >> > > > >> > > On Thu, Sep 19, 2019 at 9:45 AM Andrey Zagrebin < > azagre...@apache.org > >> > > >> > > wrote: > >> > > > >> > > > Hi Xintong, > >> > > > > >> > > > Thanks for starting the vote, +1 from my side. > >> > > > > >> > > > Best, > >> > > > Andrey > >> > > > > >> > > > On Tue, Sep 17, 2019 at 4:26 PM Xintong Song < > tonysong...@gmail.com > >> > > >> > > > wrote: > >> > > > > >> > > > > Hi all, > >> > > > > > >> > > > > I would like to start the vote for FLIP-56 [1], on which a > >> consensus > >> > is > >> > > > > reached in this discussion thread [2]. > >> > > > > > >> > > > > The vote will be open for at least 72 hours. I'll try to close > it > >> > after > >> > > > > Sep. 20 15:00 UTC, unless there is an objection or not enough > >> votes. > >> > > > > > >> > > > > Thank you~ > >> > > > > > >> > > > > Xintong Song > >> > > > > > >> > > > > > >> > > > > [1] > >> > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > https://cwiki.apache.org/confluence/display/FLINK/FLIP-56%3A+Dynamic+Slot+Allocation > >> > > > > > >> > > > > [2] > >> > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > http://apache-flink-mailing-list-archive.1008284.n3.nabble.com/DISCUSS-FLIP-56-Dynamic-Slot-Allocation-td31960.html > >> > > > > > >> > > > > >> > > > >> > > >> > > >
