You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Chris Westin <ch...@gmail.com> on 2015/03/04 01:21:32 UTC
Re: Review Request 31435: DRILL-2245-core: core changes for improved
drillbit stability
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31435/
-----------------------------------------------------------
(Updated March 3, 2015, 4:21 p.m.)
Review request for drill and Jacques Nadeau.
Changes
-------
Some small changes including comments, TODO(DRILL-XXXX) entries, and some try-catch and try-finally blocks.
Bugs: DRILL-2245
https://issues.apache.org/jira/browse/DRILL-2245
Repository: drill-git
Description
-------
DRILL-2245-core: Clean up query setup and execution kickoff in Foreman/WorkManager in order to ensure consistent handling, and avoid hangs and races, with the goal of improving Drillbit robustness.
I did my best to keep these clean when I split them up, but this core commit
may depend on some minor changes in the hygiene commit that is also
associated with this bug, so either both should be applied, or neither.
The core commit should be applied first.
AutoCloseables
- created org.apache.drill.common.AutoCloseables to handle closing these
quietly
BaseTestQuery, and derivatives
- factored out pieces into QueryTestUtil so they can be reused
Drillbit
- uses AutoCloseables for the WorkManager and for the storeProvider
- allow start() to take a RemoteServiceSet
- private, final, formatting
Foreman
- does not need to implement Comparable
- does not need to implement Closeable
- thread blocking fixes
- add resultSent flag
- add code to log plan fragments with endpoint assignments
- added finals, cleaned up formatting
- do queue management in acquireQuerySemaphore; local tests pass
- rename getContext() to getQueryContext()
- retain DrillbitContext
- a couple of exception injections for testing
- minor formatting
- TODOs
FragmentExecutor
- eliminated CancelableQuery
- common subexpression elimination
- cleaned up
QueryContext
- removed unnecessary functions (with some outside classes tweaked for this)
- finals, formatting
QueryManager
- merge in QueryStatus
- affects Foreman, ../batch/ControlHandlerImpl,
and ../../server/rest/ProfileResources
- made some methods private
- removed unused imports
- add finals and formatting
- variable renaming to improve readability
- formatting
- comments
- TODOs
QueryStatus
- getAsInfo() private
- member renaming
- member access changes
- formatting
- TODOs
QueryTestUtil, BaseTestQuery, TestDrillbitResilience
- make maxWidth a parameter to server startup
SelfCleaningRunnable
- created org.apache.drill.common.SelfCleaningRunnable
SingleRowListener
- created org.apache.drill.SingleRowListener results listener
- use in TestDrillbitResilience
TestDrillbitResilience
- created org.apache.drill.exec.server.TestDrillbitResilience to test drillb
resilience in the face of exceptions and failures during queries
TestWithZookeeper
- factor out work into ZookeeperHelper so that it can be reused by
TestDrillbitResilience
UserBitShared
- get rid of unused UNKNOWN_QUERY
WorkEventBus
- rename methods, affects Foreman and ControlHandlerImpl
- remove unused WorkerBee reference
- most members final
- formatting
WorkManager
- Closeable to AutoCloseable
- removed unused incomingFragments Set
- eliminated unnecessary eventThread and pendingTasks by posting Runnables
directly to executor
- use SelfCleaningRunnable for Foreman management
- FragmentExecutor management uses SelfCleaningRunnable
- runningFragments to be a ConcurrentHashMap; TestTpchDistributed passes
- other improvements due to bee no longer needed in various places
- most members final
- minor formatting
- comments
- TODOs
(*) Created exception injection classes to simulate exceptions for testing
- ExceptionInjection
- ExceptionInjector
- ExceptionInjectionUtil
- TestExceptionInjection
Diffs (updated)
-----
common/src/main/java/org/apache/drill/common/AutoCloseables.java PRE-CREATION
common/src/main/java/org/apache/drill/common/SelfCleaningRunnable.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java 5efcce8
exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java 04b955b
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/rpc/control/WorkEventBus.java d6b8637
exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java 67342c4
exec/java-exec/src/main/java/org/apache/drill/exec/server/DrillbitContext.java 83a89df
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java aa0a5ad
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java ae04bad
exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExceptionInjection.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExceptionInjector.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/testing/InjectionSite.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/testing/SimulatedExceptions.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/work/CancelableQuery.java 5b11943
exec/java-exec/src/main/java/org/apache/drill/exec/work/StatusProvider.java 6086f74
exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java 99c6ab8
exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/ControlHandlerImpl.java 3228da9
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 378e81a
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/FragmentData.java 52fd0a9
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/FragmentStatusListener.java 6a719d2
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java 2de3592
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryStatus.java 4e18da6
exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java 7ccb64e
exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/NonRootFragmentManager.java 3671804
exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/RootFragmentManager.java 54fc8c4
exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java cf99577
exec/java-exec/src/test/java/org/apache/drill/PlanTestBase.java c52545d
exec/java-exec/src/test/java/org/apache/drill/QueryTestUtil.java PRE-CREATION
exec/java-exec/src/test/java/org/apache/drill/SingleRowListener.java PRE-CREATION
exec/java-exec/src/test/java/org/apache/drill/TestBugFixes.java 51f3121
exec/java-exec/src/test/java/org/apache/drill/TestBuilder.java 978e565
exec/java-exec/src/test/java/org/apache/drill/exec/DrillSystemTestBase.java 75ba3a9
exec/java-exec/src/test/java/org/apache/drill/exec/TestWithZookeeper.java bb69c9a
exec/java-exec/src/test/java/org/apache/drill/exec/ZookeeperHelper.java PRE-CREATION
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestOptiqPlans.java 4aaaa78
exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java PRE-CREATION
exec/java-exec/src/test/java/org/apache/drill/exec/testing/ExceptionInjectionUtil.java PRE-CREATION
exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestExceptionInjection.java PRE-CREATION
protocol/src/main/protobuf/UserBitShared.proto 8f05c45
Diff: https://reviews.apache.org/r/31435/diff/
Testing
-------
mvn install
Functional - Passing - New
Advanced - TPCH SF100 - Parquet
Thanks,
Chris Westin
Re: Review Request 31435: DRILL-2245-core: core changes for improved
drillbit stability
Posted by Jacques Nadeau <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31435/#review75811
-----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java
<https://reviews.apache.org/r/31435/#comment123117>
The web ui presents what is stored in the query profile. So the query profile needs to store a cancelled outcome. Profile storage is here: https://github.com/apache/drill/blob/fc58c693ae0cd9234e5e62a7602b4cc216b8549d/exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryStatus.java#L143
Web UI presentatio is here:
https://github.com/apache/drill/blob/master/exec/java-exec/src/main/resources/rest/profile/list.ftl#L50
This is not dependent on any interaction with rpc client.
- Jacques Nadeau
On March 4, 2015, 12:21 a.m., Chris Westin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31435/
> -----------------------------------------------------------
>
> (Updated March 4, 2015, 12:21 a.m.)
>
>
> Review request for drill and Jacques Nadeau.
>
>
> Bugs: DRILL-2245
> https://issues.apache.org/jira/browse/DRILL-2245
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> DRILL-2245-core: Clean up query setup and execution kickoff in Foreman/WorkManager in order to ensure consistent handling, and avoid hangs and races, with the goal of improving Drillbit robustness.
>
> I did my best to keep these clean when I split them up, but this core commit
> may depend on some minor changes in the hygiene commit that is also
> associated with this bug, so either both should be applied, or neither.
> The core commit should be applied first.
>
> AutoCloseables
> - created org.apache.drill.common.AutoCloseables to handle closing these
> quietly
>
> BaseTestQuery, and derivatives
> - factored out pieces into QueryTestUtil so they can be reused
>
> Drillbit
> - uses AutoCloseables for the WorkManager and for the storeProvider
> - allow start() to take a RemoteServiceSet
> - private, final, formatting
>
> Foreman
> - does not need to implement Comparable
> - does not need to implement Closeable
> - thread blocking fixes
> - add resultSent flag
> - add code to log plan fragments with endpoint assignments
> - added finals, cleaned up formatting
> - do queue management in acquireQuerySemaphore; local tests pass
> - rename getContext() to getQueryContext()
> - retain DrillbitContext
> - a couple of exception injections for testing
> - minor formatting
> - TODOs
>
> FragmentExecutor
> - eliminated CancelableQuery
> - common subexpression elimination
> - cleaned up
>
> QueryContext
> - removed unnecessary functions (with some outside classes tweaked for this)
> - finals, formatting
>
> QueryManager
> - merge in QueryStatus
> - affects Foreman, ../batch/ControlHandlerImpl,
> and ../../server/rest/ProfileResources
> - made some methods private
> - removed unused imports
> - add finals and formatting
> - variable renaming to improve readability
> - formatting
> - comments
> - TODOs
>
> QueryStatus
> - getAsInfo() private
> - member renaming
> - member access changes
> - formatting
> - TODOs
>
> QueryTestUtil, BaseTestQuery, TestDrillbitResilience
> - make maxWidth a parameter to server startup
>
> SelfCleaningRunnable
> - created org.apache.drill.common.SelfCleaningRunnable
>
> SingleRowListener
> - created org.apache.drill.SingleRowListener results listener
> - use in TestDrillbitResilience
>
> TestDrillbitResilience
> - created org.apache.drill.exec.server.TestDrillbitResilience to test drillb
> resilience in the face of exceptions and failures during queries
>
> TestWithZookeeper
> - factor out work into ZookeeperHelper so that it can be reused by
> TestDrillbitResilience
>
> UserBitShared
> - get rid of unused UNKNOWN_QUERY
>
> WorkEventBus
> - rename methods, affects Foreman and ControlHandlerImpl
> - remove unused WorkerBee reference
> - most members final
> - formatting
>
> WorkManager
> - Closeable to AutoCloseable
> - removed unused incomingFragments Set
> - eliminated unnecessary eventThread and pendingTasks by posting Runnables
> directly to executor
> - use SelfCleaningRunnable for Foreman management
> - FragmentExecutor management uses SelfCleaningRunnable
> - runningFragments to be a ConcurrentHashMap; TestTpchDistributed passes
> - other improvements due to bee no longer needed in various places
> - most members final
> - minor formatting
> - comments
> - TODOs
>
> (*) Created exception injection classes to simulate exceptions for testing
> - ExceptionInjection
> - ExceptionInjector
> - ExceptionInjectionUtil
> - TestExceptionInjection
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/drill/common/AutoCloseables.java PRE-CREATION
> common/src/main/java/org/apache/drill/common/SelfCleaningRunnable.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java 5efcce8
> exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java 04b955b
> 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/rpc/control/WorkEventBus.java d6b8637
> exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java 67342c4
> exec/java-exec/src/main/java/org/apache/drill/exec/server/DrillbitContext.java 83a89df
> exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java aa0a5ad
> exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java ae04bad
> exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExceptionInjection.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExceptionInjector.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/testing/InjectionSite.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/testing/SimulatedExceptions.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/work/CancelableQuery.java 5b11943
> exec/java-exec/src/main/java/org/apache/drill/exec/work/StatusProvider.java 6086f74
> exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java 99c6ab8
> exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/ControlHandlerImpl.java 3228da9
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 378e81a
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/FragmentData.java 52fd0a9
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/FragmentStatusListener.java 6a719d2
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java 2de3592
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryStatus.java 4e18da6
> exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java 7ccb64e
> exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/NonRootFragmentManager.java 3671804
> exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/RootFragmentManager.java 54fc8c4
> exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java cf99577
> exec/java-exec/src/test/java/org/apache/drill/PlanTestBase.java c52545d
> exec/java-exec/src/test/java/org/apache/drill/QueryTestUtil.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/SingleRowListener.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/TestBugFixes.java 51f3121
> exec/java-exec/src/test/java/org/apache/drill/TestBuilder.java 978e565
> exec/java-exec/src/test/java/org/apache/drill/exec/DrillSystemTestBase.java 75ba3a9
> exec/java-exec/src/test/java/org/apache/drill/exec/TestWithZookeeper.java bb69c9a
> exec/java-exec/src/test/java/org/apache/drill/exec/ZookeeperHelper.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestOptiqPlans.java 4aaaa78
> exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/exec/testing/ExceptionInjectionUtil.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestExceptionInjection.java PRE-CREATION
> protocol/src/main/protobuf/UserBitShared.proto 8f05c45
>
> Diff: https://reviews.apache.org/r/31435/diff/
>
>
> Testing
> -------
>
> mvn install
> Functional - Passing - New
> Advanced - TPCH SF100 - Parquet
>
>
> Thanks,
>
> Chris Westin
>
>
Re: Review Request 31435: DRILL-2245-core: core changes for improved
drillbit stability
Posted by Chris Westin <ch...@gmail.com>.
> On March 9, 2015, 3:11 p.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java, line 659
> > <https://reviews.apache.org/r/31435/diff/1/?file=876396#file876396line659>
> >
> > I'm speaking about the Web UI rather than the client. If you cancel a query, then it should show up as cancelled in the WebUI. Currently, it does. With this change, it wouldn't.
In my attempts to write automated testing, I discovered that the query state doesn't make it back to the client (DrillClient). Isn't the web UI using that also? Please provide a pointer to this UI client code so that I can take a look at it.
- Chris
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31435/#review75796
-----------------------------------------------------------
On March 3, 2015, 4:21 p.m., Chris Westin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31435/
> -----------------------------------------------------------
>
> (Updated March 3, 2015, 4:21 p.m.)
>
>
> Review request for drill and Jacques Nadeau.
>
>
> Bugs: DRILL-2245
> https://issues.apache.org/jira/browse/DRILL-2245
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> DRILL-2245-core: Clean up query setup and execution kickoff in Foreman/WorkManager in order to ensure consistent handling, and avoid hangs and races, with the goal of improving Drillbit robustness.
>
> I did my best to keep these clean when I split them up, but this core commit
> may depend on some minor changes in the hygiene commit that is also
> associated with this bug, so either both should be applied, or neither.
> The core commit should be applied first.
>
> AutoCloseables
> - created org.apache.drill.common.AutoCloseables to handle closing these
> quietly
>
> BaseTestQuery, and derivatives
> - factored out pieces into QueryTestUtil so they can be reused
>
> Drillbit
> - uses AutoCloseables for the WorkManager and for the storeProvider
> - allow start() to take a RemoteServiceSet
> - private, final, formatting
>
> Foreman
> - does not need to implement Comparable
> - does not need to implement Closeable
> - thread blocking fixes
> - add resultSent flag
> - add code to log plan fragments with endpoint assignments
> - added finals, cleaned up formatting
> - do queue management in acquireQuerySemaphore; local tests pass
> - rename getContext() to getQueryContext()
> - retain DrillbitContext
> - a couple of exception injections for testing
> - minor formatting
> - TODOs
>
> FragmentExecutor
> - eliminated CancelableQuery
> - common subexpression elimination
> - cleaned up
>
> QueryContext
> - removed unnecessary functions (with some outside classes tweaked for this)
> - finals, formatting
>
> QueryManager
> - merge in QueryStatus
> - affects Foreman, ../batch/ControlHandlerImpl,
> and ../../server/rest/ProfileResources
> - made some methods private
> - removed unused imports
> - add finals and formatting
> - variable renaming to improve readability
> - formatting
> - comments
> - TODOs
>
> QueryStatus
> - getAsInfo() private
> - member renaming
> - member access changes
> - formatting
> - TODOs
>
> QueryTestUtil, BaseTestQuery, TestDrillbitResilience
> - make maxWidth a parameter to server startup
>
> SelfCleaningRunnable
> - created org.apache.drill.common.SelfCleaningRunnable
>
> SingleRowListener
> - created org.apache.drill.SingleRowListener results listener
> - use in TestDrillbitResilience
>
> TestDrillbitResilience
> - created org.apache.drill.exec.server.TestDrillbitResilience to test drillb
> resilience in the face of exceptions and failures during queries
>
> TestWithZookeeper
> - factor out work into ZookeeperHelper so that it can be reused by
> TestDrillbitResilience
>
> UserBitShared
> - get rid of unused UNKNOWN_QUERY
>
> WorkEventBus
> - rename methods, affects Foreman and ControlHandlerImpl
> - remove unused WorkerBee reference
> - most members final
> - formatting
>
> WorkManager
> - Closeable to AutoCloseable
> - removed unused incomingFragments Set
> - eliminated unnecessary eventThread and pendingTasks by posting Runnables
> directly to executor
> - use SelfCleaningRunnable for Foreman management
> - FragmentExecutor management uses SelfCleaningRunnable
> - runningFragments to be a ConcurrentHashMap; TestTpchDistributed passes
> - other improvements due to bee no longer needed in various places
> - most members final
> - minor formatting
> - comments
> - TODOs
>
> (*) Created exception injection classes to simulate exceptions for testing
> - ExceptionInjection
> - ExceptionInjector
> - ExceptionInjectionUtil
> - TestExceptionInjection
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/drill/common/AutoCloseables.java PRE-CREATION
> common/src/main/java/org/apache/drill/common/SelfCleaningRunnable.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java 5efcce8
> exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java 04b955b
> 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/rpc/control/WorkEventBus.java d6b8637
> exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java 67342c4
> exec/java-exec/src/main/java/org/apache/drill/exec/server/DrillbitContext.java 83a89df
> exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java aa0a5ad
> exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java ae04bad
> exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExceptionInjection.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExceptionInjector.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/testing/InjectionSite.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/testing/SimulatedExceptions.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/work/CancelableQuery.java 5b11943
> exec/java-exec/src/main/java/org/apache/drill/exec/work/StatusProvider.java 6086f74
> exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java 99c6ab8
> exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/ControlHandlerImpl.java 3228da9
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 378e81a
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/FragmentData.java 52fd0a9
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/FragmentStatusListener.java 6a719d2
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java 2de3592
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryStatus.java 4e18da6
> exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java 7ccb64e
> exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/NonRootFragmentManager.java 3671804
> exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/RootFragmentManager.java 54fc8c4
> exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java cf99577
> exec/java-exec/src/test/java/org/apache/drill/PlanTestBase.java c52545d
> exec/java-exec/src/test/java/org/apache/drill/QueryTestUtil.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/SingleRowListener.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/TestBugFixes.java 51f3121
> exec/java-exec/src/test/java/org/apache/drill/TestBuilder.java 978e565
> exec/java-exec/src/test/java/org/apache/drill/exec/DrillSystemTestBase.java 75ba3a9
> exec/java-exec/src/test/java/org/apache/drill/exec/TestWithZookeeper.java bb69c9a
> exec/java-exec/src/test/java/org/apache/drill/exec/ZookeeperHelper.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestOptiqPlans.java 4aaaa78
> exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/exec/testing/ExceptionInjectionUtil.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestExceptionInjection.java PRE-CREATION
> protocol/src/main/protobuf/UserBitShared.proto 8f05c45
>
> Diff: https://reviews.apache.org/r/31435/diff/
>
>
> Testing
> -------
>
> mvn install
> Functional - Passing - New
> Advanced - TPCH SF100 - Parquet
>
>
> Thanks,
>
> Chris Westin
>
>
Re: Review Request 31435: DRILL-2245-core: core changes for improved
drillbit stability
Posted by Jacques Nadeau <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31435/#review75796
-----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java
<https://reviews.apache.org/r/31435/#comment123086>
I'm speaking about the Web UI rather than the client. If you cancel a query, then it should show up as cancelled in the WebUI. Currently, it does. With this change, it wouldn't.
exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java
<https://reviews.apache.org/r/31435/#comment123089>
We can enter this code in multiple scenarios. One is if the query succeeded, another when the query failed. If the body of the query succeeded and then we failed to clean up, we should propogate that back to the end user. However, if the query already failed and then we fail to cleanup resources, the primary thing we should propogate to the user is that query failed, not the cleanup problem. I propose that to make this cleaner, we update the failure cause and FragmentContext to be able to add multiple exceptions via suppressed exceptions. That way we can follow the approach of first exception is root cause, subsequent exceptions are added as suppressed exceptions. We should do that across the code over time.
- Jacques Nadeau
On March 4, 2015, 12:21 a.m., Chris Westin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31435/
> -----------------------------------------------------------
>
> (Updated March 4, 2015, 12:21 a.m.)
>
>
> Review request for drill and Jacques Nadeau.
>
>
> Bugs: DRILL-2245
> https://issues.apache.org/jira/browse/DRILL-2245
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> DRILL-2245-core: Clean up query setup and execution kickoff in Foreman/WorkManager in order to ensure consistent handling, and avoid hangs and races, with the goal of improving Drillbit robustness.
>
> I did my best to keep these clean when I split them up, but this core commit
> may depend on some minor changes in the hygiene commit that is also
> associated with this bug, so either both should be applied, or neither.
> The core commit should be applied first.
>
> AutoCloseables
> - created org.apache.drill.common.AutoCloseables to handle closing these
> quietly
>
> BaseTestQuery, and derivatives
> - factored out pieces into QueryTestUtil so they can be reused
>
> Drillbit
> - uses AutoCloseables for the WorkManager and for the storeProvider
> - allow start() to take a RemoteServiceSet
> - private, final, formatting
>
> Foreman
> - does not need to implement Comparable
> - does not need to implement Closeable
> - thread blocking fixes
> - add resultSent flag
> - add code to log plan fragments with endpoint assignments
> - added finals, cleaned up formatting
> - do queue management in acquireQuerySemaphore; local tests pass
> - rename getContext() to getQueryContext()
> - retain DrillbitContext
> - a couple of exception injections for testing
> - minor formatting
> - TODOs
>
> FragmentExecutor
> - eliminated CancelableQuery
> - common subexpression elimination
> - cleaned up
>
> QueryContext
> - removed unnecessary functions (with some outside classes tweaked for this)
> - finals, formatting
>
> QueryManager
> - merge in QueryStatus
> - affects Foreman, ../batch/ControlHandlerImpl,
> and ../../server/rest/ProfileResources
> - made some methods private
> - removed unused imports
> - add finals and formatting
> - variable renaming to improve readability
> - formatting
> - comments
> - TODOs
>
> QueryStatus
> - getAsInfo() private
> - member renaming
> - member access changes
> - formatting
> - TODOs
>
> QueryTestUtil, BaseTestQuery, TestDrillbitResilience
> - make maxWidth a parameter to server startup
>
> SelfCleaningRunnable
> - created org.apache.drill.common.SelfCleaningRunnable
>
> SingleRowListener
> - created org.apache.drill.SingleRowListener results listener
> - use in TestDrillbitResilience
>
> TestDrillbitResilience
> - created org.apache.drill.exec.server.TestDrillbitResilience to test drillb
> resilience in the face of exceptions and failures during queries
>
> TestWithZookeeper
> - factor out work into ZookeeperHelper so that it can be reused by
> TestDrillbitResilience
>
> UserBitShared
> - get rid of unused UNKNOWN_QUERY
>
> WorkEventBus
> - rename methods, affects Foreman and ControlHandlerImpl
> - remove unused WorkerBee reference
> - most members final
> - formatting
>
> WorkManager
> - Closeable to AutoCloseable
> - removed unused incomingFragments Set
> - eliminated unnecessary eventThread and pendingTasks by posting Runnables
> directly to executor
> - use SelfCleaningRunnable for Foreman management
> - FragmentExecutor management uses SelfCleaningRunnable
> - runningFragments to be a ConcurrentHashMap; TestTpchDistributed passes
> - other improvements due to bee no longer needed in various places
> - most members final
> - minor formatting
> - comments
> - TODOs
>
> (*) Created exception injection classes to simulate exceptions for testing
> - ExceptionInjection
> - ExceptionInjector
> - ExceptionInjectionUtil
> - TestExceptionInjection
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/drill/common/AutoCloseables.java PRE-CREATION
> common/src/main/java/org/apache/drill/common/SelfCleaningRunnable.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java 5efcce8
> exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java 04b955b
> 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/rpc/control/WorkEventBus.java d6b8637
> exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java 67342c4
> exec/java-exec/src/main/java/org/apache/drill/exec/server/DrillbitContext.java 83a89df
> exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java aa0a5ad
> exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java ae04bad
> exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExceptionInjection.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExceptionInjector.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/testing/InjectionSite.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/testing/SimulatedExceptions.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/work/CancelableQuery.java 5b11943
> exec/java-exec/src/main/java/org/apache/drill/exec/work/StatusProvider.java 6086f74
> exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java 99c6ab8
> exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/ControlHandlerImpl.java 3228da9
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 378e81a
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/FragmentData.java 52fd0a9
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/FragmentStatusListener.java 6a719d2
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java 2de3592
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryStatus.java 4e18da6
> exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java 7ccb64e
> exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/NonRootFragmentManager.java 3671804
> exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/RootFragmentManager.java 54fc8c4
> exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java cf99577
> exec/java-exec/src/test/java/org/apache/drill/PlanTestBase.java c52545d
> exec/java-exec/src/test/java/org/apache/drill/QueryTestUtil.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/SingleRowListener.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/TestBugFixes.java 51f3121
> exec/java-exec/src/test/java/org/apache/drill/TestBuilder.java 978e565
> exec/java-exec/src/test/java/org/apache/drill/exec/DrillSystemTestBase.java 75ba3a9
> exec/java-exec/src/test/java/org/apache/drill/exec/TestWithZookeeper.java bb69c9a
> exec/java-exec/src/test/java/org/apache/drill/exec/ZookeeperHelper.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestOptiqPlans.java 4aaaa78
> exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/exec/testing/ExceptionInjectionUtil.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestExceptionInjection.java PRE-CREATION
> protocol/src/main/protobuf/UserBitShared.proto 8f05c45
>
> Diff: https://reviews.apache.org/r/31435/diff/
>
>
> Testing
> -------
>
> mvn install
> Functional - Passing - New
> Advanced - TPCH SF100 - Parquet
>
>
> Thanks,
>
> Chris Westin
>
>
Re: Review Request 31435: DRILL-2245-core: core changes for improved
drillbit stability
Posted by Jacques Nadeau <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31435/#review77103
-----------------------------------------------------------
Ship it!
Ship It!
- Jacques Nadeau
On March 19, 2015, 8:21 p.m., Chris Westin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31435/
> -----------------------------------------------------------
>
> (Updated March 19, 2015, 8:21 p.m.)
>
>
> Review request for drill, Jacques Nadeau and Jason Altekruse.
>
>
> Bugs: DRILL-2245
> https://issues.apache.org/jira/browse/DRILL-2245
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> DRILL-2245-core: Clean up query setup and execution kickoff in Foreman/WorkManager in order to ensure consistent handling, and avoid hangs and races, with the goal of improving Drillbit robustness.
>
> I did my best to keep these clean when I split them up, but this core commit
> may depend on some minor changes in the hygiene commit that is also
> associated with this bug, so either both should be applied, or neither.
> The core commit should be applied first.
>
> AutoCloseables
> - created org.apache.drill.common.AutoCloseables to handle closing these
> quietly
>
> BaseTestQuery, and derivatives
> - factored out pieces into QueryTestUtil so they can be reused
>
> Drillbit
> - uses AutoCloseables for the WorkManager and for the storeProvider
> - allow start() to take a RemoteServiceSet
> - private, final, formatting
>
> Foreman
> - does not need to implement Comparable
> - does not need to implement Closeable
> - thread blocking fixes
> - add resultSent flag
> - add code to log plan fragments with endpoint assignments
> - added finals, cleaned up formatting
> - do queue management in acquireQuerySemaphore; local tests pass
> - rename getContext() to getQueryContext()
> - retain DrillbitContext
> - a couple of exception injections for testing
> - minor formatting
> - TODOs
>
> FragmentExecutor
> - eliminated CancelableQuery
> - common subexpression elimination
> - cleaned up
>
> QueryContext
> - removed unnecessary functions (with some outside classes tweaked for this)
> - finals, formatting
>
> QueryManager
> - merge in QueryStatus
> - affects Foreman, ../batch/ControlHandlerImpl,
> and ../../server/rest/ProfileResources
> - made some methods private
> - removed unused imports
> - add finals and formatting
> - variable renaming to improve readability
> - formatting
> - comments
> - TODOs
>
> QueryStatus
> - getAsInfo() private
> - member renaming
> - member access changes
> - formatting
> - TODOs
>
> QueryTestUtil, BaseTestQuery, TestDrillbitResilience
> - make maxWidth a parameter to server startup
>
> SelfCleaningRunnable
> - created org.apache.drill.common.SelfCleaningRunnable
>
> SingleRowListener
> - created org.apache.drill.SingleRowListener results listener
> - use in TestDrillbitResilience
>
> TestDrillbitResilience
> - created org.apache.drill.exec.server.TestDrillbitResilience to test drillb
> resilience in the face of exceptions and failures during queries
>
> TestWithZookeeper
> - factor out work into ZookeeperHelper so that it can be reused by
> TestDrillbitResilience
>
> UserBitShared
> - get rid of unused UNKNOWN_QUERY
>
> WorkEventBus
> - rename methods, affects Foreman and ControlHandlerImpl
> - remove unused WorkerBee reference
> - most members final
> - formatting
>
> WorkManager
> - Closeable to AutoCloseable
> - removed unused incomingFragments Set
> - eliminated unnecessary eventThread and pendingTasks by posting Runnables
> directly to executor
> - use SelfCleaningRunnable for Foreman management
> - FragmentExecutor management uses SelfCleaningRunnable
> - runningFragments to be a ConcurrentHashMap; TestTpchDistributed passes
> - other improvements due to bee no longer needed in various places
> - most members final
> - minor formatting
> - comments
> - TODOs
>
> (*) Created exception injection classes to simulate exceptions for testing
> - ExceptionInjection
> - ExceptionInjector
> - ExceptionInjectionUtil
> - TestExceptionInjection
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/drill/common/AutoCloseables.java PRE-CREATION
> common/src/main/java/org/apache/drill/common/DeferredException.java PRE-CREATION
> common/src/main/java/org/apache/drill/common/SelfCleaningRunnable.java PRE-CREATION
> common/src/main/java/org/apache/drill/common/config/DrillConfig.java e8b2478
> exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java 93d06f0
> exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java 04b955b
> exec/java-exec/src/main/java/org/apache/drill/exec/compile/ClassTransformer.java 5c65724
> 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/ops/FragmentContext.java 2a6660e
> exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java fb6b4aa
> exec/java-exec/src/main/java/org/apache/drill/exec/opt/BasicOptimizer.java 9f89f24
> exec/java-exec/src/main/java/org/apache/drill/exec/opt/IdentityOptimizer.java 979c5e2
> exec/java-exec/src/main/java/org/apache/drill/exec/opt/Optimizer.java 34d0622
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BaseRootExec.java 412da85
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/RootExec.java a644c34
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java d884200
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SendingAccountor.java 3920f9c
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/materialize/QueryWritableBatch.java e6c3fba
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java 352e7ae
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/producer/ProducerConsumerBatch.java 4c9b33b
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/Fragment.java 2436a0e
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/SimpleParallelizer.java eebd40e
> exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SetOptionHandler.java b5d3f4a
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/BasicClient.java f358097
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/BasicServer.java c00df4e
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/NamedThreadFactory.java 2b49579
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/ReconnectingConnection.java f214c4d
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RemoteConnection.java 3a139f8
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java 5eab16a
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/control/Controller.java 7f84a2b
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/control/ControllerImpl.java f8f6fd7
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/control/WorkEventBus.java d6b8637
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/data/DataConnectionCreator.java 33f0d09
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/data/DataResponseHandlerImpl.java e0392fd
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/data/DataTunnel.java 5aa4aa6
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java 4e7fc92
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserServer.java dffb9a1
> exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java 6521303
> exec/java-exec/src/main/java/org/apache/drill/exec/server/DrillbitContext.java 1b35aec
> exec/java-exec/src/main/java/org/apache/drill/exec/server/options/DrillConfigIterator.java 5c2aede
> exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SessionOptionManager.java 45c0ce8
> exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java fa4725d
> exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java ae04bad
> exec/java-exec/src/main/java/org/apache/drill/exec/service/ServiceEngine.java ff6e13c
> exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/local/FilePStore.java baa998d
> exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExceptionInjection.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExceptionInjector.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/testing/InjectionSite.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/testing/SimulatedExceptions.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/work/CancelableQuery.java 5b11943
> exec/java-exec/src/main/java/org/apache/drill/exec/work/QueryWorkUnit.java 9743d6e
> exec/java-exec/src/main/java/org/apache/drill/exec/work/StatusProvider.java 6086f74
> exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java 125c56d
> exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/ControlHandlerImpl.java 3228da9
> exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/SpoolingRawBatchBuffer.java f0b4983
> exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/UnlimitedRawBatchBuffer.java 3d5b948
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/DrillbitStatusListener.java ca52f0c
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 8e0780b
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/FragmentData.java 52fd0a9
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/FragmentStatusListener.java 6a719d2
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java 2de3592
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryStatus.java 4e18da6
> exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java e2f7bbf
> exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/NonRootFragmentManager.java 3671804
> exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/RootFragmentManager.java 54fc8c4
> exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java 775bccb
> exec/java-exec/src/test/java/org/apache/drill/PlanTestBase.java 36091af
> exec/java-exec/src/test/java/org/apache/drill/QueryTestUtil.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/SingleRowListener.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/TestBuilder.java 978e565
> exec/java-exec/src/test/java/org/apache/drill/TestTpchDistributed.java e0f830d
> exec/java-exec/src/test/java/org/apache/drill/exec/DrillSystemTestBase.java 75ba3a9
> exec/java-exec/src/test/java/org/apache/drill/exec/TestWithZookeeper.java bb69c9a
> exec/java-exec/src/test/java/org/apache/drill/exec/ZookeeperHelper.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/SimpleRootExec.java 0277876
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestComparisonFunctions.java 609bc14
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestOptiqPlans.java 2a0aedc
> exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/exec/testing/ExceptionInjectionUtil.java PRE-CREATION
> exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestExceptionInjection.java PRE-CREATION
> exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java 2c51ec0
> exec/jdbc/src/test/java/org/apache/drill/jdbc/test/JdbcAssert.java b88d880
> protocol/pom.xml 8c3e9b0
> protocol/src/main/java/org/apache/drill/common/types/TypeProtos.java 74ac444
> protocol/src/main/java/org/apache/drill/exec/proto/BitControl.java 813d961
> protocol/src/main/java/org/apache/drill/exec/proto/BitData.java 5de0a07
> protocol/src/main/java/org/apache/drill/exec/proto/CoordinationProtos.java 177e560
> protocol/src/main/java/org/apache/drill/exec/proto/ExecProtos.java 7ca17f1
> protocol/src/main/java/org/apache/drill/exec/proto/GeneralRPCProtos.java f47e719
> protocol/src/main/java/org/apache/drill/exec/proto/SchemaDefProtos.java d7f3536
> protocol/src/main/java/org/apache/drill/exec/proto/UserBitShared.java 5682a3b
> protocol/src/main/java/org/apache/drill/exec/proto/UserProtos.java 048bd20
> protocol/src/main/java/org/apache/drill/exec/proto/beans/QueryResult.java d8841e8
> protocol/src/main/protobuf/UserBitShared.proto 8f05c45
>
> Diff: https://reviews.apache.org/r/31435/diff/
>
>
> Testing
> -------
>
> mvn install
> Functional - Passing - New
> Advanced - TPCH SF100 - Parquet
>
>
> Thanks,
>
> Chris Westin
>
>
Re: Review Request 31435: DRILL-2245-core: core changes for improved
drillbit stability
Posted by Chris Westin <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31435/
-----------------------------------------------------------
(Updated March 19, 2015, 1:21 p.m.)
Review request for drill, Jacques Nadeau and Jason Altekruse.
Changes
-------
Addressed Jacques last review comments. It should only be necessary to review DeferredException (new), FragmentContext and FragmentExecutor for the first, and Foreman (Foreman.ForemanResult and Foreman.moveToState()) for the second.
Bugs: DRILL-2245
https://issues.apache.org/jira/browse/DRILL-2245
Repository: drill-git
Description
-------
DRILL-2245-core: Clean up query setup and execution kickoff in Foreman/WorkManager in order to ensure consistent handling, and avoid hangs and races, with the goal of improving Drillbit robustness.
I did my best to keep these clean when I split them up, but this core commit
may depend on some minor changes in the hygiene commit that is also
associated with this bug, so either both should be applied, or neither.
The core commit should be applied first.
AutoCloseables
- created org.apache.drill.common.AutoCloseables to handle closing these
quietly
BaseTestQuery, and derivatives
- factored out pieces into QueryTestUtil so they can be reused
Drillbit
- uses AutoCloseables for the WorkManager and for the storeProvider
- allow start() to take a RemoteServiceSet
- private, final, formatting
Foreman
- does not need to implement Comparable
- does not need to implement Closeable
- thread blocking fixes
- add resultSent flag
- add code to log plan fragments with endpoint assignments
- added finals, cleaned up formatting
- do queue management in acquireQuerySemaphore; local tests pass
- rename getContext() to getQueryContext()
- retain DrillbitContext
- a couple of exception injections for testing
- minor formatting
- TODOs
FragmentExecutor
- eliminated CancelableQuery
- common subexpression elimination
- cleaned up
QueryContext
- removed unnecessary functions (with some outside classes tweaked for this)
- finals, formatting
QueryManager
- merge in QueryStatus
- affects Foreman, ../batch/ControlHandlerImpl,
and ../../server/rest/ProfileResources
- made some methods private
- removed unused imports
- add finals and formatting
- variable renaming to improve readability
- formatting
- comments
- TODOs
QueryStatus
- getAsInfo() private
- member renaming
- member access changes
- formatting
- TODOs
QueryTestUtil, BaseTestQuery, TestDrillbitResilience
- make maxWidth a parameter to server startup
SelfCleaningRunnable
- created org.apache.drill.common.SelfCleaningRunnable
SingleRowListener
- created org.apache.drill.SingleRowListener results listener
- use in TestDrillbitResilience
TestDrillbitResilience
- created org.apache.drill.exec.server.TestDrillbitResilience to test drillb
resilience in the face of exceptions and failures during queries
TestWithZookeeper
- factor out work into ZookeeperHelper so that it can be reused by
TestDrillbitResilience
UserBitShared
- get rid of unused UNKNOWN_QUERY
WorkEventBus
- rename methods, affects Foreman and ControlHandlerImpl
- remove unused WorkerBee reference
- most members final
- formatting
WorkManager
- Closeable to AutoCloseable
- removed unused incomingFragments Set
- eliminated unnecessary eventThread and pendingTasks by posting Runnables
directly to executor
- use SelfCleaningRunnable for Foreman management
- FragmentExecutor management uses SelfCleaningRunnable
- runningFragments to be a ConcurrentHashMap; TestTpchDistributed passes
- other improvements due to bee no longer needed in various places
- most members final
- minor formatting
- comments
- TODOs
(*) Created exception injection classes to simulate exceptions for testing
- ExceptionInjection
- ExceptionInjector
- ExceptionInjectionUtil
- TestExceptionInjection
Diffs (updated)
-----
common/src/main/java/org/apache/drill/common/AutoCloseables.java PRE-CREATION
common/src/main/java/org/apache/drill/common/DeferredException.java PRE-CREATION
common/src/main/java/org/apache/drill/common/SelfCleaningRunnable.java PRE-CREATION
common/src/main/java/org/apache/drill/common/config/DrillConfig.java e8b2478
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java 93d06f0
exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java 04b955b
exec/java-exec/src/main/java/org/apache/drill/exec/compile/ClassTransformer.java 5c65724
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/ops/FragmentContext.java 2a6660e
exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java fb6b4aa
exec/java-exec/src/main/java/org/apache/drill/exec/opt/BasicOptimizer.java 9f89f24
exec/java-exec/src/main/java/org/apache/drill/exec/opt/IdentityOptimizer.java 979c5e2
exec/java-exec/src/main/java/org/apache/drill/exec/opt/Optimizer.java 34d0622
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BaseRootExec.java 412da85
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/RootExec.java a644c34
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java d884200
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SendingAccountor.java 3920f9c
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/materialize/QueryWritableBatch.java e6c3fba
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java 352e7ae
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/producer/ProducerConsumerBatch.java 4c9b33b
exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/Fragment.java 2436a0e
exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/SimpleParallelizer.java eebd40e
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SetOptionHandler.java b5d3f4a
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/BasicClient.java f358097
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/BasicServer.java c00df4e
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/NamedThreadFactory.java 2b49579
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/ReconnectingConnection.java f214c4d
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RemoteConnection.java 3a139f8
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java 5eab16a
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/control/Controller.java 7f84a2b
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/control/ControllerImpl.java f8f6fd7
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/control/WorkEventBus.java d6b8637
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/data/DataConnectionCreator.java 33f0d09
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/data/DataResponseHandlerImpl.java e0392fd
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/data/DataTunnel.java 5aa4aa6
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java 4e7fc92
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserServer.java dffb9a1
exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java 6521303
exec/java-exec/src/main/java/org/apache/drill/exec/server/DrillbitContext.java 1b35aec
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/DrillConfigIterator.java 5c2aede
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SessionOptionManager.java 45c0ce8
exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java fa4725d
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/profile/ProfileResources.java ae04bad
exec/java-exec/src/main/java/org/apache/drill/exec/service/ServiceEngine.java ff6e13c
exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/local/FilePStore.java baa998d
exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExceptionInjection.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/testing/ExceptionInjector.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/testing/InjectionSite.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/testing/SimulatedExceptions.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/work/CancelableQuery.java 5b11943
exec/java-exec/src/main/java/org/apache/drill/exec/work/QueryWorkUnit.java 9743d6e
exec/java-exec/src/main/java/org/apache/drill/exec/work/StatusProvider.java 6086f74
exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java 125c56d
exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/ControlHandlerImpl.java 3228da9
exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/SpoolingRawBatchBuffer.java f0b4983
exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/UnlimitedRawBatchBuffer.java 3d5b948
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/DrillbitStatusListener.java ca52f0c
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 8e0780b
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/FragmentData.java 52fd0a9
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/FragmentStatusListener.java 6a719d2
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java 2de3592
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryStatus.java 4e18da6
exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java e2f7bbf
exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/NonRootFragmentManager.java 3671804
exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/RootFragmentManager.java 54fc8c4
exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java 775bccb
exec/java-exec/src/test/java/org/apache/drill/PlanTestBase.java 36091af
exec/java-exec/src/test/java/org/apache/drill/QueryTestUtil.java PRE-CREATION
exec/java-exec/src/test/java/org/apache/drill/SingleRowListener.java PRE-CREATION
exec/java-exec/src/test/java/org/apache/drill/TestBuilder.java 978e565
exec/java-exec/src/test/java/org/apache/drill/TestTpchDistributed.java e0f830d
exec/java-exec/src/test/java/org/apache/drill/exec/DrillSystemTestBase.java 75ba3a9
exec/java-exec/src/test/java/org/apache/drill/exec/TestWithZookeeper.java bb69c9a
exec/java-exec/src/test/java/org/apache/drill/exec/ZookeeperHelper.java PRE-CREATION
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/SimpleRootExec.java 0277876
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestComparisonFunctions.java 609bc14
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestOptiqPlans.java 2a0aedc
exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java PRE-CREATION
exec/java-exec/src/test/java/org/apache/drill/exec/testing/ExceptionInjectionUtil.java PRE-CREATION
exec/java-exec/src/test/java/org/apache/drill/exec/testing/TestExceptionInjection.java PRE-CREATION
exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java 2c51ec0
exec/jdbc/src/test/java/org/apache/drill/jdbc/test/JdbcAssert.java b88d880
protocol/pom.xml 8c3e9b0
protocol/src/main/java/org/apache/drill/common/types/TypeProtos.java 74ac444
protocol/src/main/java/org/apache/drill/exec/proto/BitControl.java 813d961
protocol/src/main/java/org/apache/drill/exec/proto/BitData.java 5de0a07
protocol/src/main/java/org/apache/drill/exec/proto/CoordinationProtos.java 177e560
protocol/src/main/java/org/apache/drill/exec/proto/ExecProtos.java 7ca17f1
protocol/src/main/java/org/apache/drill/exec/proto/GeneralRPCProtos.java f47e719
protocol/src/main/java/org/apache/drill/exec/proto/SchemaDefProtos.java d7f3536
protocol/src/main/java/org/apache/drill/exec/proto/UserBitShared.java 5682a3b
protocol/src/main/java/org/apache/drill/exec/proto/UserProtos.java 048bd20
protocol/src/main/java/org/apache/drill/exec/proto/beans/QueryResult.java d8841e8
protocol/src/main/protobuf/UserBitShared.proto 8f05c45
Diff: https://reviews.apache.org/r/31435/diff/
Testing
-------
mvn install
Functional - Passing - New
Advanced - TPCH SF100 - Parquet
Thanks,
Chris Westin