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/cc67eddb Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/cc67eddb Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/cc67eddb Branch: refs/heads/feature/GEODE-2017 Commit: cc67eddb6c385bf5f87db0dae488bf0b0a5a8d5d Parents: fdf6329 Author: Dan Smith <[email protected]> Authored: Mon Oct 31 14:15:47 2016 -0700 Committer: Udo Kohlmeyer <[email protected]> Committed: Tue Nov 8 05:39:40 2016 +1100 ---------------------------------------------------------------------- .../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/cc67eddb/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 7ccf3d6..e307c86 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/cc67eddb/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);
