jdeppe-pivotal commented on a change in pull request #6832:
URL: https://github.com/apache/geode/pull/6832#discussion_r701566966
##########
File path:
geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/string/AbstractGetSetIntegrationTest.java
##########
@@ -120,41 +114,20 @@ public void testGetSet_whenWrongType_shouldReturnError() {
}
@Test
- public void testGetSet_shouldBeAtomic()
- throws ExecutionException, InterruptedException {
- jedis.set("contestedKey", "0");
- assertThat(jedis.get("contestedKey")).isEqualTo("0");
- CountDownLatch latch = new CountDownLatch(1);
- ExecutorService pool = Executors.newFixedThreadPool(2);
- Callable<Integer> callable1 = () -> doABunchOfIncrs(jedis, latch);
- Callable<Integer> callable2 = () -> doABunchOfGetSets(jedis, latch);
- Future<Integer> future1 = pool.submit(callable1);
- Future<Integer> future2 = pool.submit(callable2);
-
- latch.countDown();
-
- GeodeAwaitility.await().untilAsserted(() ->
assertThat(future2.get()).isEqualTo(future1.get()));
- assertThat(future1.get() + future2.get()).isEqualTo(2 * ITERATION_COUNT);
- }
-
- private Integer doABunchOfIncrs(JedisCluster jedis, CountDownLatch latch)
- throws InterruptedException {
- latch.await();
- for (int i = 0; i < ITERATION_COUNT; i++) {
- jedis.incr("contestedKey");
- }
- return ITERATION_COUNT;
- }
-
- private Integer doABunchOfGetSets(JedisCluster jedis, CountDownLatch latch)
- throws InterruptedException {
- int sum = 0;
- latch.await();
-
- while (sum < ITERATION_COUNT) {
- sum += Integer.parseInt(jedis.getSet("contestedKey", "0"));
- }
- return sum;
+ public void testGetSet_shouldBeAtomic() {
+ String contestedKey = "contestedKey";
+ jedis.set(contestedKey, "0");
+ assertThat(jedis.get(contestedKey)).isEqualTo("0");
+
+ final AtomicInteger sumHolder = new AtomicInteger(0);
+ new ConcurrentLoopingThreads(ITERATION_COUNT,
+ (ignored) -> jedis.incr(contestedKey),
+ (i) -> {
+ // continue fetching with getSet until caught up with incrementing
+ // noinspection StatementWithEmptyBody
+ while
(sumHolder.getAndAdd(Integer.parseInt(jedis.getSet(contestedKey, "0"))) <= i);
+ }).run();
+ assertThat(sumHolder.get()).isEqualTo(ITERATION_COUNT);
Review comment:
It took me a moment to figure out the while here. Consider this as an
alternative perhaps?
```
final AtomicInteger sumHolder = new AtomicInteger(0);
new ConcurrentLoopingThreads(ITERATION_COUNT,
i -> jedis.incr(contestedKey),
i -> sumHolder.getAndAdd(Integer.parseInt(jedis.getSet(contestedKey,
"0"))))
.runInLockstep();
sumHolder.getAndAdd(Integer.parseInt(jedis.getSet(contestedKey, "0")));
assertThat(sumHolder.get()).isEqualTo(ITERATION_COUNT);
```
The `runInLockstep` will ensure that each thread is in lockstep on the
iteration so no one thread can "run ahead". Still needs a final `getSet` since
either thread could complete first.
--
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]
