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

Reply via email to