timoninmaxim commented on code in PR #12608:
URL: https://github.com/apache/ignite/pull/12608#discussion_r2686163735
##########
modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerWalTest.java:
##########
@@ -173,33 +173,117 @@ public void testWalStateInMemoryCdcCluster() throws
Exception {
}
/**
- * Test WAL mode change for a cache group contains multiple caches.
+ * Test for WAL enable/disable commands.
* @throws Exception If failed.
*/
@Test
- public void testWalChangeForMultiCacheGroup() throws Exception {
+ public void testWalManagementOperations() throws Exception {
clusterState = 0; // PDS cluster.
IgniteEx srv = startGrids(2);
srv.cluster().state(ClusterState.ACTIVE);
srv.createCache(new CacheConfiguration<>("cache1")
- .setGroupName("testGroup"));
+ .setGroupName("group1"));
srv.createCache(new CacheConfiguration<>("cache2")
- .setGroupName("testGroup"));
+ .setGroupName("group1"));
+ srv.createCache(new CacheConfiguration<>("cache3")
+ .setGroupName("group2"));
+ srv.createCache("cache4");
+
+ assertEquals(EXIT_CODE_OK, execute("--wal", "state", "--groups",
"group1,group2,cache4"));
+ outputContains(".*group1.*true.*true.*true.*true.*false");
+ outputContains(".*group2.*true.*true.*true.*true.*false");
+ outputContains(".*cache4.*true.*true.*true.*true.*false");
+
+ assertEquals(EXIT_CODE_OK, execute("--wal", "disable", "--groups",
"group1"));
+ outputContains("Successfully disabled WAL for groups:");
+ outputContains("group1");
+
+ assertEquals(EXIT_CODE_OK, execute("--wal", "state", "--groups",
"group1"));
+ outputContains(".*group1.*true.*false.*true.*true.*false");
+
+ assertEquals(EXIT_CODE_OK, execute("--wal", "disable", "--groups",
"group1,group2"));
+ outputContains("Successfully disabled WAL for groups:");
+ outputContains("group1");
+ outputContains("group2");
+
+ assertEquals(EXIT_CODE_OK, execute("--wal", "state", "--groups",
"group1,group2"));
+ outputContains(".*group1.*true.*false.*true.*true.*false");
+ outputContains(".*group2.*true.*false.*true.*true.*false");
+
+ assertEquals(EXIT_CODE_OK, execute("--wal", "disable", "--groups",
"cache4,nonExistentGroup"));
+ outputContains("Successfully disabled WAL for groups:");
+ outputContains("cache4");
+ outputContains("Errors occurred:");
+ outputContains("Cache group not found: nonExistentGroup");
+
+ assertEquals(EXIT_CODE_OK, execute("--wal", "state", "--groups",
"cache4"));
+ outputContains(".*cache4.*true.*false.*true.*true.*false");
+
+ //Error when using cache name instead of group name
+ assertEquals(EXIT_CODE_OK, execute("--wal", "enable", "--groups",
"cache3"));
+ outputContains("Errors occurred:");
+ outputContains("Cache group not found: cache3");
+
+ assertEquals(EXIT_CODE_OK, execute("--wal", "enable", "--groups",
"group2,cache4"));
+ outputContains("Successfully enabled WAL for groups:");
+ outputContains("group2");
+ outputContains("cache4");
+
+ assertEquals(EXIT_CODE_OK, execute("--wal", "state", "--groups",
"group2,cache4"));
+ outputContains(".*group2.*true.*true.*true.*true.*false");
+ outputContains(".*cache4.*true.*true.*true.*true.*false");
+
+ assertEquals(EXIT_CODE_OK, execute("--wal", "disable"));
+ outputContains("Successfully disabled WAL for groups:");
+ outputContains("group1");
+ outputContains("group2");
+ outputContains("cache4");
- assertEquals(EXIT_CODE_OK, execute("--wal", "state", "--groups",
"testGroup"));
- outputContains(".*testGroup.*true.*true.*true.*true.*false");
+ assertEquals(EXIT_CODE_OK, execute("--wal", "state"));
+ outputContains(".*group1.*true.*false.*true.*true.*false");
+ outputContains(".*group2.*true.*false.*true.*true.*false");
+ outputContains(".*cache4.*true.*false.*true.*true.*false");
+
+ assertEquals(EXIT_CODE_OK, execute("--wal", "enable"));
+ outputContains("Successfully enabled WAL for groups:");
+ outputContains("group1");
+ outputContains("group2");
+ outputContains("cache4");
- assertEquals(EXIT_CODE_OK, execute("--wal", "disable", "--groups",
"testGroup"));
+ assertEquals(EXIT_CODE_OK, execute("--wal", "state"));
+ outputContains(".*group1.*true.*true.*true.*true.*false");
+ outputContains(".*group2.*true.*true.*true.*true.*false");
+ outputContains(".*cache4.*true.*true.*true.*true.*false");
+ }
+
+ /**
+ * Test WAL mode change attempts for non-persistent cache groups.
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testWalChangeForNonPersistentCaches() throws Exception {
Review Comment:
Let's fix this behavior in different patch
##########
modules/core/src/main/java/org/apache/ignite/internal/management/wal/WalSetStateTask.java:
##########
@@ -57,22 +78,50 @@ protected WalDisableJob(@Nullable WalDisableCommandArg arg,
boolean debug) {
}
/** {@inheritDoc} */
- @Override protected Void run(@Nullable WalDisableCommandArg arg)
throws IgniteException {
- Set<String> grps = F.isEmpty(arg.groups()) ? null : new
HashSet<>(Arrays.asList(arg.groups()));
+ @Override protected WalSetStateTaskResult run(@Nullable
WalDisableCommandArg arg) throws IgniteException {
+ Set<String> requestedGrps = F.isEmpty(arg.groups()) ? null : new
HashSet<>(Arrays.asList(arg.groups()));
Review Comment:
arr -> list -> set?
##########
modules/core/src/main/java/org/apache/ignite/internal/management/wal/WalSetStateTask.java:
##########
@@ -32,22 +33,42 @@
import org.jetbrains.annotations.Nullable;
/** */
-public class WalSetStateTask extends VisorMultiNodeTask<WalDisableCommandArg,
Void, Void> {
+public class WalSetStateTask extends VisorMultiNodeTask<WalDisableCommandArg,
WalSetStateTaskResult, WalSetStateTaskResult> {
/** */
private static final long serialVersionUID = 0;
/** {@inheritDoc} */
- @Override protected VisorJob<WalDisableCommandArg, Void>
job(WalDisableCommandArg arg) {
- return new WalDisableJob(arg, false);
+ @Override protected VisorJob<WalDisableCommandArg, WalSetStateTaskResult>
job(WalDisableCommandArg arg) {
+ return new WalDisableJob(arg, debug);
}
/** {@inheritDoc} */
- @Override protected @Nullable Void reduce0(List<ComputeJobResult> res)
throws IgniteException {
- return null;
+ @Override protected @Nullable WalSetStateTaskResult
reduce0(List<ComputeJobResult> res) throws IgniteException {
+ Set<String> successGrps = new HashSet<>();
+ List<String> errors = new ArrayList<>();
+
+ for (ComputeJobResult jobRes : res) {
+ if (jobRes.getException() != null) {
+ Throwable e = jobRes.getException();
+ errors.add("Node " + jobRes.getNode().consistentId() + ": Task
execution failed - " + e.getMessage());
+ }
+ else {
+ WalSetStateTaskResult result = jobRes.getData();
+ if (result.successGroups() != null)
+ successGrps.addAll(result.successGroups());
+ if (!Boolean.TRUE.equals(result.success()) &&
result.errorMessages() != null)
+ errors.addAll(result.errorMessages());
+ }
+ }
+
+ if (errors.isEmpty())
+ return new WalSetStateTaskResult(new ArrayList<>(successGrps));
+ else
+ return new WalSetStateTaskResult(new ArrayList<>(successGrps),
errors);
Review Comment:
make successGrps list in definition
##########
modules/core/src/main/java/org/apache/ignite/internal/management/wal/WalSetStateTaskResult.java:
##########
@@ -0,0 +1,106 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.management.wal;
+
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.ignite.internal.dto.IgniteDataTransferObject;
+import org.apache.ignite.internal.util.typedef.internal.U;
+
+/**
+ * Result of WAL enable/disable operation.
+ */
+public class WalSetStateTaskResult extends IgniteDataTransferObject {
+ /** */
+ private static final long serialVersionUID = 0L;
+
+ /** Success flag. */
+ private Boolean success;
Review Comment:
can check errors for empty.
##########
modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerWalTest.java:
##########
@@ -173,33 +173,117 @@ public void testWalStateInMemoryCdcCluster() throws
Exception {
}
/**
- * Test WAL mode change for a cache group contains multiple caches.
+ * Test for WAL enable/disable commands.
* @throws Exception If failed.
*/
@Test
- public void testWalChangeForMultiCacheGroup() throws Exception {
+ public void testWalManagementOperations() throws Exception {
Review Comment:
Keep old test name
##########
modules/core/src/main/java/org/apache/ignite/internal/management/wal/WalSetStateTask.java:
##########
@@ -57,22 +78,50 @@ protected WalDisableJob(@Nullable WalDisableCommandArg arg,
boolean debug) {
}
/** {@inheritDoc} */
- @Override protected Void run(@Nullable WalDisableCommandArg arg)
throws IgniteException {
- Set<String> grps = F.isEmpty(arg.groups()) ? null : new
HashSet<>(Arrays.asList(arg.groups()));
+ @Override protected WalSetStateTaskResult run(@Nullable
WalDisableCommandArg arg) throws IgniteException {
+ Set<String> requestedGrps = F.isEmpty(arg.groups()) ? null : new
HashSet<>(Arrays.asList(arg.groups()));
+ boolean isEnable = arg instanceof WalEnableCommandArg;
+ List<String> successGrps = new ArrayList<>();
+ List<String> errors = new ArrayList<>();
+
+ try {
+ Set<String> availableGrps = new HashSet<>();
- for (CacheGroupContext gctx :
ignite.context().cache().cacheGroups()) {
- String grpName = gctx.cacheOrGroupName();
+ for (CacheGroupContext gctx :
ignite.context().cache().cacheGroups()) {
+ String grpName = gctx.cacheOrGroupName();
+ availableGrps.add(grpName);
- if (grps != null && !grps.contains(grpName))
- continue;
+ if (requestedGrps != null &&
!requestedGrps.contains(grpName))
+ continue;
- if (arg instanceof WalEnableCommandArg)
- ignite.cluster().enableWal(grpName);
+ try {
+ if (isEnable)
+ ignite.cluster().enableWal(grpName);
+ else
+ ignite.cluster().disableWal(grpName);
+
+ successGrps.add(grpName);
+ }
+ catch (Exception e) {
+ errors.add("Failed to " + (isEnable ? "enable" :
"disable") +
+ " WAL for cache group: " + grpName + " - " +
e.getMessage());
+ }
+ }
+
+ for (String requestedGrp : requestedGrps) {
Review Comment:
Can reuse request groups after removing
##########
modules/core/src/main/java/org/apache/ignite/internal/management/wal/WalSetStateTask.java:
##########
@@ -32,22 +33,42 @@
import org.jetbrains.annotations.Nullable;
/** */
-public class WalSetStateTask extends VisorMultiNodeTask<WalDisableCommandArg,
Void, Void> {
+public class WalSetStateTask extends VisorMultiNodeTask<WalDisableCommandArg,
WalSetStateTaskResult, WalSetStateTaskResult> {
/** */
private static final long serialVersionUID = 0;
/** {@inheritDoc} */
- @Override protected VisorJob<WalDisableCommandArg, Void>
job(WalDisableCommandArg arg) {
- return new WalDisableJob(arg, false);
+ @Override protected VisorJob<WalDisableCommandArg, WalSetStateTaskResult>
job(WalDisableCommandArg arg) {
+ return new WalDisableJob(arg, debug);
}
/** {@inheritDoc} */
- @Override protected @Nullable Void reduce0(List<ComputeJobResult> res)
throws IgniteException {
- return null;
+ @Override protected @Nullable WalSetStateTaskResult
reduce0(List<ComputeJobResult> res) throws IgniteException {
+ Set<String> successGrps = new HashSet<>();
+ List<String> errors = new ArrayList<>();
+
+ for (ComputeJobResult jobRes : res) {
+ if (jobRes.getException() != null) {
+ Throwable e = jobRes.getException();
+ errors.add("Node " + jobRes.getNode().consistentId() + ": Task
execution failed - " + e.getMessage());
+ }
+ else {
+ WalSetStateTaskResult result = jobRes.getData();
+ if (result.successGroups() != null)
+ successGrps.addAll(result.successGroups());
Review Comment:
You can safely reuse success groups from any response
##########
modules/core/src/main/java/org/apache/ignite/internal/management/wal/WalSetStateTask.java:
##########
@@ -57,22 +78,50 @@ protected WalDisableJob(@Nullable WalDisableCommandArg arg,
boolean debug) {
}
/** {@inheritDoc} */
- @Override protected Void run(@Nullable WalDisableCommandArg arg)
throws IgniteException {
- Set<String> grps = F.isEmpty(arg.groups()) ? null : new
HashSet<>(Arrays.asList(arg.groups()));
+ @Override protected WalSetStateTaskResult run(@Nullable
WalDisableCommandArg arg) throws IgniteException {
+ Set<String> requestedGrps = F.isEmpty(arg.groups()) ? null : new
HashSet<>(Arrays.asList(arg.groups()));
+ boolean isEnable = arg instanceof WalEnableCommandArg;
+ List<String> successGrps = new ArrayList<>();
+ List<String> errors = new ArrayList<>();
+
+ try {
+ Set<String> availableGrps = new HashSet<>();
- for (CacheGroupContext gctx :
ignite.context().cache().cacheGroups()) {
- String grpName = gctx.cacheOrGroupName();
+ for (CacheGroupContext gctx :
ignite.context().cache().cacheGroups()) {
+ String grpName = gctx.cacheOrGroupName();
+ availableGrps.add(grpName);
- if (grps != null && !grps.contains(grpName))
- continue;
+ if (requestedGrps != null &&
!requestedGrps.contains(grpName))
+ continue;
- if (arg instanceof WalEnableCommandArg)
- ignite.cluster().enableWal(grpName);
+ try {
+ if (isEnable)
+ ignite.cluster().enableWal(grpName);
+ else
+ ignite.cluster().disableWal(grpName);
+
+ successGrps.add(grpName);
+ }
+ catch (Exception e) {
+ errors.add("Failed to " + (isEnable ? "enable" :
"disable") +
+ " WAL for cache group: " + grpName + " - " +
e.getMessage());
+ }
+ }
+
+ for (String requestedGrp : requestedGrps) {
+ if (!availableGrps.contains(requestedGrp))
+ errors.add("Cache group not found: " + requestedGrp);
+ }
+
+ if (errors.isEmpty())
+ return new WalSetStateTaskResult(successGrps);
else
- ignite.cluster().disableWal(grpName);
+ return new WalSetStateTaskResult(successGrps, errors);
+ }
+ catch (Exception e) {
+ errors.add("Failed to execute operation - " + e.getMessage());
+ return new WalSetStateTaskResult(successGrps, errors);
Review Comment:
add NL before return
##########
modules/core/src/main/java/org/apache/ignite/internal/management/wal/WalSetStateTask.java:
##########
@@ -32,22 +33,42 @@
import org.jetbrains.annotations.Nullable;
/** */
-public class WalSetStateTask extends VisorMultiNodeTask<WalDisableCommandArg,
Void, Void> {
+public class WalSetStateTask extends VisorMultiNodeTask<WalDisableCommandArg,
WalSetStateTaskResult, WalSetStateTaskResult> {
/** */
private static final long serialVersionUID = 0;
/** {@inheritDoc} */
- @Override protected VisorJob<WalDisableCommandArg, Void>
job(WalDisableCommandArg arg) {
- return new WalDisableJob(arg, false);
+ @Override protected VisorJob<WalDisableCommandArg, WalSetStateTaskResult>
job(WalDisableCommandArg arg) {
+ return new WalDisableJob(arg, debug);
}
/** {@inheritDoc} */
- @Override protected @Nullable Void reduce0(List<ComputeJobResult> res)
throws IgniteException {
- return null;
+ @Override protected @Nullable WalSetStateTaskResult
reduce0(List<ComputeJobResult> res) throws IgniteException {
+ Set<String> successGrps = new HashSet<>();
+ List<String> errors = new ArrayList<>();
+
+ for (ComputeJobResult jobRes : res) {
+ if (jobRes.getException() != null) {
+ Throwable e = jobRes.getException();
+ errors.add("Node " + jobRes.getNode().consistentId() + ": Task
execution failed - " + e.getMessage());
+ }
+ else {
+ WalSetStateTaskResult result = jobRes.getData();
+ if (result.successGroups() != null)
+ successGrps.addAll(result.successGroups());
+ if (!Boolean.TRUE.equals(result.success()) &&
result.errorMessages() != null)
+ errors.addAll(result.errorMessages());
Review Comment:
There are duplicates of errors
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
