You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by Misty Linville <mi...@apache.org> on 2018/12/01 05:55:54 UTC
Re: The child procedure is scheduled in the reversed order in
Procedure V2 Framework
I like your solution in principle, but I think it would be better to loop
until the queue reports that it's empty.
On Wed, Nov 28, 2018, 8:18 PM OpenInx <openinx@gmail.com wrote:
> Thanks Allan's reply
>
> So if we can ensure that all children are independent, then it won't be a
> problem.
> But the reversed shcedule order is some counterintuitive. I think a minor
> change can
> help fix this:
>
> diff --git
>
> a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java
>
> b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java
> index cbdb9b8..b688ec3 100644
> ---
>
> a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java
> +++
>
> b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java
> @@ -1794,7 +1794,7 @@ public class ProcedureExecutor<TEnvironment> {
> }
>
> private void submitChildrenProcedures(Procedure<TEnvironment>[]
> subprocs) {
> - for (int i = 0; i < subprocs.length; ++i) {
> + for (int i = subprocs.length - 1; i >= 0; i--) {
> Procedure<TEnvironment> subproc = subprocs[i];
> subproc.updateMetricsOnSubmit(getEnvironment());
> assert !procedures.containsKey(subproc.getProcId());
>
> Thanks.
>
> On Wed, Nov 28, 2018 at 11:13 PM Allan Yang <al...@apache.org> wrote:
>
> > Yes, you are right, every procedure will be add to front, so the final
> > execution order may be reversed, But actually, there will be more than
> one
> > worker threads, so likely, they will be executed at the same time. I
> think
> > the design is unintentionally, since all the child procedure should be
> > independent and won't depend on each other, so they can be executed at
> any
> > order. And more, after HBASE-21375
> > <https://issues.apache.org/jira/browse/HBASE-21375> checked in all 2.x
> > branch, the worker thread will execute every possible procedure in the
> > queue, so the front ones won't block, so this won't be a problem.
> > Best Regards
> > Allan Yang
> >
> >
> > OpenInx <op...@gmail.com> 于2018年11月28日周三 下午10:42写道：
> >
> > > Hi :
> > >
> > > I read parts of the procedure v2 framework, and found that if a
> > procedure
> > > has 3 added child procedure,
> > > then it's children will be schedued in the reversed order.
> > >
> > > Let me give an example. if a procedure A added 3 child procedure: B,
> C ,
> > > D.
> > >
> > > a.addChildProcedure(B, C, D)
> > >
> > > The procedure framework will add the B,C,D child produre in a dequeue
> to
> > > schedule
> > >
> > > ( Code Path --- ProcedureExecutor#execProcedure
> > > -- submitChildrenProcedures -- dequeue#addFront )
> > >
> > > So the dequeue will be : (front) D, C, B (back)
> > >
> > > Then we will poll each procedure from the dequeue, and dispatch to the
> > > executor to run ..
> > >
> > > In the final, the procedure executing order will be : D, C, B,
> which
> > is
> > > the revered order compared to the addChildProcedure order.
> > >
> > > My question is : is it designed intentionally ? Or unintentionally
> > doing
> > > the wrong implementation ?
> > >
> > > Seems most the child procedure are region assign or unassign, looks
> like
> > no
> > > problem now..
> > >
> > > Please correct me if I am wrong or missing something.
> > >
> > > Thanks.
> > >
> >
>
Re: The child procedure is scheduled in the reversed order in
Procedure V2 Framework
Posted by Misty Linville <mi...@apache.org>.
People could come to rely on the order.
The reason I suggested you should rely on the queue being empty to break
the loop instead of getting the number of elements at the start is to avoid
falling off the end of the queue in the presence of multiple threads.
On Sat, Dec 1, 2018, 1:44 AM 张铎(Duo Zhang) <palomino219@gmail.com wrote:
> I think it is fine to keep the current implementation? What is the problem
> if we schedule in reverse order? We do not guarantee any order when
> executing the sub procedures...
>
> Misty Linville <mi...@apache.org> 于2018年12月1日周六 下午1:56写道：
>
> > I like your solution in principle, but I think it would be better to loop
> > until the queue reports that it's empty.
> >
> > On Wed, Nov 28, 2018, 8:18 PM OpenInx <openinx@gmail.com wrote:
> >
> > > Thanks Allan's reply
> > >
> > > So if we can ensure that all children are independent, then it won't
> be a
> > > problem.
> > > But the reversed shcedule order is some counterintuitive. I think a
> > minor
> > > change can
> > > help fix this:
> > >
> > > diff --git
> > >
> > >
> >
> a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java
> > >
> > >
> >
> b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java
> > > index cbdb9b8..b688ec3 100644
> > > ---
> > >
> > >
> >
> a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java
> > > +++
> > >
> > >
> >
> b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java
> > > @@ -1794,7 +1794,7 @@ public class ProcedureExecutor<TEnvironment> {
> > > }
> > >
> > > private void submitChildrenProcedures(Procedure<TEnvironment>[]
> > > subprocs) {
> > > - for (int i = 0; i < subprocs.length; ++i) {
> > > + for (int i = subprocs.length - 1; i >= 0; i--) {
> > > Procedure<TEnvironment> subproc = subprocs[i];
> > > subproc.updateMetricsOnSubmit(getEnvironment());
> > > assert !procedures.containsKey(subproc.getProcId());
> > >
> > > Thanks.
> > >
> > > On Wed, Nov 28, 2018 at 11:13 PM Allan Yang <al...@apache.org>
> wrote:
> > >
> > > > Yes, you are right, every procedure will be add to front, so the
> final
> > > > execution order may be reversed, But actually, there will be more
> than
> > > one
> > > > worker threads, so likely, they will be executed at the same time. I
> > > think
> > > > the design is unintentionally, since all the child procedure should
> be
> > > > independent and won't depend on each other, so they can be executed
> at
> > > any
> > > > order. And more, after HBASE-21375
> > > > <https://issues.apache.org/jira/browse/HBASE-21375> checked in all
> 2.x
> > > > branch, the worker thread will execute every possible procedure in
> the
> > > > queue, so the front ones won't block, so this won't be a problem.
> > > > Best Regards
> > > > Allan Yang
> > > >
> > > >
> > > > OpenInx <op...@gmail.com> 于2018年11月28日周三 下午10:42写道：
> > > >
> > > > > Hi :
> > > > >
> > > > > I read parts of the procedure v2 framework, and found that if a
> > > > procedure
> > > > > has 3 added child procedure,
> > > > > then it's children will be schedued in the reversed order.
> > > > >
> > > > > Let me give an example. if a procedure A added 3 child procedure:
> B,
> > > C ,
> > > > > D.
> > > > >
> > > > > a.addChildProcedure(B, C, D)
> > > > >
> > > > > The procedure framework will add the B,C,D child produre in a
> dequeue
> > > to
> > > > > schedule
> > > > >
> > > > > ( Code Path --- ProcedureExecutor#execProcedure
> > > > > -- submitChildrenProcedures -- dequeue#addFront )
> > > > >
> > > > > So the dequeue will be : (front) D, C, B (back)
> > > > >
> > > > > Then we will poll each procedure from the dequeue, and dispatch to
> > the
> > > > > executor to run ..
> > > > >
> > > > > In the final, the procedure executing order will be : D, C, B,
> > > which
> > > > is
> > > > > the revered order compared to the addChildProcedure order.
> > > > >
> > > > > My question is : is it designed intentionally ? Or
> unintentionally
> > > > doing
> > > > > the wrong implementation ?
> > > > >
> > > > > Seems most the child procedure are region assign or unassign, looks
> > > like
> > > > no
> > > > > problem now..
> > > > >
> > > > > Please correct me if I am wrong or missing something.
> > > > >
> > > > > Thanks.
> > > > >
> > > >
> > >
> >
>
Re: The child procedure is scheduled in the reversed order in
Procedure V2 Framework
Posted by "张铎 (Duo Zhang)" <pa...@gmail.com>.
I think it is fine to keep the current implementation? What is the problem
if we schedule in reverse order? We do not guarantee any order when
executing the sub procedures...
Misty Linville <mi...@apache.org> 于2018年12月1日周六 下午1:56写道：
> I like your solution in principle, but I think it would be better to loop
> until the queue reports that it's empty.
>
> On Wed, Nov 28, 2018, 8:18 PM OpenInx <openinx@gmail.com wrote:
>
> > Thanks Allan's reply
> >
> > So if we can ensure that all children are independent, then it won't be a
> > problem.
> > But the reversed shcedule order is some counterintuitive. I think a
> minor
> > change can
> > help fix this:
> >
> > diff --git
> >
> >
> a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java
> >
> >
> b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java
> > index cbdb9b8..b688ec3 100644
> > ---
> >
> >
> a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java
> > +++
> >
> >
> b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureExecutor.java
> > @@ -1794,7 +1794,7 @@ public class ProcedureExecutor<TEnvironment> {
> > }
> >
> > private void submitChildrenProcedures(Procedure<TEnvironment>[]
> > subprocs) {
> > - for (int i = 0; i < subprocs.length; ++i) {
> > + for (int i = subprocs.length - 1; i >= 0; i--) {
> > Procedure<TEnvironment> subproc = subprocs[i];
> > subproc.updateMetricsOnSubmit(getEnvironment());
> > assert !procedures.containsKey(subproc.getProcId());
> >
> > Thanks.
> >
> > On Wed, Nov 28, 2018 at 11:13 PM Allan Yang <al...@apache.org> wrote:
> >
> > > Yes, you are right, every procedure will be add to front, so the final
> > > execution order may be reversed, But actually, there will be more than
> > one
> > > worker threads, so likely, they will be executed at the same time. I
> > think
> > > the design is unintentionally, since all the child procedure should be
> > > independent and won't depend on each other, so they can be executed at
> > any
> > > order. And more, after HBASE-21375
> > > <https://issues.apache.org/jira/browse/HBASE-21375> checked in all 2.x
> > > branch, the worker thread will execute every possible procedure in the
> > > queue, so the front ones won't block, so this won't be a problem.
> > > Best Regards
> > > Allan Yang
> > >
> > >
> > > OpenInx <op...@gmail.com> 于2018年11月28日周三 下午10:42写道：
> > >
> > > > Hi :
> > > >
> > > > I read parts of the procedure v2 framework, and found that if a
> > > procedure
> > > > has 3 added child procedure,
> > > > then it's children will be schedued in the reversed order.
> > > >
> > > > Let me give an example. if a procedure A added 3 child procedure: B,
> > C ,
> > > > D.
> > > >
> > > > a.addChildProcedure(B, C, D)
> > > >
> > > > The procedure framework will add the B,C,D child produre in a dequeue
> > to
> > > > schedule
> > > >
> > > > ( Code Path --- ProcedureExecutor#execProcedure
> > > > -- submitChildrenProcedures -- dequeue#addFront )
> > > >
> > > > So the dequeue will be : (front) D, C, B (back)
> > > >
> > > > Then we will poll each procedure from the dequeue, and dispatch to
> the
> > > > executor to run ..
> > > >
> > > > In the final, the procedure executing order will be : D, C, B,
> > which
> > > is
> > > > the revered order compared to the addChildProcedure order.
> > > >
> > > > My question is : is it designed intentionally ? Or unintentionally
> > > doing
> > > > the wrong implementation ?
> > > >
> > > > Seems most the child procedure are region assign or unassign, looks
> > like
> > > no
> > > > problem now..
> > > >
> > > > Please correct me if I am wrong or missing something.
> > > >
> > > > Thanks.
> > > >
> > >
> >
>