You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Venki Korukanti <ve...@gmail.com> on 2015/05/09 18:40:23 UTC

Review Request 34014: DRILL-2755: Use and handle InterruptedException during query processing

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

Review request for drill, Chris Westin and Jacques Nadeau.


Bugs: DRILL-2755
    https://issues.apache.org/jira/browse/DRILL-2755


Repository: drill-git


Description
-------

DRILL-2755: Use and handle InterruptedException during query processing.

    - Interrupt FragmentExecutor thread as part of FragmentExecutor.cancel()
    - Handle InterruptedException in ExternalSortBatch.newSV2(). If the fragment status says
      should not continue, then throw the InterruptedException to caller which returns IterOutcome.STOP
    - Add comments reg not handling of InterruptedException in SendingAccountor.waitForSendComplete()
    - Handle InterruptedException in OrderedPartitionRecordBatch.getPartitionVectors()
    - If interrupted in Thread.sleep calls and fragment status says should not run, then
      return IterOutcome.STOP downstream.
    - Interrupt partitioner threads if PartitionerRecordBatch is interrupted while waiting for
      partitioner threads to complete.
    - Preserve interrupt status if not handled
    - Handle null RecordBatches returned by RawBatchBuffer.getNext() in MergingRecordBatch.buildSchema()
    - Change timeout in Foreman to be proportional to the number of intermediate fragments sent instead
      of hard coded limit of 90s.
    - Change TimedRunnable to enforce a timeout of 15s per runnable.
      Total timeout is (5s * numOfRunnableTasks) / parallelism.


Diffs
-----

  pom.xml 8c9f09e 

Diff: https://reviews.apache.org/r/34014/diff/


Testing
-------

- Add unit tests
      * Testing cancelling a query interrupts the query fragments which are currently blocked
      * Testing interrupting the partitioner sender which in turn interrupts its helper threads
      * Testing TimedRunanble enforeces timeout for the whole task list.


Thanks,

Venki Korukanti


Re: Review Request 34014: DRILL-2755: Use and handle InterruptedException during query processing

Posted by Venki Korukanti <ve...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34014/
-----------------------------------------------------------

(Updated May 9, 2015, 5:42 p.m.)


Review request for drill, Chris Westin and Jacques Nadeau.


Changes
-------

* addressed review comments
* fixed potential concurrency issue with myThread variable in FragmentExecutor


Bugs: DRILL-2755
    https://issues.apache.org/jira/browse/DRILL-2755


Repository: drill-git


Description
-------

DRILL-2755: Use and handle InterruptedException during query processing.

    - Interrupt FragmentExecutor thread as part of FragmentExecutor.cancel()
    - Handle InterruptedException in ExternalSortBatch.newSV2(). If the fragment status says
      should not continue, then throw the InterruptedException to caller which returns IterOutcome.STOP
    - Add comments reg not handling of InterruptedException in SendingAccountor.waitForSendComplete()
    - Handle InterruptedException in OrderedPartitionRecordBatch.getPartitionVectors()
    - If interrupted in Thread.sleep calls and fragment status says should not run, then
      return IterOutcome.STOP downstream.
    - Interrupt partitioner threads if PartitionerRecordBatch is interrupted while waiting for
      partitioner threads to complete.
    - Preserve interrupt status if not handled
    - Handle null RecordBatches returned by RawBatchBuffer.getNext() in MergingRecordBatch.buildSchema()
    - Change timeout in Foreman to be proportional to the number of intermediate fragments sent instead
      of hard coded limit of 90s.
    - Change TimedRunnable to enforce a timeout of 15s per runnable.
      Total timeout is (5s * numOfRunnableTasks) / parallelism.


Diffs (updated)
-----

  common/src/main/java/org/apache/drill/common/concurrent/ExtendedLatch.java a75ac32 
  exec/java-exec/src/main/java/org/apache/drill/exec/ops/SendingAccountor.java 0cb5fbf 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java ce683cb 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java 63b7eba 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java c3261dc 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java cbea267 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverBatch.java 66a2092 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java e88bc67 
  exec/java-exec/src/main/java/org/apache/drill/exec/record/RawFragmentBatchProvider.java d4dfe96 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/BasicClient.java 1661f81 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/BasicServer.java a148436 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/ReconnectingConnection.java 9948d3e 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RemoteConnection.java 0f095c0 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/data/DataTunnel.java 11f5496 
  exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java e7a9a3c 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/TimedRunnable.java 0fb778b 
  exec/java-exec/src/main/java/org/apache/drill/exec/testing/CountDownLatchInjection.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/testing/CountDownLatchInjectionImpl.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExecutionControlsInjector.java 4b1cd0c 
  exec/java-exec/src/main/java/org/apache/drill/exec/testing/NoOpControlsInjector.java 80d9790 
  exec/java-exec/src/main/java/org/apache/drill/exec/testing/PauseInjection.java e5f9c9c 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java a3ceb8f 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/SpoolingRawBatchBuffer.java 2a79e42 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/UnlimitedRawBatchBuffer.java d23655c 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 49d0c94 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java ddb828c 
  exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java da69e9e 
  exec/java-exec/src/test/java/org/apache/drill/exec/store/TestTimedRunnable.java PRE-CREATION 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java 8ef2af3 

Diff: https://reviews.apache.org/r/34014/diff/


Testing
-------

- Add unit tests
      * Testing cancelling a query interrupts the query fragments which are currently blocked
      * Testing interrupting the partitioner sender which in turn interrupts its helper threads
      * Testing TimedRunanble enforeces timeout for the whole task list.


Thanks,

Venki Korukanti


Re: Review Request 34014: DRILL-2755: Use and handle InterruptedException during query processing

Posted by Jacques Nadeau <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34014/#review83164
-----------------------------------------------------------



exec/java-exec/src/main/java/org/apache/drill/exec/store/TimedRunnable.java
<https://reviews.apache.org/r/34014/#comment134042>

    don't we need threadPool.awaitTermination(timeout) here?



exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java
<https://reviews.apache.org/r/34014/#comment134043>

    I think we need to remove the loop.  Can you ask Chris why he added?  This is probably while Drill doesn't exit in so many situations.


- Jacques Nadeau


On May 9, 2015, 4:41 p.m., Venki Korukanti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34014/
> -----------------------------------------------------------
> 
> (Updated May 9, 2015, 4:41 p.m.)
> 
> 
> Review request for drill, Chris Westin and Jacques Nadeau.
> 
> 
> Bugs: DRILL-2755
>     https://issues.apache.org/jira/browse/DRILL-2755
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> DRILL-2755: Use and handle InterruptedException during query processing.
> 
>     - Interrupt FragmentExecutor thread as part of FragmentExecutor.cancel()
>     - Handle InterruptedException in ExternalSortBatch.newSV2(). If the fragment status says
>       should not continue, then throw the InterruptedException to caller which returns IterOutcome.STOP
>     - Add comments reg not handling of InterruptedException in SendingAccountor.waitForSendComplete()
>     - Handle InterruptedException in OrderedPartitionRecordBatch.getPartitionVectors()
>     - If interrupted in Thread.sleep calls and fragment status says should not run, then
>       return IterOutcome.STOP downstream.
>     - Interrupt partitioner threads if PartitionerRecordBatch is interrupted while waiting for
>       partitioner threads to complete.
>     - Preserve interrupt status if not handled
>     - Handle null RecordBatches returned by RawBatchBuffer.getNext() in MergingRecordBatch.buildSchema()
>     - Change timeout in Foreman to be proportional to the number of intermediate fragments sent instead
>       of hard coded limit of 90s.
>     - Change TimedRunnable to enforce a timeout of 15s per runnable.
>       Total timeout is (5s * numOfRunnableTasks) / parallelism.
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/drill/common/concurrent/ExtendedLatch.java a75ac32 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/SendingAccountor.java 0cb5fbf 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java ce683cb 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java 63b7eba 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java c3261dc 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java cbea267 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverBatch.java 66a2092 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java e88bc67 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/RawFragmentBatchProvider.java d4dfe96 
>   exec/java-exec/src/main/java/org/apache/drill/exec/rpc/BasicClient.java 72ae130 
>   exec/java-exec/src/main/java/org/apache/drill/exec/rpc/BasicServer.java 3a7032b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/rpc/ReconnectingConnection.java 9948d3e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RemoteConnection.java a72dd32 
>   exec/java-exec/src/main/java/org/apache/drill/exec/rpc/data/DataTunnel.java 11f5496 
>   exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java e7a9a3c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/TimedRunnable.java 0fb778b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/testing/CountDownLatchInjection.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/testing/CountDownLatchInjectionImpl.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExecutionControlsInjector.java 4b1cd0c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/testing/NoOpControlsInjector.java 80d9790 
>   exec/java-exec/src/main/java/org/apache/drill/exec/testing/PauseInjection.java e5f9c9c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java a3ceb8f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/SpoolingRawBatchBuffer.java 2a79e42 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/UnlimitedRawBatchBuffer.java d23655c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 49d0c94 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java ddb828c 
>   exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java da69e9e 
>   exec/java-exec/src/test/java/org/apache/drill/exec/store/TestTimedRunnable.java PRE-CREATION 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java 8ef2af3 
> 
> Diff: https://reviews.apache.org/r/34014/diff/
> 
> 
> Testing
> -------
> 
> - Add unit tests
>       * Testing cancelling a query interrupts the query fragments which are currently blocked
>       * Testing interrupting the partitioner sender which in turn interrupts its helper threads
>       * Testing TimedRunanble enforeces timeout for the whole task list.
> 
> 
> Thanks,
> 
> Venki Korukanti
> 
>


Re: Review Request 34014: DRILL-2755: Use and handle InterruptedException during query processing

Posted by Venki Korukanti <ve...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34014/
-----------------------------------------------------------

(Updated May 9, 2015, 4:41 p.m.)


Review request for drill, Chris Westin and Jacques Nadeau.


Changes
-------

Uploading the actual path


Bugs: DRILL-2755
    https://issues.apache.org/jira/browse/DRILL-2755


Repository: drill-git


Description
-------

DRILL-2755: Use and handle InterruptedException during query processing.

    - Interrupt FragmentExecutor thread as part of FragmentExecutor.cancel()
    - Handle InterruptedException in ExternalSortBatch.newSV2(). If the fragment status says
      should not continue, then throw the InterruptedException to caller which returns IterOutcome.STOP
    - Add comments reg not handling of InterruptedException in SendingAccountor.waitForSendComplete()
    - Handle InterruptedException in OrderedPartitionRecordBatch.getPartitionVectors()
    - If interrupted in Thread.sleep calls and fragment status says should not run, then
      return IterOutcome.STOP downstream.
    - Interrupt partitioner threads if PartitionerRecordBatch is interrupted while waiting for
      partitioner threads to complete.
    - Preserve interrupt status if not handled
    - Handle null RecordBatches returned by RawBatchBuffer.getNext() in MergingRecordBatch.buildSchema()
    - Change timeout in Foreman to be proportional to the number of intermediate fragments sent instead
      of hard coded limit of 90s.
    - Change TimedRunnable to enforce a timeout of 15s per runnable.
      Total timeout is (5s * numOfRunnableTasks) / parallelism.


Diffs (updated)
-----

  common/src/main/java/org/apache/drill/common/concurrent/ExtendedLatch.java a75ac32 
  exec/java-exec/src/main/java/org/apache/drill/exec/ops/SendingAccountor.java 0cb5fbf 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java ce683cb 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java 63b7eba 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java c3261dc 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java cbea267 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverBatch.java 66a2092 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java e88bc67 
  exec/java-exec/src/main/java/org/apache/drill/exec/record/RawFragmentBatchProvider.java d4dfe96 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/BasicClient.java 72ae130 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/BasicServer.java 3a7032b 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/ReconnectingConnection.java 9948d3e 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RemoteConnection.java a72dd32 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/data/DataTunnel.java 11f5496 
  exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java e7a9a3c 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/TimedRunnable.java 0fb778b 
  exec/java-exec/src/main/java/org/apache/drill/exec/testing/CountDownLatchInjection.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/testing/CountDownLatchInjectionImpl.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExecutionControlsInjector.java 4b1cd0c 
  exec/java-exec/src/main/java/org/apache/drill/exec/testing/NoOpControlsInjector.java 80d9790 
  exec/java-exec/src/main/java/org/apache/drill/exec/testing/PauseInjection.java e5f9c9c 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java a3ceb8f 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/SpoolingRawBatchBuffer.java 2a79e42 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/UnlimitedRawBatchBuffer.java d23655c 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 49d0c94 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java ddb828c 
  exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java da69e9e 
  exec/java-exec/src/test/java/org/apache/drill/exec/store/TestTimedRunnable.java PRE-CREATION 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java 8ef2af3 

Diff: https://reviews.apache.org/r/34014/diff/


Testing
-------

- Add unit tests
      * Testing cancelling a query interrupts the query fragments which are currently blocked
      * Testing interrupting the partitioner sender which in turn interrupts its helper threads
      * Testing TimedRunanble enforeces timeout for the whole task list.


Thanks,

Venki Korukanti