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/20 17:58:25 UTC
[GitHub] [ignite] Erixonich opened a new pull request #9023: IGNITE-14469 Adding a list of caches that will not be forced to rebuild indexes to the control.sh --cache indexes_force_rebuild
Erixonich opened a new pull request #9023:
URL: https://github.com/apache/ignite/pull/9023
Thank you for submitting the pull request to the Apache Ignite.
In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:
### The Contribution Checklist
- [ ] There is a single JIRA ticket related to the pull request.
- [ ] The web-link to the pull request is attached to the JIRA ticket.
- [ ] The JIRA ticket has the _Patch Available_ state.
- [ ] The pull request body describes changes that have been made.
The description explains _WHAT_ and _WHY_ was made instead of _HOW_.
- [ ] The pull request title is treated as the final commit message.
The following pattern must be used: `IGNITE-XXXX Change summary` where `XXXX` - number of JIRA issue.
- [ ] A reviewer has been mentioned through the JIRA comments
(see [the Maintainers list](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-ReviewProcessandMaintainers))
- [ ] The pull request has been checked by the Teamcity Bot and
the `green visa` attached to the JIRA ticket (see [TC.Bot: Check PR](https://mtcga.gridgain.com/prs.html))
### Notes
- [How to Contribute](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute)
- [Coding abbreviation rules](https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules)
- [Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines)
- [Apache Ignite Teamcity Bot](https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+Teamcity+Bot)
If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com _#ignite_ channel.
--
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
[GitHub] [ignite] Erixonich closed pull request #9023: IGNITE-14469 Add test that ensures correctness of sequential call index_force_rebuild, some refactoring
Posted by GitBox <gi...@apache.org>.
Erixonich closed pull request #9023:
URL: https://github.com/apache/ignite/pull/9023
--
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
[GitHub] [ignite] Erixonich commented on a change in pull request #9023: IGNITE-14469 Add test that ensures correctness of sequential call index_force_rebuild, some refactoring
Posted by GitBox <gi...@apache.org>.
Erixonich commented on a change in pull request #9023:
URL: https://github.com/apache/ignite/pull/9023#discussion_r621502685
##########
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:
Substitution with Map<String, List<String>> makes body of method complicated. I think removing Multimap to simplify method makes it more complicated
--
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
[GitHub] [ignite] Erixonich commented on pull request #9023: IGNITE-14469 Add test that ensures correctness of sequential call index_force_rebuild, some refactoring
Posted by GitBox <gi...@apache.org>.
Erixonich commented on pull request #9023:
URL: https://github.com/apache/ignite/pull/9023#issuecomment-828542700
Closing in favor for #9061 (wrong branch name in this)
--
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
[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
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [ignite] Erixonich commented on a change in pull request #9023: IGNITE-14469 Add test that ensures correctness of sequential call index_force_rebuild, some refactoring
Posted by GitBox <gi...@apache.org>.
Erixonich commented on a change in pull request #9023:
URL: https://github.com/apache/ignite/pull/9023#discussion_r621504481
##########
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:
Unfortunately, java.util.Map and com.google.common.collect.Multimap not compatible
--
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