tkalkirill commented on a change in pull request #9061:
URL: https://github.com/apache/ignite/pull/9061#discussion_r624000421
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheProcessor.java
##########
@@ -4463,6 +4463,7 @@ public void completeProxyInitialize(String name) {
* @param <K> type of keys.
* @param <V> type of values.
* @return Cache instance for given name.
+ * @throws IllegalArgumentException if cache not exists.
Review comment:
If cache not exists.
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/visor/cache/index/IndexForceRebuildTask.java
##########
@@ -68,111 +65,60 @@ protected IndexForceRebuildJob(@Nullable
IndexForceRebuildTaskArg arg, boolean d
throws IgniteException
{
//Either cacheNames or cacheGrps must be specified
- assert (arg.cacheNames() == null) != (arg.cacheGrps() == null) :
- "Either cacheNames or cacheGroups must be specified";
-
- // Collect info about indexes being rebuilt.
- Set<IndexRebuildStatusInfoContainer> rebuildIdxCaches =
- ignite.context().cache().publicCaches()
- .stream()
- .filter(c -> !c.indexReadyFuture().isDone())
- .map(this::fromIgniteCache)
- .collect(Collectors.toSet());
-
- Set<String> rebuildIdxCachesNames = rebuildIdxCaches.stream()
- .map(IndexRebuildStatusInfoContainer::cacheName)
- .collect(Collectors.toSet());
+ assert (arg.cacheNames() == null) ^ (arg.cacheGrps() == null) :
+ "Either cacheNames or cacheGroups must be specified.";
- if (arg.cacheNames() != null)
- return rebuildByCacheNames(arg.cacheNames(), rebuildIdxCaches,
rebuildIdxCachesNames);
- else if (arg.cacheGrps() != null)
- return rebuildByGroupNames(arg.cacheGrps(), rebuildIdxCaches,
rebuildIdxCachesNames);
- else {
- assert false : "Neither cache names nor cache groups
specified";
+ if (arg.cacheNames() == null && arg.cacheGrps() == null) {
+ assert false : "Neither cache names nor cache groups
specified.";
return null;
}
- }
-
- /**
- * Triggers force rebuild of indexes in caches from {@code cacheNames}.
- *
- * @param cacheNames Set of cache names.
- * @param rebuildIdxCaches Set of infos about cached which have
indexes being rebuilt at the moment.
- * @param rebuildIdxCachesNames Set of names of cached which have
indexes being rebuilt at the moment.
- * @return {@code IndexForceRebuildTaskRes} object.
- */
- @NotNull private IndexForceRebuildTaskRes rebuildByCacheNames(
- Set<String> cacheNames,
- Set<IndexRebuildStatusInfoContainer> rebuildIdxCaches,
- Set<String> rebuildIdxCachesNames)
- {
- final GridCacheProcessor cacheProcessor = ignite.context().cache();
- // Collect info about not found caches.
- Set<String> notFoundCaches = new HashSet<>(cacheNames);
- notFoundCaches.removeIf(name -> cacheProcessor.cache(name) !=
null);
-
- Set<GridCacheContext> cacheContexts =
- cacheProcessor.publicCaches()
- .stream()
- .filter(c -> !rebuildIdxCachesNames.contains(c.getName()))
- .filter(c -> cacheNames.contains(c.getName()))
- .map(IgniteCacheProxy::context)
- .collect(Collectors.toSet());
+ Set<GridCacheContext> cachesToRebuild = new HashSet<>();
+ Set<String> notFound = new HashSet<>();
- // Collect info about started index rebuild.
- Set<IndexRebuildStatusInfoContainer> cachesWithStartedRebuild =
- cacheContexts.stream()
- .map(c -> new IndexRebuildStatusInfoContainer(c.config()))
- .collect(Collectors.toSet());
+ final GridCacheProcessor cacheProcessor = ignite.context().cache();
-
cacheProcessor.context().database().forceRebuildIndexes(cacheContexts);
+ if (arg.cacheNames() != null) {
+ for (String cacheName : arg.cacheNames()) {
+ IgniteInternalCache<Object, Object> cache =
cacheProcessor.cache(cacheName);
Review comment:
<Object, Object> - not needed
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/visor/cache/index/IndexForceRebuildTask.java
##########
@@ -68,111 +65,60 @@ protected IndexForceRebuildJob(@Nullable
IndexForceRebuildTaskArg arg, boolean d
throws IgniteException
{
//Either cacheNames or cacheGrps must be specified
- assert (arg.cacheNames() == null) != (arg.cacheGrps() == null) :
- "Either cacheNames or cacheGroups must be specified";
-
- // Collect info about indexes being rebuilt.
- Set<IndexRebuildStatusInfoContainer> rebuildIdxCaches =
- ignite.context().cache().publicCaches()
- .stream()
- .filter(c -> !c.indexReadyFuture().isDone())
- .map(this::fromIgniteCache)
- .collect(Collectors.toSet());
-
- Set<String> rebuildIdxCachesNames = rebuildIdxCaches.stream()
- .map(IndexRebuildStatusInfoContainer::cacheName)
- .collect(Collectors.toSet());
+ assert (arg.cacheNames() == null) ^ (arg.cacheGrps() == null) :
+ "Either cacheNames or cacheGroups must be specified.";
- if (arg.cacheNames() != null)
- return rebuildByCacheNames(arg.cacheNames(), rebuildIdxCaches,
rebuildIdxCachesNames);
- else if (arg.cacheGrps() != null)
- return rebuildByGroupNames(arg.cacheGrps(), rebuildIdxCaches,
rebuildIdxCachesNames);
- else {
- assert false : "Neither cache names nor cache groups
specified";
+ if (arg.cacheNames() == null && arg.cacheGrps() == null) {
+ assert false : "Neither cache names nor cache groups
specified.";
return null;
}
- }
-
- /**
- * Triggers force rebuild of indexes in caches from {@code cacheNames}.
- *
- * @param cacheNames Set of cache names.
- * @param rebuildIdxCaches Set of infos about cached which have
indexes being rebuilt at the moment.
- * @param rebuildIdxCachesNames Set of names of cached which have
indexes being rebuilt at the moment.
- * @return {@code IndexForceRebuildTaskRes} object.
- */
- @NotNull private IndexForceRebuildTaskRes rebuildByCacheNames(
- Set<String> cacheNames,
- Set<IndexRebuildStatusInfoContainer> rebuildIdxCaches,
- Set<String> rebuildIdxCachesNames)
- {
- final GridCacheProcessor cacheProcessor = ignite.context().cache();
- // Collect info about not found caches.
- Set<String> notFoundCaches = new HashSet<>(cacheNames);
- notFoundCaches.removeIf(name -> cacheProcessor.cache(name) !=
null);
-
- Set<GridCacheContext> cacheContexts =
- cacheProcessor.publicCaches()
- .stream()
- .filter(c -> !rebuildIdxCachesNames.contains(c.getName()))
- .filter(c -> cacheNames.contains(c.getName()))
- .map(IgniteCacheProxy::context)
- .collect(Collectors.toSet());
+ Set<GridCacheContext> cachesToRebuild = new HashSet<>();
+ Set<String> notFound = new HashSet<>();
- // Collect info about started index rebuild.
- Set<IndexRebuildStatusInfoContainer> cachesWithStartedRebuild =
- cacheContexts.stream()
- .map(c -> new IndexRebuildStatusInfoContainer(c.config()))
- .collect(Collectors.toSet());
+ final GridCacheProcessor cacheProcessor = ignite.context().cache();
-
cacheProcessor.context().database().forceRebuildIndexes(cacheContexts);
+ if (arg.cacheNames() != null) {
+ for (String cacheName : arg.cacheNames()) {
+ IgniteInternalCache<Object, Object> cache =
cacheProcessor.cache(cacheName);
- return new IndexForceRebuildTaskRes(cachesWithStartedRebuild,
rebuildIdxCaches, notFoundCaches);
- }
+ if (cache != null)
+ cachesToRebuild.add(cache.context());
+ else
+ notFound.add(cacheName);
+ }
+ }
+ else { //arg.cacheGrps() != null
Review comment:
//arg.cacheGrps() != null - not needed
##########
File path:
modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerIndexForceRebuildTest.java
##########
@@ -476,40 +500,95 @@ 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 cacheNames cache names to print.
+ */
+ private void validateOutputCacheNamesNotFound(String outputStr, String...
cacheNames) {
+ assertContains(
+ log,
+ outputStr,
+ "WARNING: These caches were not found:" + U.nl() +
makeStringListWithIndent(cacheNames)
+ );
+ }
+
+ /**
+ * Validates control.sh output when caches by group not found.
+ *
+ * @param outputStr control.sh output.
+ * @param cacheGrps cache groups to print.
+ */
+ private void validateOutputCacheGroupsNotFound(String outputStr, String...
cacheGrps) {
+ assertContains(
+ log,
+ outputStr,
+ "WARNING: These cache groups were not found:" + U.nl() +
makeStringListWithIndent(cacheGrps)
+ );
+ }
+
+ /**
+ * Makes new-line list with indent.
+ * @param strings list of strings.
+ * @return formated text.
*/
- private void validateTestCacheNamesArgOutput() {
- String outputStr = testOut.toString();
+ private String makeStringListWithIndent(String... strings) {
+ return INDENT + String.join(U.nl() + INDENT, strings);
+ }
- assertTrue(outputStr.contains("WARNING: These caches were not
found:\n" +
- " " + CACHE_NAME_NON_EXISTING));
+ /**
+ * 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 String makeStringListForCacheGroupsAndNames(Map<String,
List<String>> cacheGroputToNames) {
+ StringBuilder sb = new StringBuilder();
Review comment:
You can use org.apache.ignite.internal.util.typedef.internal.SB
##########
File path:
modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerIndexForceRebuildTest.java
##########
@@ -476,40 +500,95 @@ 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 cacheNames cache names to print.
+ */
+ private void validateOutputCacheNamesNotFound(String outputStr, String...
cacheNames) {
+ assertContains(
+ log,
+ outputStr,
+ "WARNING: These caches were not found:" + U.nl() +
makeStringListWithIndent(cacheNames)
+ );
+ }
+
+ /**
+ * Validates control.sh output when caches by group not found.
+ *
+ * @param outputStr control.sh output.
+ * @param cacheGrps cache groups to print.
+ */
+ private void validateOutputCacheGroupsNotFound(String outputStr, String...
cacheGrps) {
+ assertContains(
+ log,
+ outputStr,
+ "WARNING: These cache groups were not found:" + U.nl() +
makeStringListWithIndent(cacheGrps)
+ );
+ }
+
+ /**
+ * Makes new-line list with indent.
+ * @param strings list of strings.
+ * @return formated text.
*/
- private void validateTestCacheNamesArgOutput() {
- String outputStr = testOut.toString();
+ private String makeStringListWithIndent(String... strings) {
+ return INDENT + String.join(U.nl() + INDENT, strings);
+ }
- assertTrue(outputStr.contains("WARNING: These caches were not
found:\n" +
- " " + CACHE_NAME_NON_EXISTING));
+ /**
+ * 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.
Review comment:
With a capital letter.
##########
File path:
modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerIndexForceRebuildTest.java
##########
@@ -476,40 +500,95 @@ 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 cacheNames cache names to print.
+ */
+ private void validateOutputCacheNamesNotFound(String outputStr, String...
cacheNames) {
+ assertContains(
+ log,
+ outputStr,
+ "WARNING: These caches were not found:" + U.nl() +
makeStringListWithIndent(cacheNames)
+ );
+ }
+
+ /**
+ * Validates control.sh output when caches by group not found.
+ *
+ * @param outputStr control.sh output.
Review comment:
Capitalized argument names.
##########
File path:
modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerIndexForceRebuildTest.java
##########
@@ -476,40 +500,95 @@ 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.
Review comment:
Capitalized argument names.
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/visor/cache/index/IndexForceRebuildTask.java
##########
@@ -68,111 +65,60 @@ protected IndexForceRebuildJob(@Nullable
IndexForceRebuildTaskArg arg, boolean d
throws IgniteException
{
//Either cacheNames or cacheGrps must be specified
- assert (arg.cacheNames() == null) != (arg.cacheGrps() == null) :
- "Either cacheNames or cacheGroups must be specified";
-
- // Collect info about indexes being rebuilt.
- Set<IndexRebuildStatusInfoContainer> rebuildIdxCaches =
- ignite.context().cache().publicCaches()
- .stream()
- .filter(c -> !c.indexReadyFuture().isDone())
- .map(this::fromIgniteCache)
- .collect(Collectors.toSet());
-
- Set<String> rebuildIdxCachesNames = rebuildIdxCaches.stream()
- .map(IndexRebuildStatusInfoContainer::cacheName)
- .collect(Collectors.toSet());
+ assert (arg.cacheNames() == null) ^ (arg.cacheGrps() == null) :
+ "Either cacheNames or cacheGroups must be specified.";
- if (arg.cacheNames() != null)
- return rebuildByCacheNames(arg.cacheNames(), rebuildIdxCaches,
rebuildIdxCachesNames);
- else if (arg.cacheGrps() != null)
- return rebuildByGroupNames(arg.cacheGrps(), rebuildIdxCaches,
rebuildIdxCachesNames);
- else {
- assert false : "Neither cache names nor cache groups
specified";
+ if (arg.cacheNames() == null && arg.cacheGrps() == null) {
+ assert false : "Neither cache names nor cache groups
specified.";
Review comment:
Don't do that: **assert false**
##########
File path:
modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerIndexForceRebuildTest.java
##########
@@ -657,4 +736,54 @@ private void removeLogListener(IgniteEx ignite,
LogListener lsnr) {
return idxRebuildFuts.get(cacheId);
}
+
+ /**
+ * Force rebuilds indices for chosen caches, and waits until rebuild
process is complete.
+ *
+ * @param cacheNames Cache names need indices to rebuild.
+ * @param grid Ignite node.
+ * @throws Exception if failed.
+ */
+ private void forceRebuildIndices(Iterable<String> cacheNames, IgniteEx
grid) throws Exception {
+ String cacheNamesArg = String.join(",", cacheNames);
+
+ assertEquals(
+ EXIT_CODE_OK,
+ execute(
+ "--cache", "indexes_force_rebuild",
+ "--node-id", grid.localNode().id().toString(),
+ "--cache-names", cacheNamesArg
+ )
+ );
+
+ waitForIndexesRebuild(grid, getTestTimeout(), Collections.emptyList());
+ }
+
+ /**
+ * Checking that a sequence of forced rebuild of indexes is possible
+ *
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testSequentialForceRebuildIndexes() throws Exception {
Review comment:
Let the test methods be close to each other, move to line 502.
##########
File path:
modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerIndexForceRebuildTest.java
##########
@@ -657,4 +736,54 @@ private void removeLogListener(IgniteEx ignite,
LogListener lsnr) {
return idxRebuildFuts.get(cacheId);
}
+
+ /**
+ * Force rebuilds indices for chosen caches, and waits until rebuild
process is complete.
+ *
+ * @param cacheNames Cache names need indices to rebuild.
+ * @param grid Ignite node.
+ * @throws Exception if failed.
Review comment:
With a capital letter.
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/visor/cache/index/IndexForceRebuildTask.java
##########
@@ -68,111 +65,60 @@ protected IndexForceRebuildJob(@Nullable
IndexForceRebuildTaskArg arg, boolean d
throws IgniteException
{
//Either cacheNames or cacheGrps must be specified
- assert (arg.cacheNames() == null) != (arg.cacheGrps() == null) :
- "Either cacheNames or cacheGroups must be specified";
-
- // Collect info about indexes being rebuilt.
- Set<IndexRebuildStatusInfoContainer> rebuildIdxCaches =
- ignite.context().cache().publicCaches()
- .stream()
- .filter(c -> !c.indexReadyFuture().isDone())
- .map(this::fromIgniteCache)
- .collect(Collectors.toSet());
-
- Set<String> rebuildIdxCachesNames = rebuildIdxCaches.stream()
- .map(IndexRebuildStatusInfoContainer::cacheName)
- .collect(Collectors.toSet());
+ assert (arg.cacheNames() == null) ^ (arg.cacheGrps() == null) :
+ "Either cacheNames or cacheGroups must be specified.";
- if (arg.cacheNames() != null)
- return rebuildByCacheNames(arg.cacheNames(), rebuildIdxCaches,
rebuildIdxCachesNames);
- else if (arg.cacheGrps() != null)
- return rebuildByGroupNames(arg.cacheGrps(), rebuildIdxCaches,
rebuildIdxCachesNames);
- else {
- assert false : "Neither cache names nor cache groups
specified";
+ if (arg.cacheNames() == null && arg.cacheGrps() == null) {
Review comment:
This condition will not work because there is a XOR above:
https://en.wikipedia.org/wiki/Exclusive_or#Truth_table
--
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]
