keith-turner commented on code in PR #3063:
URL: https://github.com/apache/accumulo/pull/3063#discussion_r1011519848
##########
core/src/test/java/org/apache/accumulo/core/file/blockfile/cache/TestCachedBlockQueue.java:
##########
@@ -58,34 +106,38 @@ public void testQueue() {
long expectedSize = cb1.heapSize() + cb2.heapSize() + cb3.heapSize() +
cb4.heapSize()
+ cb5.heapSize() + cb6.heapSize() + cb7.heapSize() + cb8.heapSize();
- assertEquals(queue.heapSize(), expectedSize);
+ assertEquals(expectedSize, queue.heapSize());
Review Comment:
Would be nice to do some checks on `sum`. Could add the following check, I
think its correct but not 100% sure.
```suggestion
assertEquals(expectedSize, queue.heapSize());
assertEquals(expectedSize,sum.get());
```
##########
core/src/test/java/org/apache/accumulo/core/file/blockfile/cache/TestCachedBlockQueue.java:
##########
@@ -20,26 +20,74 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
-import java.util.LinkedList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.stream.Collectors;
import org.apache.accumulo.core.file.blockfile.cache.lru.CachedBlockQueue;
import org.junit.jupiter.api.Test;
public class TestCachedBlockQueue {
+ @Test
+ public void testLargeBlock() {
+ CachedBlockQueue queue = new CachedBlockQueue(10000L, 1000L);
+ CachedBlock cb1 = new CachedBlock(10001L, "cb1", 1L);
+ queue.add(cb1);
+
+ List<org.apache.accumulo.core.file.blockfile.cache.lru.CachedBlock> blocks
= getList(queue);
+ assertEquals("cb1", Objects.requireNonNull(blocks.get(0)).getName());
+ }
@Test
- public void testQueue() {
+ public void testAddNewerBlock() {
+ CachedBlockQueue queue = new CachedBlockQueue(10000L, 1000L);
+
+ AtomicLong sum = new AtomicLong();
+
+ CachedBlock cb1 = new CachedBlock(5000L, "cb1", 1L);
+ cb1.recordSize(sum);
+ CachedBlock cb2 = new CachedBlock(5000, "cb2", 2L);
+ cb2.recordSize(sum);
+ CachedBlock cb3 = new CachedBlock(5000, "cb3", 3L);
+ cb3.recordSize(sum);
+
+ queue.add(cb1);
+ queue.add(cb2);
+ queue.add(cb3);
+ List<org.apache.accumulo.core.file.blockfile.cache.lru.CachedBlock> blocks
= getList(queue);
+
+ assertEquals(2, blocks.size());
+
+ assertEquals(Arrays.asList("cb1", "cb2"),
+ blocks.stream().map(cb -> cb.getName()).collect(Collectors.toList()));
+ }
+
+ @Test
+ public void testQueue() {
+ AtomicLong sum = new AtomicLong();
CachedBlock cb1 = new CachedBlock(1000, "cb1", 1);
+ cb1.recordSize(sum);
CachedBlock cb2 = new CachedBlock(1500, "cb2", 2);
+ cb2.recordSize(sum);
CachedBlock cb3 = new CachedBlock(1000, "cb3", 3);
+ cb3.recordSize(sum);
CachedBlock cb4 = new CachedBlock(1500, "cb4", 4);
+ cb4.recordSize(sum);
CachedBlock cb5 = new CachedBlock(1000, "cb5", 5);
+ cb5.recordSize(sum);
CachedBlock cb6 = new CachedBlock(1750, "cb6", 6);
+ cb6.recordSize(sum);
CachedBlock cb7 = new CachedBlock(1000, "cb7", 7);
+ cb7.recordSize(sum);
CachedBlock cb8 = new CachedBlock(1500, "cb8", 8);
+ cb8.recordSize(sum);
CachedBlock cb9 = new CachedBlock(1000, "cb9", 9);
+ cb9.recordSize(sum);
CachedBlock cb10 = new CachedBlock(1500, "cb10", 10);
+ cb10.recordSize(sum);
Review Comment:
```suggestion
// validate that sum was not improperly added to heapSize in recordSize
method.
assertEquals(cb3.heapSize(),cb7.heapSize());
```
##########
core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/lru/CachedBlockQueue.java:
##########
@@ -96,20 +96,7 @@ public CachedBlock[] get() {
while (!queue.isEmpty()) {
blocks.addFirst(queue.poll());
}
- return blocks.toArray(new CachedBlock[blocks.size()]);
- }
-
- /**
- * Get a sorted List of all elements in this queue, in descending order.
- *
- * @return list of cached elements in descending order
- */
- public LinkedList<CachedBlock> getList() {
- LinkedList<CachedBlock> blocks = new LinkedList<>();
- while (!queue.isEmpty()) {
- blocks.addFirst(queue.poll());
- }
- return blocks;
+ return blocks.toArray(new CachedBlock[0]);
Review Comment:
```suggestion
return blocks.toArray(new CachedBlock[blocks.size()]);
```
--
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]
