GEODE-245: QueryMonitor cancellation is being ignored by query using CompactRangeIndex Fixed QueryMonitor test hook to pause at the correct point in code
Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/57d84860 Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/57d84860 Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/57d84860 Branch: refs/heads/feature/GEODE-217 Commit: 57d8486061c827a349edd7f86aa1eac91d45b668 Parents: d302af5 Author: Jason Huynh <huyn...@gmail.com> Authored: Wed Jan 6 09:55:49 2016 -0800 Committer: Jason Huynh <huyn...@gmail.com> Committed: Fri Jan 8 13:29:33 2016 -0800 ---------------------------------------------------------------------- .../cache/query/internal/DefaultQuery.java | 8 ++++-- .../cache/query/internal/QueryMonitor.java | 26 +++----------------- .../query/internal/index/CompactRangeIndex.java | 2 ++ 3 files changed, 12 insertions(+), 24 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/57d84860/gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/DefaultQuery.java ---------------------------------------------------------------------- diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/DefaultQuery.java b/gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/DefaultQuery.java index 9419d04..eb6948c 100644 --- a/gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/DefaultQuery.java +++ b/gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/DefaultQuery.java @@ -360,6 +360,7 @@ public class DefaultQuery implements Query { if (DefaultQuery.testHook != null) { DefaultQuery.testHook.doTestHook(1); } + result = qe.executeQuery(this, parameters, null); // For local queries returning pdx objects wrap the resultset with ResultsCollectionPdxDeserializerWrapper // which deserializes these pdx objects. @@ -382,6 +383,7 @@ public class DefaultQuery implements Query { // starts on the Local Buckets. queryMonitor.monitorQueryThread(Thread.currentThread(), this); } + context.setCqQueryContext(this.isCqQuery); result = executeUsingContext(context); //Only wrap/copy results when copy on read is set and an index is used @@ -552,7 +554,6 @@ public class DefaultQuery implements Query { public Object executeUsingContext(ExecutionContext context) throws FunctionDomainException, TypeMismatchException, NameResolutionException, QueryInvocationTargetException { QueryObserver observer = QueryObserverHolder.getInstance(); - long startTime = CachePerfStats.getStatTime(); TXStateProxy tx = null; if (!((GemFireCacheImpl)this.cache).isClient()) { // fixes bug 42294 @@ -562,6 +563,10 @@ public class DefaultQuery implements Query { observer.startQuery(this); observer.beforeQueryEvaluation(compiledQuery, context); Object results = null; + + if (DefaultQuery.testHook != null) { + DefaultQuery.testHook.doTestHook(6); + } try { // two-pass evaluation. // first pre-compute dependencies, cached in the context. @@ -990,7 +995,6 @@ public class DefaultQuery implements Query { public Object execute(RegionFunctionContext context, Object[] parameters) throws FunctionDomainException, TypeMismatchException, NameResolutionException, QueryInvocationTargetException { - Object result = null; // Supported only with RegionFunctionContext http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/57d84860/gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/QueryMonitor.java ---------------------------------------------------------------------- diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/QueryMonitor.java b/gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/QueryMonitor.java index 65a64a4..6843576 100755 --- a/gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/QueryMonitor.java +++ b/gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/QueryMonitor.java @@ -130,35 +130,19 @@ public class QueryMonitor implements Runnable { long maxTimeSet = GemFireCacheImpl.getInstance().TEST_MAX_QUERY_EXECUTION_TIME; QueryThreadTask queryTask = (QueryThreadTask)queryThreads.peek(); + long currentTime = System.currentTimeMillis(); + // This is to check if the QueryMonitoring thread slept longer than the expected time. // Its seen that in some cases based on OS thread scheduling the thread can sleep much // longer than the specified time. if (queryTask != null) { - if ((System.currentTimeMillis() - queryTask.StartTime) > maxTimeSet){ + if ((currentTime - queryTask.StartTime) > maxTimeSet){ // The sleep() is unpredictable. testException = new QueryExecutionTimeoutException("The QueryMonitor thread may be sleeping longer than" + " the set sleep time. This will happen as the sleep is based on OS thread scheduling," + " verify the time spent by the executor thread."); } } - - // This is to see if query finished before it could get canceled, this could happen because - // of a faster machine. - // Check if the query finished before the max_query_execution time. - queryTask = (QueryThreadTask)this.queryMonitorTasks.get(queryThread); - if (queryTask != null){ - this.queryMonitorTasks.remove(queryThread); - if (!GemFireCacheImpl.getInstance().TEST_MAX_QUERY_EXECUTION_TIME_OVERRIDE_EXCEPTION && testException == null && ((System.currentTimeMillis() - queryTask.StartTime) < - (maxTimeSet + 10 /* 10ms buffer */))){ - testException = new QueryExecutionTimeoutException("The Query completed sucessfully before it got canceled."); - } - } - - if ((testException == null) && (query instanceof DefaultQuery)) { - if (((DefaultQuery)query).isCanceled()) { - testException = new QueryExecutionTimeoutException("The query task could not be found but the query is marked as having been canceled"); - } - } } // END - DUnit Test purpose. @@ -233,9 +217,7 @@ public class QueryMonitor implements Runnable { } continue; } - if (DefaultQuery.testHook != null) { - DefaultQuery.testHook.doTestHook(6); - } + long currentTime = System.currentTimeMillis(); // Check if the sleepTime is greater than the remaining query execution time. http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/57d84860/gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/index/CompactRangeIndex.java ---------------------------------------------------------------------- diff --git a/gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/index/CompactRangeIndex.java b/gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/index/CompactRangeIndex.java index 79d0c54..13c8a8e 100644 --- a/gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/index/CompactRangeIndex.java +++ b/gemfire-core/src/main/java/com/gemstone/gemfire/cache/query/internal/index/CompactRangeIndex.java @@ -767,6 +767,8 @@ public class CompactRangeIndex extends AbstractIndex { while (entriesIter.hasNext()) { try { + // Check if query execution on this thread is canceled. + QueryMonitor.isQueryExecutionCanceled(); if (IndexManager.testHook != null) { if (this.region.getCache().getLogger().fineEnabled()) { this.region