drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jinfeng Ni" <...@maprtech.com>
Subject Re: Review Request 27271: DRILL-1592: Detect drillbit node failure and cancel affected running queries
Date Tue, 11 Nov 2014 22:31:53 GMT


> On Nov. 10, 2014, 6:11 p.m., Aditya Kishore wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/coord/ClusterCoordinator.java,
lines 81-91
> > <https://reviews.apache.org/r/27271/diff/6/?file=757417#file757417line81>
> >
> >     To avoid ambiguity from the event names (registered, unregistered), would you
mind changing the function names to `addDrillbitStatusListener()` and `removeDrillbitStatusListener()`.

changed the function name.


> On Nov. 10, 2014, 6:11 p.m., Aditya Kishore wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/DrillbitStatusListener.java,
lines 36-42
> > <https://reviews.apache.org/r/27271/diff/6/?file=757422#file757422line36>
> >
> >     Thanks for separating the events out.
> >     
> >     However instead of calling these function for a set of drillbits, I think we
should make one call for each individual drillbit that joins or leaves the cluster.
> >     
> >     Somthing like
> >     
> >     ```
> >     public void drillbitUnregistered(CoordinationProtos.DrillbitEndpoint unregisteredDrillbit);
> >     
> >     public void drillbitRegistered(CoordinationProtos.DrillbitEndpoint registeredDrillbit);
> >     
> >     ```
> >     
> >     What do you think?

Thanks for the suggestion. 

The ZKClusterCoordinator gets the active drillbits status from ServiceCaches, which provides
a set of active drillbits. I think it may be fine to pass in the set of unregistered/registered
drillbit, and let the listener decide the appropriate actions to take, upon receiving such
set of drillbits. For now, I'll keep it. We could modify the interface, if later on the listener
has to take action based on one individual drillbit.


- Jinfeng


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27271/#review60731
-----------------------------------------------------------


On Nov. 10, 2014, 5:20 p.m., Jinfeng Ni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27271/
> -----------------------------------------------------------
> 
> (Updated Nov. 10, 2014, 5:20 p.m.)
> 
> 
> Review request for drill and Jacques Nadeau.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> Use cluster's ZK to keep track the available drillbit node. Whenever there is a node
change detected by ClusterCoordinator, such change will be notified to either Foreman or the
non-root Fragment's FragmentExecutor. The notification would lead to drillbit to cancel the
affected queries.
> 
> Basic design is to register a DrillbitStatusListener with ClusterCoordinator. QueryManager
or FragmentExecutor will implemented a different DrilbitStatusListener. 
> 1. For Foreman's DrillbitStatusListener, it will check if the active drillbits still
contain all the drillbit runnning the queries. If not, send cancel requet to non-root fragments.
> 2. For Non-root fragment's DrillbitStatusListener, it will check if the foreman drillbit
is in the active drillbit. If not, cancel itself.
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/coord/ClusterCoordinator.java 508a5b2

>   exec/java-exec/src/main/java/org/apache/drill/exec/coord/local/LocalClusterCoordinator.java
035c1aa 
>   exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKClusterCoordinator.java
7f538d2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java 9b78c1d

>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java ea48b05 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/DrillbitStatusListener.java
PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 0979f34

>   exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java b200edc

>   exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java
37074d8 
> 
> Diff: https://reviews.apache.org/r/27271/diff/
> 
> 
> Testing
> -------
> 
> Unit test. 
> 
> functional / TPCH SF100. 
> 
> 
> Thanks,
> 
> Jinfeng Ni
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message