drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "abdelhakim deneche" <adene...@gmail.com>
Subject Re: Review Request 33115: DRILL-2762: Update Fragment state reporting and error collection
Date Tue, 14 Apr 2015 04:49:05 GMT

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



exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java
<https://reviews.apache.org/r/33115/#comment129709>

    finishedNodes <= finishedNodes
    
    shouldn't this be:
    
    finishedNodes <= totalNodes ?



exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java
<https://reviews.apache.org/r/33115/#comment129710>

    unnecessary semicolon



exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java
<https://reviews.apache.org/r/33115/#comment129707>

    unused import



exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java
<https://reviews.apache.org/r/33115/#comment129706>

    updateState() is not used anywhere, should we just remove it ?



exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java
<https://reviews.apache.org/r/33115/#comment129708>

    iff -> if


- abdelhakim deneche


On April 13, 2015, 12:07 a.m., Jacques Nadeau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33115/
> -----------------------------------------------------------
> 
> (Updated April 13, 2015, 12:07 a.m.)
> 
> 
> Review request for drill, abdelhakim deneche, Chris Westin, and Steven Phillips.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> DRILL-2762: Update Fragment state reporting and error collection
> 
> DeferredException
> - Add new throwAndClear operation on to allow checking for exceptions preClose in FragmentContext
> 
> BufferManager
> - Ensure close() can be called multiple times by clearing managed buffer list on close().
> 
> FragmentContext/FragmentExecutor
> - Update FragmentContext to have a preClose so that we can check closure state before
doing final close.
> - Clean up FragmentExecutor run() method to better manage error states and have only
single terminal point (avoiding multiple messages to Foreman).
> 
> WorkManager Updates
> - Add new afterExecute command to the WorkManager ExecutorService so that we get log
entries if a thread leaks an exception. (Otherwise logs don't show these exceptions and they
only go to standard out.)
> - Change status thread so it only reports non-terminal status to the Foreman to avoid
race conditions and confusion when receiving multiple possibly incomplete terminal messages.
> 
> Foreman/QueryManager
> - Extract listenable interfaces into anonymous inner classes from body of Foreman
> 
> QueryManager
> - Update QueryManager to track completed nodes rather than completed fragments using
NodeTracker
> - Update DrillbitStatusListener to decrement expected completion messages on Nodes that
have died to avoid query hang when a node dies
> 
> FragmentData/MinorFragmentProfile
> - Add ability to track last status update as well as last time fragment made progress
> 
> AbstractRecordBatch
> - Update awareness of current cancellation state to avoid cancellation delays
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/drill/common/DeferredException.java 99f18f1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/BufferManager.java 2d22d84 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java da2229c

>   exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractRecordBatch.java
2bb29e5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java e2bcec3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 23ef0d3

>   exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/FragmentData.java 433ab26

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

>   exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java
a7e6c46 
>   protocol/src/main/java/org/apache/drill/exec/proto/SchemaUserBitShared.java f72d5e1

>   protocol/src/main/java/org/apache/drill/exec/proto/UserBitShared.java ac1bcbb 
>   protocol/src/main/java/org/apache/drill/exec/proto/beans/MinorFragmentProfile.java
5cd71f9 
>   protocol/src/main/protobuf/UserBitShared.proto 2938114 
> 
> Diff: https://reviews.apache.org/r/33115/diff/
> 
> 
> Testing
> -------
> 
> Regression & Unit, more manual testing planned before final patch.
> 
> 
> Thanks,
> 
> Jacques Nadeau
> 
>


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