From b4ebfec9599aa7a32f5a6168d0ccf5c38cfda1d1 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Wed, 10 Jan 2024 10:23:57 -0800 Subject: [PATCH 1/2] Simplified snapshotCache with just one ConcurrentHashMap. Removed pendingEvictionList and ReferenceCountedCallback. --- .../ozone/om/TestSnapshotDeletingService.java | 1 - .../ozone/om/snapshot/ReferenceCounted.java | 13 +- .../ozone/om/snapshot/SnapshotCache.java | 244 ++++-------------- .../ozone/om/snapshot/TestSnapshotCache.java | 93 +------ 4 files changed, 65 insertions(+), 286 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestSnapshotDeletingService.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestSnapshotDeletingService.java index a14255d3c15..12844c23cd7 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestSnapshotDeletingService.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestSnapshotDeletingService.java @@ -194,7 +194,6 @@ public void testMultipleSnapshotKeyReclaim() throws Exception { // /vol1/bucket2/bucket2snap1 has been cleaned up from cache map SnapshotCache snapshotCache = om.getOmSnapshotManager().getSnapshotCache(); assertEquals(2, snapshotCache.size()); - assertEquals(2, snapshotCache.getPendingEvictionListSize()); } @SuppressWarnings("checkstyle:MethodLength") diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/ReferenceCounted.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/ReferenceCounted.java index e064812de8e..808a5ed4c19 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/ReferenceCounted.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/ReferenceCounted.java @@ -25,7 +25,7 @@ /** * Add reference counter to an object instance. */ -public class ReferenceCounted +public class ReferenceCounted implements AutoCloseable { /** @@ -95,12 +95,6 @@ public long incrementRefCount() { long newValTotal = refCount.incrementAndGet(); Preconditions.checkState(newValTotal > 0L, "Total reference count overflown"); - - if (refCount.get() == 1L) { - // ref count increased to one (from zero), remove from - // pendingEvictionList if added - parentWithCallback.callback(this); - } } return refCount.get(); @@ -131,11 +125,6 @@ public long decrementRefCount() { long newValTotal = refCount.decrementAndGet(); Preconditions.checkState(newValTotal >= 0L, "Total reference count underflow"); - - if (refCount.get() == 0L) { - // ref count decreased to zero, add to pendingEvictionList - parentWithCallback.callback(this); - } } return refCount.get(); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotCache.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotCache.java index d8932c0e7e0..226acbb7dd1 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotCache.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotCache.java @@ -18,7 +18,6 @@ package org.apache.hadoop.ozone.om.snapshot; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Preconditions; import com.google.common.cache.CacheLoader; import org.apache.hadoop.ozone.om.IOmMetadataReader; import org.apache.hadoop.ozone.om.OmSnapshot; @@ -28,11 +27,8 @@ import org.slf4j.LoggerFactory; import java.io.IOException; -import java.util.Collections; import java.util.Iterator; -import java.util.LinkedHashSet; import java.util.Map; -import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.FILE_NOT_FOUND; @@ -41,7 +37,7 @@ /** * Thread-safe custom unbounded LRU cache to manage open snapshot DB instances. */ -public class SnapshotCache implements ReferenceCountedCallback { +public class SnapshotCache { static final Logger LOG = LoggerFactory.getLogger(SnapshotCache.class); @@ -49,15 +45,8 @@ public class SnapshotCache implements ReferenceCountedCallback { // Key: DB snapshot table key // Value: OmSnapshot instance, each holds a DB instance handle inside // TODO: [SNAPSHOT] Consider wrapping SoftReference<> around IOmMetadataReader - private final ConcurrentHashMap> dbMap; + private final ConcurrentHashMap> dbMap; - // Linked hash set that holds OmSnapshot instances whose reference count - // has reached zero. Those entries are eligible to be evicted and closed. - // Sorted in last used order. - // Least-recently-used entry located at the beginning. - private final Set< - ReferenceCounted> pendingEvictionList; private final OmSnapshotManager omSnapshotManager; private final CacheLoader cacheLoader; // Soft-limit of the total number of snapshot DB instances allowed to be @@ -69,25 +58,16 @@ public SnapshotCache( CacheLoader cacheLoader, int cacheSizeLimit) { this.dbMap = new ConcurrentHashMap<>(); - this.pendingEvictionList = - Collections.synchronizedSet(new LinkedHashSet<>()); this.omSnapshotManager = omSnapshotManager; this.cacheLoader = cacheLoader; this.cacheSizeLimit = cacheSizeLimit; } @VisibleForTesting - ConcurrentHashMap> getDbMap() { + ConcurrentHashMap> getDbMap() { return dbMap; } - @VisibleForTesting - Set> getPendingEvictionList() { - return pendingEvictionList; - } - /** * @return number of DB instances currently held in cache. */ @@ -95,41 +75,34 @@ public int size() { return dbMap.size(); } - public int getPendingEvictionListSize() { - return pendingEvictionList.size(); - } - /** * Immediately invalidate an entry. * @param key DB snapshot table key */ public void invalidate(String key) throws IOException { - ReferenceCounted - rcOmSnapshot = dbMap.get(key); - if (rcOmSnapshot != null) { - pendingEvictionList.remove(rcOmSnapshot); - try { - ((OmSnapshot) rcOmSnapshot.get()).close(); - } catch (IOException e) { - throw new IllegalStateException("Failed to close snapshot: " + key, e); + dbMap.compute(key, (k, v) -> { + if (v == null) { + LOG.warn("Key: '{}' does not exist in cache.", k); + } else { + try { + ((OmSnapshot) v.get()).close(); + } catch (IOException e) { + throw new IllegalStateException("Failed to close snapshot: " + key, e); + } } - // Remove the entry from map - dbMap.remove(key); - } + return null; + }); } /** * Immediately invalidate all entries and close their DB instances in cache. */ public void invalidateAll() { - Iterator< - Map.Entry>> + Iterator>> it = dbMap.entrySet().iterator(); while (it.hasNext()) { - Map.Entry> - entry = it.next(); - pendingEvictionList.remove(entry.getValue()); + Map.Entry> entry = it.next(); OmSnapshot omSnapshot = (OmSnapshot) entry.getValue().get(); try { // TODO: If wrapped with SoftReference<>, omSnapshot could be null? @@ -152,8 +125,7 @@ public enum Reason { GARBAGE_COLLECTION_WRITE } - public ReferenceCounted get(String key) - throws IOException { + public ReferenceCounted get(String key) throws IOException { return get(key, false); } @@ -163,8 +135,8 @@ public ReferenceCounted get(String key) * @param key snapshot table key * @return an OmSnapshot instance, or null on error */ - public ReferenceCounted get(String key, - boolean skipActiveCheck) throws IOException { + public ReferenceCounted get(String key, boolean skipActiveCheck) + throws IOException { // Atomic operation to initialize the OmSnapshot instance (once) if the key // does not exist, and increment the reference count on the instance. ReferenceCounted rcOmSnapshot = @@ -186,6 +158,10 @@ public ReferenceCounted get(String key, throw new IllegalStateException(ex); } } + if (v != null) { + // When RC OmSnapshot is successfully loaded + v.incrementRefCount(); + } return v; }); @@ -195,16 +171,12 @@ public ReferenceCounted get(String key, throw new OMException("Snapshot table key '" + key + "' not found, " + "or the snapshot is no longer active", OMException.ResultCodes.FILE_NOT_FOUND); - } else { - // When RC OmSnapshot is successfully loaded - rcOmSnapshot.incrementRefCount(); } // If the snapshot is already loaded in cache, the check inside the loader // above is ignored. But we would still want to reject all get()s except // when called from SDT (and some) if the snapshot is not active anymore. - if (!skipActiveCheck && - !omSnapshotManager.isSnapshotStatus(key, SNAPSHOT_ACTIVE)) { + if (!skipActiveCheck && !omSnapshotManager.isSnapshotStatus(key, SNAPSHOT_ACTIVE)) { // Ref count was incremented. Need to decrement on exception here. rcOmSnapshot.decrementRefCount(); throw new OMException("Unable to load snapshot. " + @@ -212,11 +184,6 @@ public ReferenceCounted get(String key, FILE_NOT_FOUND); } - synchronized (pendingEvictionList) { - // Remove instance from clean up list when it exists. - pendingEvictionList.remove(rcOmSnapshot); - } - // Check if any entries can be cleaned up. // At this point, cache size might temporarily exceed cacheSizeLimit // even if there are entries that can be evicted, which is fine since it @@ -231,21 +198,16 @@ public ReferenceCounted get(String key, * @param key snapshot table key */ public void release(String key) { - ReferenceCounted - rcOmSnapshot = dbMap.get(key); - if (rcOmSnapshot == null) { - throw new IllegalArgumentException( - "Key '" + key + "' does not exist in cache"); - } - - if (rcOmSnapshot.decrementRefCount() == 0L) { - synchronized (pendingEvictionList) { - // v is eligible to be evicted and closed - pendingEvictionList.add(rcOmSnapshot); + dbMap.compute(key, (k, v) -> { + if (v == null) { + throw new IllegalArgumentException("Key '" + key + "' does not exist in cache."); + } else { + v.decrementRefCount(); } - } + return v; + }); - // The cache size might have already exceed the soft limit + // The cache size might have already exceeded the soft limit // Thus triggering cleanup() to check and evict if applicable cleanup(); } @@ -259,33 +221,12 @@ public void release(OmSnapshot omSnapshot) { release(snapshotTableKey); } - /** - * Callback method used to enqueue or dequeue ReferenceCounted from - * pendingEvictionList. - * @param referenceCounted ReferenceCounted object - */ - @Override - public void callback(ReferenceCounted referenceCounted) { - synchronized (pendingEvictionList) { - if (referenceCounted.getTotalRefCount() == 0L) { - // Reference count reaches zero, add to pendingEvictionList - Preconditions.checkState( - !pendingEvictionList.contains(referenceCounted), - "SnapshotCache is inconsistent. Entry should not be in the " - + "pendingEvictionList when ref count just reached zero."); - pendingEvictionList.add(referenceCounted); - } else if (referenceCounted.getTotalRefCount() == 1L) { - pendingEvictionList.remove(referenceCounted); - } - } - } - /** * Wrapper for cleanupInternal() that is synchronized to prevent multiple * threads from interleaving into the cleanup method. */ private synchronized void cleanup() { - synchronized (pendingEvictionList) { + if (dbMap.size() > cacheSizeLimit) { cleanupInternal(); } } @@ -296,105 +237,28 @@ private synchronized void cleanup() { * TODO: [SNAPSHOT] Add new ozone debug CLI command to trigger this directly. */ private void cleanupInternal() { - long numEntriesToEvict = (long) dbMap.size() - cacheSizeLimit; - while (numEntriesToEvict > 0L && pendingEvictionList.size() > 0) { - // Get the first instance in the clean up list - ReferenceCounted rcOmSnapshot = - pendingEvictionList.iterator().next(); - OmSnapshot omSnapshot = (OmSnapshot) rcOmSnapshot.get(); - LOG.debug("Evicting OmSnapshot instance {} with table key {}", - rcOmSnapshot, omSnapshot.getSnapshotTableKey()); - // Sanity check - Preconditions.checkState(rcOmSnapshot.getTotalRefCount() == 0L, - "Illegal state: OmSnapshot reference count non-zero (" - + rcOmSnapshot.getTotalRefCount() + ") but shows up in the " - + "clean up list"); - - final String key = omSnapshot.getSnapshotTableKey(); - final ReferenceCounted result = - dbMap.remove(key); - // Sanity check - Preconditions.checkState(rcOmSnapshot == result, - "Cache map entry removal failure. The cache is in an inconsistent " - + "state. Expected OmSnapshot instance: " + rcOmSnapshot - + ", actual: " + result + " for key: " + key); - - pendingEvictionList.remove(result); - - // Close the instance, which also closes its DB handle. - try { - ((OmSnapshot) rcOmSnapshot.get()).close(); - } catch (IOException ex) { - throw new IllegalStateException("Error while closing snapshot DB", ex); - } - - --numEntriesToEvict; - } - - // Print warning message if actual cache size is exceeding the soft limit - // even after the cleanup procedure above. - if ((long) dbMap.size() > cacheSizeLimit) { - LOG.warn("Current snapshot cache size ({}) is exceeding configured " - + "soft-limit ({}) after possible evictions.", - dbMap.size(), cacheSizeLimit); - - Preconditions.checkState(pendingEvictionList.size() == 0); - } - } - - /** - * Check cache consistency. - * @return true if the cache internal structure is consistent to the best of - * its knowledge, false if found to be inconsistent and details logged. - */ - @VisibleForTesting - public boolean isConsistent() { - // Uses dbMap as the source of truth for this check, whether dbMap entries - // are in OM DB's snapshotInfoTable is out of the scope of this check. - - LOG.info("dbMap has {} entries", dbMap.size()); - LOG.info("pendingEvictionList has {} entries", - pendingEvictionList.size()); - - // pendingEvictionList must be empty if cache size exceeds limit - if (dbMap.size() > cacheSizeLimit) { - if (pendingEvictionList.size() != 0) { - // cleanup() is not functioning correctly - LOG.error("pendingEvictionList is not empty even when cache size" - + "exceeds limit"); - } - } - - dbMap.forEach((k, v) -> { - if (v.getTotalRefCount() == 0L) { - long threadRefCount = v.getCurrentThreadRefCount(); - if (threadRefCount != 0L) { - LOG.error("snapshotTableKey='{}' instance has inconsistent " - + "ref count. Total ref count is 0 but thread " - + "ref count is {}", k, threadRefCount); - } - // Zero ref count values in dbMap must be in pendingEvictionList - if (!pendingEvictionList.contains(v)) { - LOG.error("snapshotTableKey='{}' instance has zero ref count but " - + "not in pendingEvictionList", k); + for (Map.Entry> entry : dbMap.entrySet()) { + dbMap.compute(entry.getKey(), (k, v) -> { + if (v == null) { + throw new IllegalStateException("Key '" + k + "' does not exist in cache. The RocksDB " + + "instance of the Snapshot may not be closed properly."); } - } - }); - pendingEvictionList.forEach(v -> { - // Objects in pendingEvictionList should still be in dbMap - if (!dbMap.contains(v)) { - LOG.error("Instance '{}' is in pendingEvictionList but not in " - + "dbMap", v); - } - // Instances in pendingEvictionList must have ref count equals 0 - if (v.getTotalRefCount() != 0L) { - LOG.error("Instance '{}' is in pendingEvictionList but ref count " - + "is not zero", v); - } - }); - - return true; + if (v.getTotalRefCount() > 0) { + LOG.debug("Snapshot {} is still being referenced ({}), skipping its clean up", + k, v.getTotalRefCount()); + return v; + } else { + LOG.debug("Closing Snapshot {}. It is not being referenced anymore.", k); + // Close the instance, which also closes its DB handle. + try { + ((OmSnapshot) v.get()).close(); + } catch (IOException ex) { + throw new IllegalStateException("Error while closing snapshot DB", ex); + } + return null; + } + }); + } } - } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotCache.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotCache.java index f8c93929778..37a8c26e7b6 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotCache.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotCache.java @@ -39,7 +39,6 @@ import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; @@ -102,7 +101,6 @@ void testGet() throws IOException { assertNotNull(omSnapshot.get()); assertInstanceOf(OmSnapshot.class, omSnapshot.get()); assertEquals(1, snapshotCache.size()); - assertTrue(snapshotCache.isConsistent()); } @Test @@ -113,7 +111,6 @@ void testGetTwice() throws IOException { snapshotCache.get(dbKey1); assertNotNull(omSnapshot1); assertEquals(1, snapshotCache.size()); - assertTrue(snapshotCache.isConsistent()); ReferenceCounted omSnapshot1again = snapshotCache.get(dbKey1); @@ -121,7 +118,6 @@ void testGetTwice() throws IOException { assertEquals(omSnapshot1, omSnapshot1again); assertEquals(omSnapshot1.get(), omSnapshot1again.get()); assertEquals(1, snapshotCache.size()); - assertTrue(snapshotCache.isConsistent()); } @Test @@ -133,15 +129,10 @@ void testReleaseByDbKey() throws IOException { assertNotNull(omSnapshot1); assertNotNull(omSnapshot1.get()); assertEquals(1, snapshotCache.size()); - assertEquals(0, snapshotCache.getPendingEvictionList().size()); - assertTrue(snapshotCache.isConsistent()); snapshotCache.release(dbKey1); // Entry will not be immediately evicted assertEquals(1, snapshotCache.size()); - // Entry is queued for eviction as its ref count reaches zero - assertEquals(1, snapshotCache.getPendingEvictionList().size()); - assertTrue(snapshotCache.isConsistent()); } @Test @@ -152,15 +143,10 @@ void testReleaseByOmSnapshotInstance() throws IOException { snapshotCache.get(dbKey1); assertNotNull(omSnapshot1); assertEquals(1, snapshotCache.size()); - assertEquals(0, snapshotCache.getPendingEvictionList().size()); - assertTrue(snapshotCache.isConsistent()); snapshotCache.release((OmSnapshot) omSnapshot1.get()); // Entry will not be immediately evicted assertEquals(1, snapshotCache.size()); - // Entry is queued for eviction as its ref count reaches zero - assertEquals(1, snapshotCache.getPendingEvictionList().size()); - assertTrue(snapshotCache.isConsistent()); } @Test @@ -171,16 +157,13 @@ void testInvalidate() throws IOException { snapshotCache.get(dbKey1); assertNotNull(omSnapshot); assertEquals(1, snapshotCache.size()); - assertTrue(snapshotCache.isConsistent()); snapshotCache.release(dbKey1); // Entry will not be immediately evicted assertEquals(1, snapshotCache.size()); - assertTrue(snapshotCache.isConsistent()); snapshotCache.invalidate(dbKey1); assertEquals(0, snapshotCache.size()); - assertTrue(snapshotCache.isConsistent()); } @Test @@ -191,7 +174,6 @@ void testInvalidateAll() throws IOException { snapshotCache.get(dbKey1); assertNotNull(omSnapshot1); assertEquals(1, snapshotCache.size()); - assertTrue(snapshotCache.isConsistent()); final String dbKey2 = "dbKey2"; ReferenceCounted omSnapshot2 = @@ -200,31 +182,22 @@ void testInvalidateAll() throws IOException { assertEquals(2, snapshotCache.size()); // Should be difference omSnapshot instances assertNotEquals(omSnapshot1, omSnapshot2); - assertTrue(snapshotCache.isConsistent()); final String dbKey3 = "dbKey3"; ReferenceCounted omSnapshot3 = snapshotCache.get(dbKey3); assertNotNull(omSnapshot3); assertEquals(3, snapshotCache.size()); - assertEquals(0, snapshotCache.getPendingEvictionList().size()); - assertTrue(snapshotCache.isConsistent()); snapshotCache.release(dbKey1); // Entry will not be immediately evicted assertEquals(3, snapshotCache.size()); - assertEquals(1, snapshotCache.getPendingEvictionList().size()); - assertTrue(snapshotCache.isConsistent()); snapshotCache.invalidate(dbKey1); assertEquals(2, snapshotCache.size()); - assertEquals(0, snapshotCache.getPendingEvictionList().size()); - assertTrue(snapshotCache.isConsistent()); snapshotCache.invalidateAll(); assertEquals(0, snapshotCache.size()); - assertEquals(0, snapshotCache.getPendingEvictionList().size()); - assertTrue(snapshotCache.isConsistent()); } private void assertEntryExistence(String key, boolean shouldExist) { @@ -248,39 +221,27 @@ void testEviction1() throws IOException { final String dbKey1 = "dbKey1"; snapshotCache.get(dbKey1); assertEquals(1, snapshotCache.size()); - assertTrue(snapshotCache.isConsistent()); snapshotCache.release(dbKey1); assertEquals(1, snapshotCache.size()); - assertEquals(1, snapshotCache.getPendingEvictionList().size()); - assertTrue(snapshotCache.isConsistent()); final String dbKey2 = "dbKey2"; snapshotCache.get(dbKey2); assertEquals(2, snapshotCache.size()); - assertTrue(snapshotCache.isConsistent()); snapshotCache.release(dbKey2); assertEquals(2, snapshotCache.size()); - assertEquals(2, snapshotCache.getPendingEvictionList().size()); - assertTrue(snapshotCache.isConsistent()); final String dbKey3 = "dbKey3"; snapshotCache.get(dbKey3); assertEquals(3, snapshotCache.size()); - assertTrue(snapshotCache.isConsistent()); snapshotCache.release(dbKey3); assertEquals(3, snapshotCache.size()); - assertEquals(3, snapshotCache.getPendingEvictionList().size()); - assertTrue(snapshotCache.isConsistent()); final String dbKey4 = "dbKey4"; snapshotCache.get(dbKey4); - // dbKey1 would have been evicted by the end of the last get() because - // it was release()d. - assertEquals(3, snapshotCache.size()); - assertTrue(snapshotCache.isConsistent()); + // dbKey1, dbKey2 and dbKey3 would have been evicted by the end of the last get() because + // those were release()d. + assertEquals(1, snapshotCache.size()); assertEntryExistence(dbKey1, false); - assertEquals(2, snapshotCache.getPendingEvictionList().size()); - assertTrue(snapshotCache.isConsistent()); } @Test @@ -290,25 +251,20 @@ void testEviction2() throws IOException { final String dbKey1 = "dbKey1"; snapshotCache.get(dbKey1); assertEquals(1, snapshotCache.size()); - assertTrue(snapshotCache.isConsistent()); final String dbKey2 = "dbKey2"; snapshotCache.get(dbKey2); assertEquals(2, snapshotCache.size()); - assertTrue(snapshotCache.isConsistent()); final String dbKey3 = "dbKey3"; snapshotCache.get(dbKey3); assertEquals(3, snapshotCache.size()); - assertTrue(snapshotCache.isConsistent()); final String dbKey4 = "dbKey4"; snapshotCache.get(dbKey4); // dbKey1 would not have been evicted because it is not release()d assertEquals(4, snapshotCache.size()); assertEntryExistence(dbKey1, true); - assertEquals(0, snapshotCache.getPendingEvictionList().size()); - assertTrue(snapshotCache.isConsistent()); // Releasing dbKey2 at this point should immediately trigger its eviction // because the cache size exceeded the soft limit @@ -316,8 +272,6 @@ void testEviction2() throws IOException { assertEquals(3, snapshotCache.size()); assertEntryExistence(dbKey2, false); assertEntryExistence(dbKey1, true); - assertEquals(0, snapshotCache.getPendingEvictionList().size()); - assertTrue(snapshotCache.isConsistent()); } @Test @@ -325,71 +279,44 @@ void testEviction2() throws IOException { void testEviction3WithClose() throws IOException { final String dbKey1 = "dbKey1"; - try (ReferenceCounted rcOmSnapshot = - snapshotCache.get(dbKey1)) { + try (ReferenceCounted rcOmSnapshot = snapshotCache.get(dbKey1)) { assertEquals(1L, rcOmSnapshot.getTotalRefCount()); assertEquals(1, snapshotCache.size()); - assertEquals(0, snapshotCache.getPendingEvictionList().size()); - assertTrue(snapshotCache.isConsistent()); } // ref count should have been decreased because it would be close()d // upon exiting try-with-resources. assertEquals(0L, snapshotCache.getDbMap().get(dbKey1).getTotalRefCount()); assertEquals(1, snapshotCache.size()); - assertEquals(1, snapshotCache.getPendingEvictionList().size()); - assertTrue(snapshotCache.isConsistent()); final String dbKey2 = "dbKey2"; - try (ReferenceCounted rcOmSnapshot = - snapshotCache.get(dbKey2)) { + try (ReferenceCounted rcOmSnapshot = snapshotCache.get(dbKey2)) { assertEquals(1L, rcOmSnapshot.getTotalRefCount()); assertEquals(2, snapshotCache.size()); - assertEquals(1, snapshotCache.getPendingEvictionList().size()); - assertTrue(snapshotCache.isConsistent()); // Get dbKey2 entry a second time - try (ReferenceCounted rcOmSnapshot2 = - snapshotCache.get(dbKey2)) { + try (ReferenceCounted rcOmSnapshot2 = snapshotCache.get(dbKey2)) { assertEquals(2L, rcOmSnapshot.getTotalRefCount()); assertEquals(2L, rcOmSnapshot2.getTotalRefCount()); assertEquals(2, snapshotCache.size()); - assertEquals(1, snapshotCache.getPendingEvictionList().size()); - assertTrue(snapshotCache.isConsistent()); } assertEquals(1L, rcOmSnapshot.getTotalRefCount()); - assertTrue(snapshotCache.isConsistent()); } assertEquals(0L, snapshotCache.getDbMap().get(dbKey2).getTotalRefCount()); assertEquals(2, snapshotCache.size()); - assertEquals(2, snapshotCache.getPendingEvictionList().size()); - assertTrue(snapshotCache.isConsistent()); final String dbKey3 = "dbKey3"; - try (ReferenceCounted rcOmSnapshot = - snapshotCache.get(dbKey3)) { + try (ReferenceCounted rcOmSnapshot = snapshotCache.get(dbKey3)) { assertEquals(1L, rcOmSnapshot.getTotalRefCount()); assertEquals(3, snapshotCache.size()); - assertEquals(2, snapshotCache.getPendingEvictionList().size()); - assertTrue(snapshotCache.isConsistent()); } assertEquals(0L, snapshotCache.getDbMap().get(dbKey3).getTotalRefCount()); assertEquals(3, snapshotCache.size()); - assertEquals(3, snapshotCache.getPendingEvictionList().size()); - assertTrue(snapshotCache.isConsistent()); final String dbKey4 = "dbKey4"; - try (ReferenceCounted rcOmSnapshot = - snapshotCache.get(dbKey4)) { + try (ReferenceCounted rcOmSnapshot = snapshotCache.get(dbKey4)) { assertEquals(1L, rcOmSnapshot.getTotalRefCount()); - assertEquals(3, snapshotCache.size()); - // An entry has been evicted at this point - assertEquals(2, snapshotCache.getPendingEvictionList().size()); - assertTrue(snapshotCache.isConsistent()); + assertEquals(1, snapshotCache.size()); } assertEquals(0L, snapshotCache.getDbMap().get(dbKey4).getTotalRefCount()); - // Reached cache size limit - assertEquals(3, snapshotCache.size()); - assertEquals(3, snapshotCache.getPendingEvictionList().size()); - assertTrue(snapshotCache.isConsistent()); + assertEquals(1, snapshotCache.size()); } - } From f5ddec7ef2ffdad8f65f7a8d969bc970282fe42b Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Thu, 11 Jan 2024 13:54:26 -0800 Subject: [PATCH 2/2] Removed ReferenceCountedCallback --- .../om/snapshot/ReferenceCountedCallback.java | 25 ------------------- 1 file changed, 25 deletions(-) delete mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/ReferenceCountedCallback.java diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/ReferenceCountedCallback.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/ReferenceCountedCallback.java deleted file mode 100644 index d63f5783b80..00000000000 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/ReferenceCountedCallback.java +++ /dev/null @@ -1,25 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.hadoop.ozone.om.snapshot; - -/** - * Callback interface for ReferenceCounted. - */ -public interface ReferenceCountedCallback { - void callback(ReferenceCounted referenceCounted); -}