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:
[email protected]


Reply via email to