jhungund commented on code in PR #5829:
URL: https://github.com/apache/hbase/pull/5829#discussion_r1577210450


##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDataTieringManager.java:
##########
@@ -245,6 +248,184 @@ public void testColdDataFiles() {
     }
   }
 
+  @Test
+  public void testPickColdDataFiles() {
+    Map<String, String> coldDataFiles = dataTieringManager.getColdFilesList();
+    assertEquals(1, coldDataFiles.size());
+    // hStoreFiles[3] is the cold file.
+    assert 
(coldDataFiles.containsKey(hStoreFiles.get(3).getFileInfo().getActiveFileName()));
+  }
+
+  /*
+   * Verify that two cold blocks(both) are evicted when bucket reaches its 
capacity. The hot file
+   * remains in the cache.
+   */
+  @Test
+  public void testBlockEvictions() throws Exception {
+    long capacitySize = 64 * 1024;
+    int writeThreads = 3;
+    int writerQLen = 64;
+    int[] bucketSizes = new int[] { 8 * 1024 + 1024 };
+
+    // Setup: Create a bucket cache with lower capacity
+    BucketCache bucketCache = new BucketCache("file:" + testDir + 
"/bucket.cache", capacitySize,
+      8192, bucketSizes, writeThreads, writerQLen, testDir + 
"/bucket.persistence",
+      DEFAULT_ERROR_TOLERATION_DURATION, defaultConf);
+
+    // Create three Cache keys with cold data files and a block with hot data.
+    // hStoreFiles.get(3) is a cold data file, while hStoreFiles.get(0) is a 
hot file.
+    Set<BlockCacheKey> cacheKeys = new HashSet<>();
+    cacheKeys.add(new BlockCacheKey(hStoreFiles.get(3).getPath(), 0, true, 
BlockType.DATA));
+    cacheKeys.add(new BlockCacheKey(hStoreFiles.get(3).getPath(), 8192, true, 
BlockType.DATA));
+    cacheKeys.add(new BlockCacheKey(hStoreFiles.get(0).getPath(), 0, true, 
BlockType.DATA));
+
+    // Create dummy data to be cached and fill the cache completely.
+    CacheTestUtils.HFileBlockPair[] blocks = 
CacheTestUtils.generateHFileBlocks(8192, 3);
+
+    int blocksIter = 0;
+    for (BlockCacheKey key : cacheKeys) {
+      bucketCache.cacheBlock(key, blocks[blocksIter++].getBlock());
+      // Ensure that the block is persisted to the file.
+      Waiter.waitFor(defaultConf, 1000000, 100,
+        () -> (bucketCache.getBackingMap().containsKey(key)));
+    }
+
+    // Verify that the bucket cache contains 3 blocks.
+    assertEquals(3, bucketCache.getBackingMap().keySet().size());
+
+    // Add an additional block into cache with hot data which should trigger 
the eviction
+    BlockCacheKey newKey = new BlockCacheKey(hStoreFiles.get(2).getPath(), 0, 
true, BlockType.DATA);
+    CacheTestUtils.HFileBlockPair[] newBlock = 
CacheTestUtils.generateHFileBlocks(8192, 1);
+
+    bucketCache.cacheBlock(newKey, newBlock[0].getBlock());
+    Waiter.waitFor(defaultConf, 1000000, 100,
+      () -> (bucketCache.getBackingMap().containsKey(newKey)));
+
+    // Verify that the bucket cache now contains 2 hot blocks blocks only.
+    // Both cold blocks of 8KB will be evicted to make room for 1 block of 8KB 
+ an additional
+    // space.
+    validateBlocks(bucketCache.getBackingMap().keySet(), 2, 2, 0);
+  }
+
+  /*
+   * Verify that two cold blocks(both) are evicted when bucket reaches its 
capacity, but one cold
+   * block remains in the cache since the required space is freed.
+   */
+  @Test
+  public void testBlockEvictionsAllColdBlocks() throws Exception {
+    long capacitySize = 64 * 1024;
+    int writeThreads = 3;
+    int writerQLen = 64;
+    int[] bucketSizes = new int[] { 8 * 1024 + 1024 };
+
+    // Setup: Create a bucket cache with lower capacity
+    BucketCache bucketCache = new BucketCache("file:" + testDir + 
"/bucket.cache", capacitySize,
+      8192, bucketSizes, writeThreads, writerQLen, testDir + 
"/bucket.persistence",
+      DEFAULT_ERROR_TOLERATION_DURATION, defaultConf);
+
+    // Create three Cache keys with three cold data blocks.
+    // hStoreFiles.get(3) is a cold data file.
+    Set<BlockCacheKey> cacheKeys = new HashSet<>();
+    cacheKeys.add(new BlockCacheKey(hStoreFiles.get(3).getPath(), 0, true, 
BlockType.DATA));
+    cacheKeys.add(new BlockCacheKey(hStoreFiles.get(3).getPath(), 8192, true, 
BlockType.DATA));
+    cacheKeys.add(new BlockCacheKey(hStoreFiles.get(3).getPath(), 16384, true, 
BlockType.DATA));
+
+    // Create dummy data to be cached and fill the cache completely.
+    CacheTestUtils.HFileBlockPair[] blocks = 
CacheTestUtils.generateHFileBlocks(8192, 3);
+
+    int blocksIter = 0;
+    for (BlockCacheKey key : cacheKeys) {
+      bucketCache.cacheBlock(key, blocks[blocksIter++].getBlock());
+      // Ensure that the block is persisted to the file.
+      Waiter.waitFor(defaultConf, 1000000, 100,
+        () -> (bucketCache.getBackingMap().containsKey(key)));
+    }
+
+    // Verify that the bucket cache contains 3 blocks.
+    assertEquals(3, bucketCache.getBackingMap().keySet().size());
+
+    // Add an additional block into cache with hot data which should trigger 
the eviction
+    BlockCacheKey newKey = new BlockCacheKey(hStoreFiles.get(2).getPath(), 0, 
true, BlockType.DATA);
+    CacheTestUtils.HFileBlockPair[] newBlock = 
CacheTestUtils.generateHFileBlocks(8192, 1);
+
+    bucketCache.cacheBlock(newKey, newBlock[0].getBlock());
+    Waiter.waitFor(defaultConf, 1000000, 100,
+      () -> (bucketCache.getBackingMap().containsKey(newKey)));
+
+    // Verify that the bucket cache now contains 1 cold block and a newly 
added hot block.
+    validateBlocks(bucketCache.getBackingMap().keySet(), 2, 1, 1);
+  }
+
+  /*
+   * Verify that a hot block evicted along with a cold block when bucket 
reaches its capacity.
+   */
+  @Test
+  public void testBlockEvictionsHotBlocks() throws Exception {
+    long capacitySize = 64 * 1024;
+    int writeThreads = 3;
+    int writerQLen = 64;
+    int[] bucketSizes = new int[] { 8 * 1024 + 1024 };
+
+    // Setup: Create a bucket cache with lower capacity
+    BucketCache bucketCache = new BucketCache("file:" + testDir + 
"/bucket.cache", capacitySize,
+      8192, bucketSizes, writeThreads, writerQLen, testDir + 
"/bucket.persistence",
+      DEFAULT_ERROR_TOLERATION_DURATION, defaultConf);
+
+    // Create three Cache keys with two hot data blocks and one cold data block
+    // hStoreFiles.get(0) is a hot data file and hStoreFiles.get(3) is a cold 
data file.
+    Set<BlockCacheKey> cacheKeys = new HashSet<>();
+    cacheKeys.add(new BlockCacheKey(hStoreFiles.get(0).getPath(), 0, true, 
BlockType.DATA));
+    cacheKeys.add(new BlockCacheKey(hStoreFiles.get(0).getPath(), 8192, true, 
BlockType.DATA));
+    cacheKeys.add(new BlockCacheKey(hStoreFiles.get(3).getPath(), 0, true, 
BlockType.DATA));
+
+    // Create dummy data to be cached and fill the cache completely.
+    CacheTestUtils.HFileBlockPair[] blocks = 
CacheTestUtils.generateHFileBlocks(8192, 3);
+
+    int blocksIter = 0;
+    for (BlockCacheKey key : cacheKeys) {
+      bucketCache.cacheBlock(key, blocks[blocksIter++].getBlock());
+      // Ensure that the block is persisted to the file.
+      Waiter.waitFor(defaultConf, 1000000, 100,
+        () -> (bucketCache.getBackingMap().containsKey(key)));
+    }
+
+    // Verify that the bucket cache contains 3 blocks.
+    assertEquals(3, bucketCache.getBackingMap().keySet().size());
+
+    // Add an additional block which should evict the only cold block with an 
additional hot block.
+    BlockCacheKey newKey = new BlockCacheKey(hStoreFiles.get(2).getPath(), 0, 
true, BlockType.DATA);
+    CacheTestUtils.HFileBlockPair[] newBlock = 
CacheTestUtils.generateHFileBlocks(8192, 1);
+
+    bucketCache.cacheBlock(newKey, newBlock[0].getBlock());
+    Waiter.waitFor(defaultConf, 1000000, 100,
+      () -> (bucketCache.getBackingMap().containsKey(newKey)));

Review Comment:
   Hi @wchevreuil, I varied the bucket cache sizes and checked the bucket cache 
numbers (capacity, free space, used space) at various stages in the unit test. 
Below are my observations.
   For a bucket cache with 40K byes capacity:
   `At the start:   Bucket Max Size: 40960, Current size: 0,          Data 
size: 0,         free size: 36864,  Block Count: 0
    After 1 block: Bucket Max Size: 40960, Current size: 9216,    Data size: 
8213,   free size: 27648,  Block Count: 1
    After 2 block: Bucket Max Size: 40960, Current size: 18432, Data size: 
16426, free size: 18432,  Block Count: 2
    After 3 block: Bucket Max Size: 40960, Current size: 27648, Data size: 
24639, free size: 9216,   Block Count: 3`
   
   Eviction gets triggered for 4th block addition.
   For a bucket cache with 64K capacity:
   `At the start:   Bucket Max Size: 65536, Current size: 0,          Data 
size: 0,          free size: 36864, Block Count: 0
    After 1 block: Bucket Max Size: 65536, Current size: 9216,    Data size: 
8213,    free size: 27648,  Block Count: 1
    After 2 block: Bucket Max Size: 65536, Current size: 18432, Data size: 
16426, free size: 18432,  Block Count: 2
   After 3 block:  Bucket Max Size: 65536, Current size: 27648, Data size: 
24639, free size: 9216,   Block Count: 3`
   
   Eviction gets triggered for 4th block addition.
   
   For a bucket cache with 72K capacity:
   `At the start:  Bucket Max Size: 73728,  Current size: 0,          Data 
size: 0,          free size: 73728,  Block Count: 0
    After 1 block: Bucket Max Size: 73728, Current size: 9216,    Data size: 
8213,    free size: 64512,  Block Count: 1
    After 2 block: Bucket Max Size: 73728, Current size: 18432, Data size: 
16426,  free size: 55296, Block Count: 2
    After 3 block: Bucket Max Size: 73728, Current size: 27648, Data size: 
24639, free size: 46080, Block Count: 3`
   Eviction does not get triggered for 4th Block addition.
   
   Hence, as you can see that inspite of varying the bucket cache sizes between 
40K and 64K, the free space available is only 36K. Hence, the eviction is 
triggered.



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to