GEODE-1985: Updating the SAFE_QUERY_TIME after updating indexes This is a fix for pretty specific race condition 1) T1 does a put and gets to the point of calling setIndexBufferTime, but hasn't updated the indexes 2) T2 starts a query and finds the entry in the index even though the value no longer matches the query 3) T1 finishes the put 4) T2 checks to see if should revaluate the entry, but decides not to because based on the SAFE_QUERY_TIME value the entry changed before the query started.
By moving the update to SAFE_QUERY_TIME down, the if the an entry doesn't need reevaluation based on the SAFE_QUERY_TIME, we know the index was updated before the query started. There is currently an updateInProgress flag to handle the issue of the query executing before the SAFE_QUERY_TIME is updated. If the entry is updated, but not the index, the updateInProgress flag will be set. Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/2e1a8c42 Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/2e1a8c42 Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/2e1a8c42 Branch: refs/heads/feature/GEODE-288 Commit: 2e1a8c420fd984bdcc605ababe93b0e6c6b29919 Parents: 4c0d302 Author: Dan Smith <[email protected]> Authored: Mon Oct 31 14:15:47 2016 -0700 Committer: Dan Smith <[email protected]> Committed: Mon Nov 7 09:57:52 2016 -0800 ---------------------------------------------------------------------- .../main/java/org/apache/geode/internal/cache/LocalRegion.java | 3 ++- .../query/internal/index/MapRangeIndexMaintenanceJUnitTest.java | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/2e1a8c42/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java b/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java index 360c6a9..3873e6e 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java @@ -5875,6 +5875,8 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory, } } catch (QueryException e) { throw new IndexMaintenanceException(e); + } finally { + IndexManager.setIndexBufferTime(lastModifiedTime, cacheTimeMillis()); } } } @@ -7369,7 +7371,6 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory, lastModified = cacheTimeMillis(); } entry.updateStatsForPut(lastModified); - IndexManager.setIndexBufferTime(lastModified, cacheTimeMillis()); if (this.statisticsEnabled && !isProxy()) { // do not reschedule if there is already a task in the queue. // this prevents bloat in the TimerTask since cancelled tasks http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/2e1a8c42/geode-core/src/test/java/org/apache/geode/cache/query/internal/index/MapRangeIndexMaintenanceJUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/cache/query/internal/index/MapRangeIndexMaintenanceJUnitTest.java b/geode-core/src/test/java/org/apache/geode/cache/query/internal/index/MapRangeIndexMaintenanceJUnitTest.java index beffe47..5159f89 100644 --- a/geode-core/src/test/java/org/apache/geode/cache/query/internal/index/MapRangeIndexMaintenanceJUnitTest.java +++ b/geode-core/src/test/java/org/apache/geode/cache/query/internal/index/MapRangeIndexMaintenanceJUnitTest.java @@ -968,7 +968,7 @@ public class MapRangeIndexMaintenanceJUnitTest { HashMap map2 = new HashMap(); map2.put("SUN", 1); map2.put("IBM", 2); - p2.positions = map1; + p2.positions = map2; region.put(2, p2);
