You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/04/23 06:58:55 UTC
[GitHub] [ignite] tkalkirill commented on a change in pull request #9023: IGNITE-14469 Add test that ensures correctness of sequential call index_force_rebuild, some refactoring
tkalkirill commented on a change in pull request #9023:
URL: https://github.com/apache/ignite/pull/9023#discussion_r618962285
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/visor/cache/index/IndexForceRebuildTask.java
##########
@@ -68,111 +67,99 @@ protected IndexForceRebuildJob(@Nullable IndexForceRebuildTaskArg arg, boolean d
throws IgniteException
{
//Either cacheNames or cacheGrps must be specified
Review comment:
// Either cacheNames or cacheGrps must be specified.
##########
File path: modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerIndexForceRebuildTest.java
##########
@@ -476,40 +507,79 @@ public void testCorruptedIndexRebuild() throws Exception {
}
/**
- * Validated control.sh utility output for {@link #testCacheNamesArg()}.
+ * Validates control.sh output when caches by name not found.
+ *
+ * @param outputStr control.sh output.
+ * @param cacheGroupsToNames maping cache groups to non-existing cache names.
*/
- private void validateTestCacheNamesArgOutput() {
- String outputStr = testOut.toString();
-
- assertTrue(outputStr.contains("WARNING: These caches were not found:\n" +
- " " + CACHE_NAME_NON_EXISTING));
+ private void validateOutputCacheNamesNotFound(String outputStr, Multimap<String, String> cacheGroupsToNames) {
Review comment:
Same as for **validateOutputCacheGroupsNotFound**
##########
File path: modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerIndexForceRebuildTest.java
##########
@@ -476,40 +507,79 @@ public void testCorruptedIndexRebuild() throws Exception {
}
/**
- * Validated control.sh utility output for {@link #testCacheNamesArg()}.
+ * Validates control.sh output when caches by name not found.
+ *
+ * @param outputStr control.sh output.
+ * @param cacheGroupsToNames maping cache groups to non-existing cache names.
*/
- private void validateTestCacheNamesArgOutput() {
- String outputStr = testOut.toString();
-
- assertTrue(outputStr.contains("WARNING: These caches were not found:\n" +
- " " + CACHE_NAME_NON_EXISTING));
+ private void validateOutputCacheNamesNotFound(String outputStr, Multimap<String, String> cacheGroupsToNames) {
+ String cacheNames = INDENT + String.join(INDENT, cacheGroupsToNames.values());
- assertTrue(outputStr.contains("WARNING: These caches have indexes rebuilding in progress:\n" +
- " groupName=" + GRP_NAME_2 + ", cacheName=" + CACHE_NAME_2_1));
+ assertContains(
+ log,
+ outputStr,
+ "WARNING: These caches were not found:\n" + cacheNames
+ );
+ }
- assertTrue(outputStr.contains("Indexes rebuild was started for these caches:\n" +
- " groupName=" + GRP_NAME_1 + ", cacheName=" + CACHE_NAME_1_1));
+ /**
+ * Validates control.sh output when caches by group not found.
+ *
+ * @param outputStr control.sh output.
+ * @param cacheGroupsToNames maping cache groups to non-existing cache names.
+ */
+ private void validateOutputCacheGroupsNotFound(String outputStr, Multimap<String, String> cacheGroupsToNames) {
+ String cacheNames = INDENT + String.join(INDENT, cacheGroupsToNames.keys());
- assertEquals("Unexpected number of lines in output.", 19, outputStr.split("\n").length);
+ assertContains(
+ log,
+ outputStr,
+ "WARNING: These cache groups were not found:\n" + cacheNames
Review comment:
\n -> U.nl();
##########
File path: modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerIndexForceRebuildTest.java
##########
@@ -476,40 +507,79 @@ public void testCorruptedIndexRebuild() throws Exception {
}
/**
- * Validated control.sh utility output for {@link #testCacheNamesArg()}.
+ * Validates control.sh output when caches by name not found.
+ *
+ * @param outputStr control.sh output.
+ * @param cacheGroupsToNames maping cache groups to non-existing cache names.
*/
- private void validateTestCacheNamesArgOutput() {
- String outputStr = testOut.toString();
-
- assertTrue(outputStr.contains("WARNING: These caches were not found:\n" +
- " " + CACHE_NAME_NON_EXISTING));
+ private void validateOutputCacheNamesNotFound(String outputStr, Multimap<String, String> cacheGroupsToNames) {
+ String cacheNames = INDENT + String.join(INDENT, cacheGroupsToNames.values());
- assertTrue(outputStr.contains("WARNING: These caches have indexes rebuilding in progress:\n" +
- " groupName=" + GRP_NAME_2 + ", cacheName=" + CACHE_NAME_2_1));
+ assertContains(
+ log,
+ outputStr,
+ "WARNING: These caches were not found:\n" + cacheNames
+ );
+ }
- assertTrue(outputStr.contains("Indexes rebuild was started for these caches:\n" +
- " groupName=" + GRP_NAME_1 + ", cacheName=" + CACHE_NAME_1_1));
+ /**
+ * Validates control.sh output when caches by group not found.
+ *
+ * @param outputStr control.sh output.
+ * @param cacheGroupsToNames maping cache groups to non-existing cache names.
+ */
+ private void validateOutputCacheGroupsNotFound(String outputStr, Multimap<String, String> cacheGroupsToNames) {
+ String cacheNames = INDENT + String.join(INDENT, cacheGroupsToNames.keys());
Review comment:
See **CacheIndexesForceRebuild#printResult**. Each cache is on a new line.
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/visor/cache/index/IndexForceRebuildTask.java
##########
@@ -68,111 +67,99 @@ protected IndexForceRebuildJob(@Nullable IndexForceRebuildTaskArg arg, boolean d
throws IgniteException
{
//Either cacheNames or cacheGrps must be specified
- assert (arg.cacheNames() == null) != (arg.cacheGrps() == null) :
+ assert (arg.cacheNames() == null) ^ (arg.cacheGrps() == null) :
"Either cacheNames or cacheGroups must be specified";
Review comment:
The code below is difficult to understand and needs to be simplified.
What needs to be done here:
- Collect contexts for which you want to rebuild indexes;
- Determine for which caches / groups contexts were not found;
- Send contexts for rebuilding via **IgniteCacheDatabaseSharedManager#forceRebuildIndexes**, its return value will allow you to understand for which contexts the rebuilding has not started since it is already in the process;
- Send a response to the utility.
By the name of the cache, you can get the contexts like this:
- ignite.cachex("cacheName").context();
- ignite.context().cache().cache("cacheName").context();
For the group id like this:
- ignite.context().cache().cacheGroup(0).caches()
##########
File path: modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerIndexForceRebuildTest.java
##########
@@ -279,7 +293,24 @@ public void testGroupNamesArg() throws Exception {
waitForIndexesRebuild(grid(LAST_NODE_NUM));
- validateTestCacheGroupArgOutput();
+ String outputStr = testOut.toString();
+
+ validateOutputCacheGroupsNotFound(
+ outputStr,
+ ImmutableMultimap.of(GRP_NAME_NON_EXISTING, CACHE_NAME_NON_EXISTING)
+ );
+
+ validateOutputIndicesRebuildingInProgress(outputStr, ImmutableMultimap.of(GRP_NAME_1, CACHE_NAME_1_2));
Review comment:
You can just use a java.util.Collections#singletonMap or F.asMap(K, V, K, V)
##########
File path: modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerIndexForceRebuildTest.java
##########
@@ -476,40 +507,79 @@ public void testCorruptedIndexRebuild() throws Exception {
}
/**
- * Validated control.sh utility output for {@link #testCacheNamesArg()}.
+ * Validates control.sh output when caches by name not found.
+ *
+ * @param outputStr control.sh output.
+ * @param cacheGroupsToNames maping cache groups to non-existing cache names.
*/
- private void validateTestCacheNamesArgOutput() {
- String outputStr = testOut.toString();
-
- assertTrue(outputStr.contains("WARNING: These caches were not found:\n" +
- " " + CACHE_NAME_NON_EXISTING));
+ private void validateOutputCacheNamesNotFound(String outputStr, Multimap<String, String> cacheGroupsToNames) {
+ String cacheNames = INDENT + String.join(INDENT, cacheGroupsToNames.values());
- assertTrue(outputStr.contains("WARNING: These caches have indexes rebuilding in progress:\n" +
- " groupName=" + GRP_NAME_2 + ", cacheName=" + CACHE_NAME_2_1));
+ assertContains(
+ log,
+ outputStr,
+ "WARNING: These caches were not found:\n" + cacheNames
+ );
+ }
- assertTrue(outputStr.contains("Indexes rebuild was started for these caches:\n" +
- " groupName=" + GRP_NAME_1 + ", cacheName=" + CACHE_NAME_1_1));
+ /**
+ * Validates control.sh output when caches by group not found.
+ *
+ * @param outputStr control.sh output.
+ * @param cacheGroupsToNames maping cache groups to non-existing cache names.
+ */
+ private void validateOutputCacheGroupsNotFound(String outputStr, Multimap<String, String> cacheGroupsToNames) {
Review comment:
Just use a collection or varargs.
##########
File path: modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerIndexForceRebuildTest.java
##########
@@ -476,40 +507,79 @@ public void testCorruptedIndexRebuild() throws Exception {
}
/**
- * Validated control.sh utility output for {@link #testCacheNamesArg()}.
+ * Validates control.sh output when caches by name not found.
+ *
+ * @param outputStr control.sh output.
+ * @param cacheGroupsToNames maping cache groups to non-existing cache names.
*/
- private void validateTestCacheNamesArgOutput() {
- String outputStr = testOut.toString();
-
- assertTrue(outputStr.contains("WARNING: These caches were not found:\n" +
- " " + CACHE_NAME_NON_EXISTING));
+ private void validateOutputCacheNamesNotFound(String outputStr, Multimap<String, String> cacheGroupsToNames) {
+ String cacheNames = INDENT + String.join(INDENT, cacheGroupsToNames.values());
- assertTrue(outputStr.contains("WARNING: These caches have indexes rebuilding in progress:\n" +
- " groupName=" + GRP_NAME_2 + ", cacheName=" + CACHE_NAME_2_1));
+ assertContains(
+ log,
+ outputStr,
+ "WARNING: These caches were not found:\n" + cacheNames
+ );
+ }
- assertTrue(outputStr.contains("Indexes rebuild was started for these caches:\n" +
- " groupName=" + GRP_NAME_1 + ", cacheName=" + CACHE_NAME_1_1));
+ /**
+ * Validates control.sh output when caches by group not found.
+ *
+ * @param outputStr control.sh output.
+ * @param cacheGroupsToNames maping cache groups to non-existing cache names.
+ */
+ private void validateOutputCacheGroupsNotFound(String outputStr, Multimap<String, String> cacheGroupsToNames) {
+ String cacheNames = INDENT + String.join(INDENT, cacheGroupsToNames.keys());
- assertEquals("Unexpected number of lines in output.", 19, outputStr.split("\n").length);
+ assertContains(
+ log,
+ outputStr,
+ "WARNING: These cache groups were not found:\n" + cacheNames
+ );
}
/**
- * Validated control.sh utility output for {@link #testGroupNamesArg()}.
+ * Makes formatted text for given caches.
+ *
+ * @param cacheGroputToNames Cache groups mapping to non-existing cache names.
+ * @return text for CLI print output for given caches.
*/
- private void validateTestCacheGroupArgOutput() {
- String outputStr = testOut.toString();
+ private String makeStringListForCacheGroupsAndNames(Multimap<String, String> cacheGroputToNames) {
Review comment:
You can just use a java.util,Map
--
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:
users@infra.apache.org