busbey commented on a change in pull request #2685:
URL: https://github.com/apache/hbase/pull/2685#discussion_r529549030
##########
File path:
hbase-client/src/test/java/org/apache/hadoop/hbase/util/TestThreadLocalPoolMap.java
##########
@@ -43,42 +43,42 @@ protected PoolType getPoolType() {
}
@Test
- public void testSingleThreadedClient() throws InterruptedException,
ExecutionException {
+ public void testSingleThreadedClient() throws InterruptedException {
Random rand = ThreadLocalRandom.current();
- String randomKey = String.valueOf(rand.nextInt());
- String randomValue = String.valueOf(rand.nextInt());
- // As long as the pool is not full, we should get back what we put
- runThread(randomKey, randomValue, randomValue);
- assertEquals(1, poolMap.size(randomKey));
+ String key = "key";
+ String value = "value";
+ runThread(key, () -> value, value);
+ assertEquals(1, poolMap.values().size());
}
@Test
- public void testMultiThreadedClients() throws InterruptedException,
ExecutionException {
- Random rand = ThreadLocalRandom.current();
- // As long as the pool is not full, we should get back what we put
- for (int i = 0; i < POOL_SIZE; i++) {
- String randomKey = String.valueOf(rand.nextInt());
- String randomValue = String.valueOf(rand.nextInt());
- runThread(randomKey, randomValue, randomValue);
- assertEquals(1, poolMap.size(randomKey));
+ public void testMultiThreadedClients() throws InterruptedException {
+ for (int i = 0; i < KEY_COUNT; i++) {
+ String key = Integer.toString(i);
+ String value = Integer.toString(2 * i);
+ runThread(key, () -> value, value);
}
- String randomKey = String.valueOf(rand.nextInt());
+
+ assertEquals(KEY_COUNT, poolMap.values().size());
+ poolMap.clear();
+
+ String key = "key";
for (int i = 0; i < POOL_SIZE; i++) {
- String randomValue = String.valueOf(rand.nextInt());
- runThread(randomKey, randomValue, randomValue);
- assertEquals(i + 1, poolMap.size(randomKey));
+ String value = Integer.toString(i);
+ runThread(key, () -> value, value);
}
+
+ assertEquals(POOL_SIZE, poolMap.values().size());
}
@Test
- public void testPoolCap() throws InterruptedException, ExecutionException {
- Random rand = ThreadLocalRandom.current();
- String randomKey = String.valueOf(rand.nextInt());
+ public void testPoolCap() throws InterruptedException {
Review comment:
nit: a comment in the javadoc for this test that says "There is no pool
cap for the ThreadLocalPool" or a change in the test name to e.g.
`testNoPoolCap` would make this test easier to follow.
##########
File path:
hbase-client/src/test/java/org/apache/hadoop/hbase/util/TestRoundRobinPoolMap.java
##########
@@ -45,58 +46,53 @@ protected PoolType getPoolType() {
}
@Test
- public void testSingleThreadedClient() throws InterruptedException,
ExecutionException {
- Random rand = ThreadLocalRandom.current();
- String randomKey = String.valueOf(rand.nextInt());
- String randomValue = String.valueOf(rand.nextInt());
- // As long as the pool is not full, we'll get null back.
- // This forces the user to create new values that can be used to populate
- // the pool.
- runThread(randomKey, randomValue, null);
- assertEquals(1, poolMap.size(randomKey));
+ public void testSingleThreadedClient() throws InterruptedException {
+ String key = "key";
+ String value = "value";
+ // As long as the pool is not full, get calls the supplier
+ runThread(key, () -> value, value);
+ assertEquals(1, poolMap.values().size());
}
@Test
- public void testMultiThreadedClients() throws InterruptedException,
ExecutionException {
- Random rand = ThreadLocalRandom.current();
- for (int i = 0; i < POOL_SIZE; i++) {
- String randomKey = String.valueOf(rand.nextInt());
- String randomValue = String.valueOf(rand.nextInt());
- // As long as the pool is not full, we'll get null back
- runThread(randomKey, randomValue, null);
- // As long as we use distinct keys, each pool will have one value
- assertEquals(1, poolMap.size(randomKey));
+ public void testMultiThreadedClients() throws InterruptedException {
+ for (int i = 0; i < KEY_COUNT; i++) {
+ String key = Integer.toString(i);
+ String value = Integer.toString(2 * i);
+ // As long as the pool is not full, we'll get the supplied value back
+ runThread(key, () -> value, value);
}
+
+ assertEquals(KEY_COUNT, poolMap.values().size());
Review comment:
we should drop the "as long as the pool is not full" comment here. This
first stanza is testing "as long as we use distinct keys we should get get back
the supplied value" and "we end up with 1 value per pool".
##########
File path:
hbase-client/src/test/java/org/apache/hadoop/hbase/util/TestThreadLocalPoolMap.java
##########
@@ -43,42 +43,42 @@ protected PoolType getPoolType() {
}
@Test
- public void testSingleThreadedClient() throws InterruptedException,
ExecutionException {
+ public void testSingleThreadedClient() throws InterruptedException {
Random rand = ThreadLocalRandom.current();
- String randomKey = String.valueOf(rand.nextInt());
- String randomValue = String.valueOf(rand.nextInt());
- // As long as the pool is not full, we should get back what we put
- runThread(randomKey, randomValue, randomValue);
- assertEquals(1, poolMap.size(randomKey));
+ String key = "key";
+ String value = "value";
+ runThread(key, () -> value, value);
+ assertEquals(1, poolMap.values().size());
}
@Test
- public void testMultiThreadedClients() throws InterruptedException,
ExecutionException {
- Random rand = ThreadLocalRandom.current();
- // As long as the pool is not full, we should get back what we put
- for (int i = 0; i < POOL_SIZE; i++) {
- String randomKey = String.valueOf(rand.nextInt());
- String randomValue = String.valueOf(rand.nextInt());
- runThread(randomKey, randomValue, randomValue);
- assertEquals(1, poolMap.size(randomKey));
+ public void testMultiThreadedClients() throws InterruptedException {
+ for (int i = 0; i < KEY_COUNT; i++) {
+ String key = Integer.toString(i);
+ String value = Integer.toString(2 * i);
+ runThread(key, () -> value, value);
}
- String randomKey = String.valueOf(rand.nextInt());
+
+ assertEquals(KEY_COUNT, poolMap.values().size());
+ poolMap.clear();
Review comment:
this would be a stronger test for the `ThreadLocalPool` if we made each
of these distinct key lookups in a single thread. a TODO note for later?
----------------------------------------------------------------
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:
[email protected]
