You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by OpenInx <op...@gmail.com> on 2018/11/28 14:42:36 UTC
The child procedure is scheduled in the reversed order in Procedure
V2 Framework
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.
> > > >
> > >
> >
>
Re: The child procedure is scheduled in the reversed order in
Procedure V2 Framework
Posted by Misty Linville <mi...@apache.org>.
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 OpenInx <op...@gmail.com>.
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 Allan Yang <al...@apache.org>.
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.
>