gesterzhou commented on a change in pull request #6430: URL: https://github.com/apache/geode/pull/6430#discussion_r641744823
########## File path: geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java ########## @@ -388,89 +375,105 @@ public void close(BucketRegion bucketRegion) { if (logger.isDebugEnabled()) { logger.debug("Clearing entries for {} rvv={}", _getOwner(), rvv); } - LocalRegion lr = _getOwner(); + final LocalRegion lr = _getOwner(); RegionVersionVector localRvv = lr.getVersionVector(); - incClearCount(lr); - // lock for size calcs if the region might have tombstones - Object lockObj = lr.getConcurrencyChecksEnabled() ? lr.getSizeGuard() : new Object(); - synchronized (lockObj) { - if (rvv == null) { - int delta = 0; - try { - delta = sizeInVM(); // TODO soplog need to determine if stats should - // reflect only size in memory or the complete thing - } catch (GemFireIOException e) { - // ignore rather than throwing an exception during cache close - } - int tombstones = lr.getTombstoneCount(); - _mapClear(); - _getOwner().updateSizeOnClearRegion(delta - tombstones); - _getOwner().incTombstoneCount(-tombstones); - if (delta != 0) { - incEntryCount(-delta); - } - } else { - int delta = 0; - int tombstones = 0; - VersionSource myId = _getOwner().getVersionMember(); - if (localRvv != rvv) { - localRvv.recordGCVersions(rvv); - } - final boolean isTraceEnabled = logger.isTraceEnabled(); - for (RegionEntry re : regionEntries()) { - synchronized (re) { - Token value = re.getValueAsToken(); - // if it's already being removed or the entry is being created we leave it alone - if (value == Token.REMOVED_PHASE1 || value == Token.REMOVED_PHASE2) { - continue; - } - VersionSource id = re.getVersionStamp().getMemberID(); - if (id == null) { - id = myId; - } - if (rvv.contains(id, re.getVersionStamp().getRegionVersion())) { - if (isTraceEnabled) { - logger.trace("region clear op is removing {} {}", re.getKey(), - re.getVersionStamp()); + boolean isBucketRegion = false; + PartitionedRegionStats prStats = null; + long startTime = 0L; + + if (lr.isUsedForPartitionedRegionBucket()) { + prStats = lr.getPartitionedRegion().getPrStats(); + startTime = prStats.startBucketClear(); + isBucketRegion = true; + } + + try { + // lock for size calcs if the region might have tombstones + Object lockObj = lr.getConcurrencyChecksEnabled() ? lr.getSizeGuard() : new Object(); + synchronized (lockObj) { + if (rvv == null) { + int delta = 0; + try { + delta = sizeInVM(); // TODO soplog need to determine if stats should + // reflect only size in memory or the complete thing + } catch (GemFireIOException e) { + // ignore rather than throwing an exception during cache close + } + int tombstones = lr.getTombstoneCount(); + _mapClear(); + _getOwner().updateSizeOnClearRegion(delta - tombstones); + _getOwner().incTombstoneCount(-tombstones); + if (delta != 0) { + incEntryCount(-delta); + } + } else { + int delta = 0; + int tombstones = 0; + VersionSource myId = _getOwner().getVersionMember(); + if (localRvv != rvv) { + localRvv.recordGCVersions(rvv); + } + final boolean isTraceEnabled = logger.isTraceEnabled(); + for (RegionEntry re : regionEntries()) { + synchronized (re) { + Token value = re.getValueAsToken(); + // if it's already being removed or the entry is being created we leave it alone + if (value == Token.REMOVED_PHASE1 || value == Token.REMOVED_PHASE2) { + continue; } - boolean tombstone = re.isTombstone(); - // note: it.remove() did not reliably remove the entry so we use remove(K,V) here - if (getEntryMap().remove(re.getKey(), re)) { - if (OffHeapClearRequired.doesClearNeedToCheckForOffHeap()) { - GatewaySenderEventImpl.release(re.getValue()); // OFFHEAP _getValue ok - } - // If this is an overflow only region, we need to free the entry on - // disk at this point. - try { - re.removePhase1(lr, true); - } catch (RegionClearedException e) { - // do nothing, it's already cleared. + VersionSource id = re.getVersionStamp().getMemberID(); + if (id == null) { + id = myId; + } + if (rvv.contains(id, re.getVersionStamp().getRegionVersion())) { + if (isTraceEnabled) { + logger.trace("region clear op is removing {} {}", re.getKey(), + re.getVersionStamp()); } - re.removePhase2(); - lruEntryDestroy(re); - if (tombstone) { - _getOwner().incTombstoneCount(-1); - tombstones += 1; - } else { - delta += 1; + + boolean tombstone = re.isTombstone(); + // note: it.remove() did not reliably remove the entry so we use remove(K,V) here + if (getEntryMap().remove(re.getKey(), re)) { + if (OffHeapClearRequired.doesClearNeedToCheckForOffHeap()) { + GatewaySenderEventImpl.release(re.getValue()); // OFFHEAP _getValue ok + } + // If this is an overflow only region, we need to free the entry on + // disk at this point. + try { + re.removePhase1(lr, true); + } catch (RegionClearedException e) { + // do nothing, it's already cleared. + } + re.removePhase2(); + lruEntryDestroy(re); + if (tombstone) { + _getOwner().incTombstoneCount(-1); + tombstones += 1; + } else { + delta += 1; + } } + } else { // rvv does not contain this entry so it is retained + result.add(id); } - } else { // rvv does not contain this entry so it is retained - result.add(id); } } + _getOwner().updateSizeOnClearRegion(delta); + incEntryCount(-delta); + incEntryCount(-tombstones); + if (logger.isDebugEnabled()) { + logger.debug("Size after clearing = {}", getEntryMap().size()); + } + if (isTraceEnabled && getEntryMap().size() < 20) { + _getOwner().dumpBackingMap(); + } } - _getOwner().updateSizeOnClearRegion(delta); - incEntryCount(-delta); - incEntryCount(-tombstones); - if (logger.isDebugEnabled()) { - logger.debug("Size after clearing = {}", getEntryMap().size()); - } - if (isTraceEnabled && getEntryMap().size() < 20) { - _getOwner().dumpBackingMap(); - } + } + } finally { + if (isBucketRegion) { + prStats.endBucketClear(startTime); } Review comment: same. We might need "else" here -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org