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 2020/02/25 09:45:56 UTC
[GitHub] [ignite] Vladsz83 opened a new pull request #7471: IGNITE-12701 :
Disallow silent deactivation in CLI and REST.
Vladsz83 opened a new pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471
Major changes:
1) Added GridClientClusterStateRequestV2.
2) Deprecated GridClientClusterStateRequest.
3) Added flag ‘force’ for REST in GridRestClusterStateRequest
4) Added flag ‘force’ CLI in DeactivateCommand, ClusterStateChangeCommand.
5) Added IgniteFeatures# FORCED_CHANGE_OF_CLUSTER_STATE.
6) Added checking of deactivation safety: GridClusterStateProcessor# isDeactivationSafe()
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] nizhikov commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387509141
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/rest/client/message/GridClientClusterStateRequestV2.java
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.processors.rest.client.message;
+
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
+import org.apache.ignite.cluster.ClusterState;
+
+/**
+ * Enchanced version of {@link GridClientClusterStateRequest}.
+ * Introduced to support forced version of the change state command and keep backward compatibility
+ * with nodes of old version that may occur in cluster at the rolling updates.
+ */
+public class GridClientClusterStateRequestV2 extends GridClientClusterStateRequest {
+ /** */
+ private static final long serialVersionUID = 0L;
+
+ /** Forced change of cluster state. */
+ private boolean force;
Review comment:
We should rename this flast to `forceDeactivation` because it value apply to the deactivation command only.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] nizhikov commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r389037689
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/GridClusterStateProcessor.java
##########
@@ -1617,6 +1643,7 @@ public ExchangeActions autoAdjustExchangeActions(ExchangeActions exchActs) {
ctx.localNodeId(),
null,
ClusterState.active(clusterState.state()) ? clusterState.state() : ACTIVE,
+ true,
Review comment:
It's not clear why we force deactivation here.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] Vladsz83 commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387548787
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/ChangeGlobalStateMessage.java
##########
@@ -71,6 +71,9 @@
@GridToStringExclude
@Nullable private transient ServiceDeploymentActions serviceDeploymentActions;
+ /** Forced change of cluster state. */
+ private boolean force = true;
Review comment:
> Why the default value is true?
For backward compatibility. Most of the deactivation requests are still forced. Like through `Ignite, IgniteMXBean, IgniteCluster` etc. Also there is usage of this message from another places:
`GridClusterStateProcessor#autoAdjustExchangeActions(), GridClusterStateProcessor#autoAdjustInMemoryClusterState()`
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] nizhikov commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387509141
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/rest/client/message/GridClientClusterStateRequestV2.java
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.processors.rest.client.message;
+
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
+import org.apache.ignite.cluster.ClusterState;
+
+/**
+ * Enchanced version of {@link GridClientClusterStateRequest}.
+ * Introduced to support forced version of the change state command and keep backward compatibility
+ * with nodes of old version that may occur in cluster at the rolling updates.
+ */
+public class GridClientClusterStateRequestV2 extends GridClientClusterStateRequest {
+ /** */
+ private static final long serialVersionUID = 0L;
+
+ /** Forced change of cluster state. */
+ private boolean force;
Review comment:
We should rename this flag to `forceDeactivation` because it value apply to the deactivation command only.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] nizhikov commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387515325
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/GridClusterStateProcessor.java
##########
@@ -115,6 +118,11 @@
/** */
private static final String METASTORE_CURR_BLT_KEY = "metastoreBltKey";
+ /** Warning of unsafe deactivation. */
+ public static final String DATA_LOST_ON_DEACTIVATION_WARNING =
+ "Cluster has caches configured without persistence. " +
+ "During deactivation in-memory data and objects can be lost!";
Review comment:
We should change *can* to *will* because there is no case whe in-memory data will not be lost after deactivation.
So correct message to the user
`Deactivation stopped. Cluster has in-memory caches(without persistence). During deactivation, in-memory data will be lost.'
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] Vladsz83 commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387564676
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/rest/client/message/GridClientClusterStateRequestV2.java
##########
@@ -0,0 +1,83 @@
+/*
+ * 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.processors.rest.client.message;
+
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
+import org.apache.ignite.cluster.ClusterState;
+
+/**
+ * Enchanced version of {@link GridClientClusterStateRequest}.
+ * Introduced to support forced version of the change state command and keep backward compatibility
+ * with nodes of old version that may occur in cluster at the rolling updates.
+ */
+public class GridClientClusterStateRequestV2 extends GridClientClusterStateRequest {
+ /** */
+ private static final long serialVersionUID = 0L;
+
+ /** Forced change of cluster state. */
+ private boolean force;
Review comment:
Fixed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] Vladsz83 commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387548787
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/ChangeGlobalStateMessage.java
##########
@@ -71,6 +71,9 @@
@GridToStringExclude
@Nullable private transient ServiceDeploymentActions serviceDeploymentActions;
+ /** Forced change of cluster state. */
+ private boolean force = true;
Review comment:
> Why the default value is true?
For backward compatibility and due to general idea. Within this ticket we prevent unsafe deactivation only where required. Most of the deactivation requests are still forced. Like through `Ignite, IgniteMXBean, IgniteCluster` etc. Also there is usage of this message from another places:
`GridClusterStateProcessor#autoAdjustExchangeActions(), GridClusterStateProcessor#autoAdjustInMemoryClusterState()`
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] Vladsz83 commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387566298
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/GridClusterStateProcessor.java
##########
@@ -924,49 +945,8 @@ protected IgniteCheckedException concurrentStateChangeError(ClusterState state,
/** {@inheritDoc} */
@Override public IgniteInternalFuture<?> changeGlobalState(
- final boolean activate,
- Collection<? extends BaselineNode> baselineNodes,
- boolean forceChangeBaselineTopology
- ) {
- return changeGlobalState(activate, baselineNodes, forceChangeBaselineTopology, false);
- }
-
- /** {@inheritDoc} */
- @Override public IgniteInternalFuture<?> changeGlobalState(
- ClusterState state,
- Collection<? extends BaselineNode> baselineNodes,
- boolean forceChangeBaselineTopology
- ) {
- return changeGlobalState(state, baselineNodes, forceChangeBaselineTopology, false);
- }
-
- /**
- * @param activate New activate state.
- * @param baselineNodes New BLT nodes.
- * @param forceChangeBaselineTopology Force change BLT.
- * @param isAutoAdjust Auto adjusting flag.
- * @return Global change state future.
- * @deprecated Use {@link #changeGlobalState(ClusterState, Collection, boolean, boolean)} instead.
- */
- @Deprecated
- public IgniteInternalFuture<?> changeGlobalState(
- final boolean activate,
- Collection<? extends BaselineNode> baselineNodes,
- boolean forceChangeBaselineTopology,
- boolean isAutoAdjust
- ) {
- return changeGlobalState(activate ? ACTIVE : INACTIVE, baselineNodes, forceChangeBaselineTopology, isAutoAdjust);
- }
-
- /**
- * @param state New activate state.
- * @param baselineNodes New BLT nodes.
- * @param forceChangeBaselineTopology Force change BLT.
- * @param isAutoAdjust Auto adjusting flag.
- * @return Global change state future.
- */
- public IgniteInternalFuture<?> changeGlobalState(
ClusterState state,
+ boolean force,
Review comment:
Fixed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] Vladsz83 commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387566194
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/GridClusterStateProcessor.java
##########
@@ -1045,6 +1025,7 @@ else if (ClusterState.active(state)) {
/** */
private IgniteInternalFuture<?> changeGlobalState0(
ClusterState state,
+ boolean force,
Review comment:
Fixed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] Vladsz83 commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387566134
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/IGridClusterStateProcessor.java
##########
@@ -108,30 +108,20 @@ boolean onStateChangeMessage(AffinityTopologyVersion topVer,
/** */
void cacheProcessorStarted();
- /**
- * @param activate New cluster state.
- * @param baselineNodes New baseline nodes.
- * @param forceChangeBaselineTopology Force change baseline topology.
- * @return State change future.
- * @deprecated Use {@link #changeGlobalState(ClusterState, Collection, boolean)} instead.
- */
- @Deprecated
- IgniteInternalFuture<?> changeGlobalState(
- boolean activate,
- Collection<? extends BaselineNode> baselineNodes,
- boolean forceChangeBaselineTopology
- );
-
/**
* @param state New cluster state.
+ * @param force If {@code True} skips checking of the operation safety.
Review comment:
Fixed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] Vladsz83 commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387547117
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/ClusterStateChangeCommand.java
##########
@@ -91,6 +98,18 @@
catch (IllegalArgumentException e) {
throw new IllegalArgumentException("Can't parse new cluster state. State: " + s, e);
}
+
+ force = false;
Review comment:
> Do we need this line? Default `force` value is `false` already.
Yes, we need this. Commands are statefull unfortunately. Without this, next command usage will go with previous value 'force' param.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] nizhikov commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387506990
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/ChangeGlobalStateMessage.java
##########
@@ -71,6 +71,9 @@
@GridToStringExclude
@Nullable private transient ServiceDeploymentActions serviceDeploymentActions;
+ /** Forced change of cluster state. */
+ private boolean force = true;
Review comment:
Why the default value is true?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] sk0x50 commented on issue #7471: IGNITE-12701 : Disallow
silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on issue #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#issuecomment-599483109
Hello @nizhikov,
Ok, got it! Thank you for the clarification.
Thanks,
S.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] Vladsz83 commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387548787
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/ChangeGlobalStateMessage.java
##########
@@ -71,6 +71,9 @@
@GridToStringExclude
@Nullable private transient ServiceDeploymentActions serviceDeploymentActions;
+ /** Forced change of cluster state. */
+ private boolean force = true;
Review comment:
> Why the default value is true?
For backward compatibility. Most of the deactivation requests are still forcable. Through Ignite, IgniteMXBean, IgniteCluster etc.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] nizhikov commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387517031
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/rest/client/message/GridClientClusterStateRequest.java
##########
@@ -69,6 +70,21 @@ public static GridClientClusterStateRequest state(ClusterState state) {
return msg;
}
+ /** Default constructor for the exernalization. */
Review comment:
typo: ex*t*ernalization
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] nizhikov commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387509903
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/GridClusterStateProcessor.java
##########
@@ -924,49 +945,8 @@ protected IgniteCheckedException concurrentStateChangeError(ClusterState state,
/** {@inheritDoc} */
@Override public IgniteInternalFuture<?> changeGlobalState(
- final boolean activate,
- Collection<? extends BaselineNode> baselineNodes,
- boolean forceChangeBaselineTopology
- ) {
- return changeGlobalState(activate, baselineNodes, forceChangeBaselineTopology, false);
- }
-
- /** {@inheritDoc} */
- @Override public IgniteInternalFuture<?> changeGlobalState(
- ClusterState state,
- Collection<? extends BaselineNode> baselineNodes,
- boolean forceChangeBaselineTopology
- ) {
- return changeGlobalState(state, baselineNodes, forceChangeBaselineTopology, false);
- }
-
- /**
- * @param activate New activate state.
- * @param baselineNodes New BLT nodes.
- * @param forceChangeBaselineTopology Force change BLT.
- * @param isAutoAdjust Auto adjusting flag.
- * @return Global change state future.
- * @deprecated Use {@link #changeGlobalState(ClusterState, Collection, boolean, boolean)} instead.
- */
- @Deprecated
- public IgniteInternalFuture<?> changeGlobalState(
- final boolean activate,
- Collection<? extends BaselineNode> baselineNodes,
- boolean forceChangeBaselineTopology,
- boolean isAutoAdjust
- ) {
- return changeGlobalState(activate ? ACTIVE : INACTIVE, baselineNodes, forceChangeBaselineTopology, isAutoAdjust);
- }
-
- /**
- * @param state New activate state.
- * @param baselineNodes New BLT nodes.
- * @param forceChangeBaselineTopology Force change BLT.
- * @param isAutoAdjust Auto adjusting flag.
- * @return Global change state future.
- */
- public IgniteInternalFuture<?> changeGlobalState(
ClusterState state,
+ boolean force,
Review comment:
We should rename this flag to `forceDeactivation` because its value applies to the deactivation command only.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] nizhikov commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387499377
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/DeactivateCommand.java
##########
@@ -78,6 +81,21 @@
return null;
}
+ /** {@inheritDoc} */
+ @Override public void parseArguments(CommandArgIterator argIter) {
+ force = false;
Review comment:
Do we need this line? Default `force` value is `false` already.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] nizhikov commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387509547
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/IGridClusterStateProcessor.java
##########
@@ -108,30 +108,20 @@ boolean onStateChangeMessage(AffinityTopologyVersion topVer,
/** */
void cacheProcessorStarted();
- /**
- * @param activate New cluster state.
- * @param baselineNodes New baseline nodes.
- * @param forceChangeBaselineTopology Force change baseline topology.
- * @return State change future.
- * @deprecated Use {@link #changeGlobalState(ClusterState, Collection, boolean)} instead.
- */
- @Deprecated
- IgniteInternalFuture<?> changeGlobalState(
- boolean activate,
- Collection<? extends BaselineNode> baselineNodes,
- boolean forceChangeBaselineTopology
- );
-
/**
* @param state New cluster state.
+ * @param force If {@code True} skips checking of the operation safety.
Review comment:
We should rename this flag to `forceDeactivation` because it value applies to the deactivation command only.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] Vladsz83 commented on issue #7471: IGNITE-12701 : Disallow
silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on issue #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#issuecomment-599203579
> Moreover, it contains the following tickets: IGNITE-12597, IGNITE-12646, and IGNITE-12703 that not related to the issue at all. It looks like it was done by accident.
Hi, @sk0x50
This merge doesn't contain mentioned issues IGNITE-12597, IGNITE-12646, and IGNITE-12703 at all. That was just wrong comments in my branch.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] Vladsz83 commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387563217
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/GridClusterStateProcessor.java
##########
@@ -924,49 +945,8 @@ protected IgniteCheckedException concurrentStateChangeError(ClusterState state,
/** {@inheritDoc} */
@Override public IgniteInternalFuture<?> changeGlobalState(
- final boolean activate,
Review comment:
> Why this deletion?
To simplify and reduce internal API and code base. There were already 2 changeGlobalState(). First, I introduced new one with ‘force’ not to change previous API. There 3 (2 of them deprecated) became a mesh. After all, as it is internal API, I suggest to keep one change-state method. Isn’t it better?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] sk0x50 commented on issue #7471: IGNITE-12701 : Disallow
silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on issue #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#issuecomment-599209071
Hello @Vladsz83 ,
> This merge doesn't contain mentioned issues IGNITE-12597, IGNITE-12646, and IGNITE-12703 at all.
> That was just wrong comments in my branch.
Yes, I know that, and you are right.
Perhaps, my message was not quite correct. I just wanted to say that it contains incorrect commit messages that can lead to a misunderstanding. That's all.
Thanks,
Slava.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] nizhikov commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387515325
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/GridClusterStateProcessor.java
##########
@@ -115,6 +118,11 @@
/** */
private static final String METASTORE_CURR_BLT_KEY = "metastoreBltKey";
+ /** Warning of unsafe deactivation. */
+ public static final String DATA_LOST_ON_DEACTIVATION_WARNING =
+ "Cluster has caches configured without persistence. " +
+ "During deactivation in-memory data and objects can be lost!";
Review comment:
We should change "*can*" to "*will*" because there is no case when in-memory data will not be lost after deactivation.
So correct message to the user
`Deactivation stopped. Cluster has in-memory caches(without persistence). During deactivation, in-memory data will be lost.'
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] nizhikov merged pull request #7471: IGNITE-12701 :
Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
nizhikov merged pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] Vladsz83 commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387563217
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/GridClusterStateProcessor.java
##########
@@ -924,49 +945,8 @@ protected IgniteCheckedException concurrentStateChangeError(ClusterState state,
/** {@inheritDoc} */
@Override public IgniteInternalFuture<?> changeGlobalState(
- final boolean activate,
Review comment:
> Why this deletion?
To simplify and reduce internal API and code base. There were already 2 changeGlobalState(). First, I introduced new one with ‘force’ not to change previous API. There 3 methods (2 of them deprecated) became a mesh. After all, as it is internal API, I suggest to keep one change-state method. Isn’t it better?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] nizhikov commented on issue #7471: IGNITE-12701 : Disallow
silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
nizhikov commented on issue #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#issuecomment-599370048
Hello @sk0x50
First of all, this is my fault, but the reason for the fault is GitHub bug.
I wrote a good commit message before the merge.
But after pushing the "merge" button I've got "We can't merge your PR" error message and "Try again" button. After pushing "Try again" we got what we got - merge with the default commit message.
Sorry, for that.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] Vladsz83 commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387568450
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/rest/client/message/GridClientClusterStateRequest.java
##########
@@ -69,6 +70,21 @@ public static GridClientClusterStateRequest state(ClusterState state) {
return msg;
}
+ /** Default constructor for the exernalization. */
Review comment:
Fixed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] Vladsz83 commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387563805
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/IGridClusterStateProcessor.java
##########
@@ -108,30 +108,20 @@ boolean onStateChangeMessage(AffinityTopologyVersion topVer,
/** */
void cacheProcessorStarted();
- /**
- * @param activate New cluster state.
- * @param baselineNodes New baseline nodes.
- * @param forceChangeBaselineTopology Force change baseline topology.
- * @return State change future.
- * @deprecated Use {@link #changeGlobalState(ClusterState, Collection, boolean)} instead.
- */
- @Deprecated
- IgniteInternalFuture<?> changeGlobalState(
Review comment:
> Why this deletion?
Same as above ("To simplify and reduce internal API and code base...")
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] sk0x50 edited a comment on issue #7471: IGNITE-12701 :
Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
sk0x50 edited a comment on issue #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#issuecomment-599209071
Hello @Vladsz83 ,
> This merge doesn't contain mentioned issues IGNITE-12597, IGNITE-12646, and IGNITE-12703 at all. That was just wrong comments in my branch.
Yes, I know that, and you are right.
Perhaps, my message was not quite correct. I just wanted to say that it contains incorrect commit messages that can lead to a misunderstanding. That's all.
Thanks,
Slava.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] nizhikov commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387507524
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/GridClusterStateProcessor.java
##########
@@ -924,49 +945,8 @@ protected IgniteCheckedException concurrentStateChangeError(ClusterState state,
/** {@inheritDoc} */
@Override public IgniteInternalFuture<?> changeGlobalState(
- final boolean activate,
Review comment:
Why this deletion?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] Vladsz83 commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r389878598
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/GridClusterStateProcessor.java
##########
@@ -1617,6 +1643,7 @@ public ExchangeActions autoAdjustExchangeActions(ExchangeActions exchActs) {
ctx.localNodeId(),
null,
ClusterState.active(clusterState.state()) ? clusterState.state() : ACTIVE,
+ true,
Review comment:
> It's not clear why we force deactivation here.
All previous internal changes of cluster state were forced. Thus, all current internal changes of cluster are considered forced. Only the exceptions are the user commands being translated to not-forced internal calls without flag ‘force’ passed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] nizhikov commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387552701
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/ChangeGlobalStateMessage.java
##########
@@ -71,6 +71,9 @@
@GridToStringExclude
@Nullable private transient ServiceDeploymentActions serviceDeploymentActions;
+ /** Forced change of cluster state. */
+ private boolean force = true;
Review comment:
I think we should make `force == false` by default and explicitly pass `true` when required.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] nizhikov commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387509764
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/GridClusterStateProcessor.java
##########
@@ -1045,6 +1025,7 @@ else if (ClusterState.active(state)) {
/** */
private IgniteInternalFuture<?> changeGlobalState0(
ClusterState state,
+ boolean force,
Review comment:
We should rename this flag to `forceDeactivation` because its value applies to the deactivation command only.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] sk0x50 commented on issue #7471: IGNITE-12701 : Disallow
silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on issue #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#issuecomment-599202413
Hello @nizhikov ,
Please don't get me wrong, but...
What is the reason to include all intermediate commit messages into the final one? https://github.com/apache/ignite/commit/4921fcf1fecbd8a1ab02099e09cc2adb0b3ff88a
Moreover, it contains the following tickets: IGNITE-12597, IGNITE-12646, and IGNITE-12703 that not related to the issue at all. It looks like it was done by accident.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] Vladsz83 commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387568120
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/GridClusterStateProcessor.java
##########
@@ -115,6 +118,11 @@
/** */
private static final String METASTORE_CURR_BLT_KEY = "metastoreBltKey";
+ /** Warning of unsafe deactivation. */
+ public static final String DATA_LOST_ON_DEACTIVATION_WARNING =
+ "Cluster has caches configured without persistence. " +
+ "During deactivation in-memory data and objects can be lost!";
Review comment:
Fixed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] nizhikov commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387499162
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/ClusterStateChangeCommand.java
##########
@@ -91,6 +98,18 @@
catch (IllegalArgumentException e) {
throw new IllegalArgumentException("Can't parse new cluster state. State: " + s, e);
}
+
+ force = false;
Review comment:
Do we need this line? Default `force` value is `false` already.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] nizhikov commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387507811
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/IGridClusterStateProcessor.java
##########
@@ -108,30 +108,20 @@ boolean onStateChangeMessage(AffinityTopologyVersion topVer,
/** */
void cacheProcessorStarted();
- /**
- * @param activate New cluster state.
- * @param baselineNodes New baseline nodes.
- * @param forceChangeBaselineTopology Force change baseline topology.
- * @return State change future.
- * @deprecated Use {@link #changeGlobalState(ClusterState, Collection, boolean)} instead.
- */
- @Deprecated
- IgniteInternalFuture<?> changeGlobalState(
Review comment:
Why this deletion?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] Vladsz83 commented on a change in pull request #7471:
IGNITE-12701 : Disallow silent deactivation in CLI and REST.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7471: IGNITE-12701 : Disallow silent deactivation in CLI and REST.
URL: https://github.com/apache/ignite/pull/7471#discussion_r387547266
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/DeactivateCommand.java
##########
@@ -78,6 +81,21 @@
return null;
}
+ /** {@inheritDoc} */
+ @Override public void parseArguments(CommandArgIterator argIter) {
+ force = false;
Review comment:
> Do we need this line? Default `force` value is `false` already.
Yes, we need this. Commands are statefull unfortunately. Without this, next command usage will go with previous value 'force' param.
----------------------------------------------------------------
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
With regards,
Apache Git Services