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

Reply via email to