[
https://issues.apache.org/jira/browse/GEODE-8864?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17293820#comment-17293820
]
ASF GitHub Bot commented on GEODE-8864:
---------------------------------------
jhutchison commented on a change in pull request #5954:
URL: https://github.com/apache/geode/pull/5954#discussion_r585714865
##########
File path:
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/hash/AbstractHScanIntegrationTest.java
##########
@@ -336,48 +441,136 @@ public void
givenMultipleCountsAndMatches_returnsEntriesMatchingLastMatchParamet
}
@Test
- public void givenNegativeCursor_returnsEntriesUsingAbsoluteValueOfCursor() {
- Map<String, String> entryMap = new HashMap<>();
- entryMap.put("1", "yellow");
- entryMap.put("2", "green");
- entryMap.put("3", "orange");
- jedis.hmset("colors", entryMap);
+ public void should_notReturnValue_givenValueWasRemovedBeforeHSCANISCalled() {
- String cursor = "-100";
- ScanResult<Map.Entry<String, String>> result;
- List<Map.Entry<String, String>> allEntries = new ArrayList<>();
+ Map<String, String> data = new HashMap<>();
+ data.put("field_1", "yellow");
+ data.put("field_2", "green");
+ data.put("field_3", "grey");
+ jedis.hmset("colors", data);
- do {
- result = jedis.hscan("colors", cursor);
- allEntries.addAll(result.getResult());
- cursor = result.getCursor();
- } while (!result.isCompleteIteration());
+ jedis.hdel("colors", "field_3");
+ data.remove("field_3");
- assertThat(allEntries).hasSize(3);
- assertThat(new HashSet<>(allEntries)).isEqualTo(entryMap.entrySet());
+ GeodeAwaitility.await().untilAsserted(
+ () -> assertThat(jedis.hget("colors", "field_3")).isNull());
+
+ ScanResult<Map.Entry<String, String>> result = jedis.hscan("colors", "0");
+
+ assertThat(new HashSet<>(result.getResult()))
+ .containsExactlyInAnyOrderElementsOf(data.entrySet());
}
@Test
- public void givenCursorGreaterThanUnsignedLongCapacity_returnsCursorError() {
- assertThatThrownBy(() -> jedis.hscan("a", "18446744073709551616"))
- .hasMessageContaining(ERROR_CURSOR);
+ public void should_retun_not_error_given_non_zero_cursor_on_first_call() {
+
+ Map<String, String> data = new HashMap<>();
+ data.put("field_1", "yellow");
+ data.put("field_2", "green");
+ data.put("field_3", "grey");
+ jedis.hmset("colors", data);
+
+
+ ScanResult<Map.Entry<String, String>> result = jedis.hscan("colors", "5");
+
+ assertThat(new HashSet<>(result.getResult()))
+ .containsExactlyInAnyOrderElementsOf(data.entrySet());
}
+ /**** Concurrency ***/
+
+ private final int SIZE_OF_INITIAL_HASH_DATA = 100;
+ final String HASH_KEY = "key";
+ final String BASE_FIELD = "baseField_";
+
@Test
- public void
givenNegativeCursorGreaterThanUnsignedLongCapacity_returnsCursorError() {
- assertThatThrownBy(() -> jedis.hscan("a", "-18446744073709551616"))
- .hasMessageContaining(ERROR_CURSOR);
+ public void
should_notLoseFields_givenConcurrentThreadsDoingHScansAndChangingValues() {
+ final Map<String, String> INITIAL_HASH_DATA =
makeEntrySet(SIZE_OF_INITIAL_HASH_DATA);
+ jedis.hset(HASH_KEY, INITIAL_HASH_DATA);
+ final int ITERATION_COUNT = 500;
+
+ new ConcurrentLoopingThreads(ITERATION_COUNT,
+ (i) -> multipleHScanAndAssertOnSizeOfResultSet(jedis,
INITIAL_HASH_DATA),
+ (i) -> multipleHScanAndAssertOnSizeOfResultSet(jedis2,
INITIAL_HASH_DATA),
+ (i) -> {
+ int fieldSuffix = i % SIZE_OF_INITIAL_HASH_DATA;
+ jedis3.hset(HASH_KEY, BASE_FIELD + fieldSuffix, "new_value_" + i);
+ }).run();
+
}
@Test
- public void givenInvalidRegexSyntax_returnsEmptyArray() {
- jedis.hset("a", "1", "green");
- ScanParams scanParams = new ScanParams();
- scanParams.count(1);
- scanParams.match("\\p");
+ public void
should_notLoseKeysForConsistentlyPresentFields_givenConcurrentThreadsAddingAndRemovingFields()
{
+ final Map<String, String> INITIAL_HASH_DATA =
makeEntrySet(SIZE_OF_INITIAL_HASH_DATA);
+ jedis.hset(HASH_KEY, INITIAL_HASH_DATA);
+ final int ITERATION_COUNT = 500;
+
+ new ConcurrentLoopingThreads(ITERATION_COUNT,
+ (i) -> multipleHScanAndAssertOnContentOfResultSet(jedis,
INITIAL_HASH_DATA),
+ (i) -> multipleHScanAndAssertOnContentOfResultSet(jedis2,
INITIAL_HASH_DATA),
+ (i) -> {
+ String field = "new_" + BASE_FIELD + i;
+ jedis3.hset(HASH_KEY, field, "whatever");
+ jedis3.hdel(HASH_KEY, field);
+ }).run();
- ScanResult<Map.Entry<String, String>> result = jedis.hscan("a", "0",
scanParams);
+ }
- assertThat(result.getResult()).isEmpty();
+ @Test
+ public void should_notAlterUnderlyingData_givenMultipleConcurrentHscans() {
+ final Map<String, String> INITIAL_HASH_DATA =
makeEntrySet(SIZE_OF_INITIAL_HASH_DATA);
+ jedis.hset(HASH_KEY, INITIAL_HASH_DATA);
+ final int ITERATION_COUNT = 500;
+
+ new ConcurrentLoopingThreads(ITERATION_COUNT,
+ (i) -> multipleHScanAndAssertOnContentOfResultSet(jedis,
INITIAL_HASH_DATA),
+ (i) -> multipleHScanAndAssertOnContentOfResultSet(jedis2,
INITIAL_HASH_DATA));
+
+ INITIAL_HASH_DATA.forEach((field, value) -> {
+ assertThat(jedis3.hget(HASH_KEY, field).equals(value));
+ });
+
+ }
+
+ private void multipleHScanAndAssertOnContentOfResultSet(Jedis jedis,
+ final Map<String, String> intialHashData) {
+
+ List<String> allEntries = new ArrayList<>();
+ ScanResult<Map.Entry<String, String>> result;
+ String cursor = "0";
+
+ do {
+ result = jedis.hscan(HASH_KEY, cursor);
+ cursor = result.getCursor();
+ List<Map.Entry<String, String>> resultEntries = result.getResult();
+ resultEntries
+ .forEach((entry) -> allEntries.add(entry.getKey()));
+ } while (!result.isCompleteIteration());
+
+ assertThat(allEntries).containsAll(intialHashData.keySet());
+ }
+
+ private void multipleHScanAndAssertOnSizeOfResultSet(Jedis jedis,
+ final Map<String, String> intialHashData) {
+ List<Map.Entry<String, String>> allEntries = new ArrayList<>();
+ ScanResult<Map.Entry<String, String>> result;
+ String cursor = "0";
+
+ do {
+ result = jedis.hscan(HASH_KEY, cursor);
+ cursor = result.getCursor();
+ allEntries.addAll(result.getResult());
+ } while (!result.isCompleteIteration());
+
+ assertThat(allEntries.size())
+ .isEqualTo(intialHashData.size());
Review comment:
added logic to remove duplicates form the list before assertion. Good
catch
----------------------------------------------------------------
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:
us...@infra.apache.org
> finish implementation of Redis HScan Command
> --------------------------------------------
>
> Key: GEODE-8864
> URL: https://issues.apache.org/jira/browse/GEODE-8864
> Project: Geode
> Issue Type: New Feature
> Components: redis
> Reporter: John Hutchison
> Priority: Major
> Labels: blocks-1.14.0, pull-request-available
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
