drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Chris Westin" <chriswesti...@gmail.com>
Subject Review Request 31435: DRILL-2245-core: core changes for improved drillbit stability
Date Wed, 25 Feb 2015 21:42:54 GMT

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

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


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