Skip to content

Commit 71e49b2

Browse files
committed
GEODE-10453 - in case of REMOVE_DUE_TO_GII_TOMBSTONE_CLEANUP and CompactRangeIndex, specify not to lookup old key, which is very expensive operation. It's actually broken and regression. All the tombstone entries are going to be NullToken and cause class cast exception for every single remove compare if looking up old key. There is no old key during initial tombstone image sync up from lead peer.
1 parent d195814 commit 71e49b2

File tree

4 files changed

+25
-3
lines changed

4 files changed

+25
-3
lines changed

geode-core/src/main/java/org/apache/geode/cache/query/internal/index/CompactRangeIndex.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,10 +167,10 @@ void removeMapping(RegionEntry entry, int opCode) throws IMQException {
167167
if (oldKeyValue.get() == null) {
168168
return;
169169
}
170-
indexStore.removeMapping(oldKeyValue.get().getOldKey(), entry);
170+
indexStore.removeMappingGII(oldKeyValue.get().getOldKey(), entry);
171171
} else {
172172
// rely on reverse map in the index store to figure out the real key
173-
indexStore.removeMapping(IndexManager.NULL, entry);
173+
indexStore.removeMappingGII(IndexManager.NULL, entry);
174174
}
175175
} else if (opCode == CLEAN_UP_THREAD_LOCALS) {
176176
if (oldKeyValue != null) {

geode-core/src/main/java/org/apache/geode/cache/query/internal/index/IndexStore.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,13 @@ public interface IndexStore {
3838
*/
3939
void removeMapping(Object indexKey, RegionEntry re) throws IMQException;
4040

41+
/**
42+
* Remove a mapping from the index store If entry at indexKey is not found, we must crawl the
43+
* index to be sure the region entry does not exist
44+
*
45+
*/
46+
void removeMappingGII(Object indexKey, RegionEntry re) throws IMQException;
47+
4148
/**
4249
* Update a mapping in the index store. This method adds a new mapping and removes the old mapping
4350
*

geode-core/src/main/java/org/apache/geode/cache/query/internal/index/MapIndexStore.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,11 @@ public void updateMapping(Object indexKey, Object oldKey, RegionEntry re, Object
5858
addMapping(indexKey, re);
5959
}
6060

61+
@Override
62+
public void removeMappingGII(Object indexKey, RegionEntry re) {
63+
removeMapping(indexKey, re);
64+
}
65+
6166
@Override
6267
public void removeMapping(Object indexKey, RegionEntry re) {
6368
indexMap.remove(indexKey, re.getKey());

geode-core/src/main/java/org/apache/geode/cache/query/internal/index/MemoryIndexStore.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,10 +295,20 @@ public void addMapping(Object indexKey, RegionEntry re) throws IMQException {
295295
updateMapping(indexKey, null, re, null);
296296
}
297297

298+
@Override
299+
public void removeMappingGII(Object indexKey, RegionEntry re) throws IMQException {
300+
doRemoveMapping(indexKey, re, false);
301+
}
302+
298303
@Override
299304
public void removeMapping(Object indexKey, RegionEntry re) throws IMQException {
305+
doRemoveMapping(indexKey, re, true);
306+
}
307+
308+
private void doRemoveMapping(Object indexKey, RegionEntry re, boolean findOldKey)
309+
throws IMQException {
300310
// Remove from forward map
301-
boolean found = basicRemoveMapping(indexKey, re, true);
311+
boolean found = basicRemoveMapping(indexKey, re, findOldKey);
302312
// Remove from reverse map.
303313
// We do NOT need to synchronize here as different RegionEntries will be
304314
// operating concurrently i.e. different keys in entryToValuesMap which

0 commit comments

Comments
 (0)