-----------------------------------------------------------
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
