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/03 16:28:18 UTC
[GitHub] [ignite] Vladsz83 opened a new pull request #7358: IGNITE-12614 :
Disallow silent deactivation of cluster to prevent in-mem data loss.
Vladsz83 opened a new pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358
First implementation for conceptual review.
----------------------------------------------------------------
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] NSAmelchev commented on a change in pull request #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375309867
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java
##########
@@ -3946,7 +3952,27 @@ private void checkNearCacheStarted(IgniteCacheProxy<?, ?> cache) throws IgniteCh
/** {@inheritDoc} */
@Override public void active(boolean active) {
- cluster().active(active);
+ if (active)
+ activate();
+ else
+ deactivate(false);
+ }
+
+ /** {@inheritDoc} */
+ @Override public void activate() {
+ cluster().state(ClusterState.ACTIVE);
+ }
+
+ /** {@inheritDoc} */
+ @Override public void deactivate(boolean force) {
+ //Check if cluster ir ready for deactivation.
+ if (cluster().state() == ClusterState.ACTIVE && !force) {
Review comment:
Cluster state can be `ACTIVE_READ_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 a change in pull request #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375360780
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java
##########
@@ -3946,7 +3952,27 @@ private void checkNearCacheStarted(IgniteCacheProxy<?, ?> cache) throws IgniteCh
/** {@inheritDoc} */
@Override public void active(boolean active) {
- cluster().active(active);
+ if (active)
+ activate();
+ else
+ deactivate(false);
+ }
+
+ /** {@inheritDoc} */
+ @Override public void activate() {
+ cluster().state(ClusterState.ACTIVE);
+ }
+
+ /** {@inheritDoc} */
+ @Override public void deactivate(boolean force) {
+ //Check if cluster ir ready for deactivation.
+ if (cluster().state() == ClusterState.ACTIVE && !force) {
+ String msg = ClusterStateChangeCommand.isClusterReadyForDeactivation((cls -> compute().execute(cls, null)));
Review comment:
Implemented.
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r377070501
##########
File path: modules/core/src/main/java/org/apache/ignite/mxbean/IgniteMXBean.java
##########
@@ -388,11 +388,31 @@
)
public boolean pingNode(String nodeId);
+ /** Activates cluster. */
+ @MXBeanDescription(
Review comment:
@nizhikov , how do you think, can we keep IgniteMXBean as it is now in this PR? With one new
`clusterState(String state, boolean force)`
Unfortunately, we can't use two different default-methods in the interfaces IgniteMXBean and Ignite:
`
default public void active(boolean active){
active(active, false);
}
and
default public void active(boolean active){
active(active, true);
}
`
Jaca compilator forces us to choose one in IgniteKernal:
`class org.apache.ignite.internal.IgniteKernal inherits unrelated defaults for active(boolean) from types org.apache.ignite.Ignite and org.apache.ignite.mxbean.IgniteMXBean`
I think this happens because there are no real implementations of default methods within interfaces. They are just code sample for in-lining to the implementation. So, we have two different implementations.
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375358935
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java
##########
@@ -1959,6 +1962,9 @@ private void fillNodeAttributes(boolean notifyEnabled) throws IgniteCheckedExcep
ctx.addNodeAttribute(ATTR_EVENT_DRIVEN_SERVICE_PROCESSOR_ENABLED,
ctx.service() instanceof IgniteServiceProcessor);
+
+ //Allows to predict behavior on deactivation.
+ add(ATTR_MEMORY_ERASURE_ON_DEACTIVATION, !getBoolean(IGNITE_REUSE_MEMORY_ON_DEACTIVATE));
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375730624
##########
File path: modules/core/src/main/java/org/apache/ignite/mxbean/IgniteMXBean.java
##########
@@ -380,23 +376,30 @@
*/
@MXBeanDescription("Pings node with given node ID to see whether it is alive. " +
"Returns whether or not node is alive.")
- @MXBeanParametersNames(
- "nodeId"
- )
- @MXBeanParametersDescriptions(
- "String presentation of node ID. See java.util.UUID class for details."
- )
+ @MXBeanParametersNames("nodeId")
+ @MXBeanParametersDescriptions("String presentation of node ID. See java.util.UUID class for details.")
public boolean pingNode(String nodeId);
+ /** Activates cluster. */
+ @MXBeanDescription("Execute activation process.")
+ @MXBeanParametersNames("active")
+ public void activate();
+
+ /** Deactivates cluster. */
+ @MXBeanDescription("Execute deactivation process.")
+ @MXBeanParametersNames("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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r376379912
##########
File path: modules/core/src/main/java/org/apache/ignite/mxbean/IgniteMXBean.java
##########
@@ -380,23 +376,30 @@
*/
@MXBeanDescription("Pings node with given node ID to see whether it is alive. " +
"Returns whether or not node is alive.")
- @MXBeanParametersNames(
- "nodeId"
- )
- @MXBeanParametersDescriptions(
- "String presentation of node ID. See java.util.UUID class for details."
- )
+ @MXBeanParametersNames("nodeId")
+ @MXBeanParametersDescriptions("String presentation of node ID. See java.util.UUID class for details.")
public boolean pingNode(String nodeId);
+ /** Activates cluster. */
+ @MXBeanDescription("Execute activation process.")
+ @MXBeanParametersNames("active")
+ public void activate();
+
+ /** Deactivates cluster. */
+ @MXBeanDescription("Execute deactivation process.")
+ @MXBeanParametersNames("force")
Review comment:
@nizhikov , please review IgniteMXBean. We have to put flag 'force' somewhere for the deactivation. Current choice is
`IgniteMXBean#clusterState(String state) -> clusterState(String state, boolean force)`
Usually one has to support backward compatibility of an Interface. But this is an JMX. I don't think it is unsafe to add single param. Otherwise we need to implement additional method like
` clusterState(String state, boolean force)
or
deactivate(boolean force)`
whereas the mxbean already has several methods for the cluster state:
`void active(boolean active)
`boolean active()`
`void clusterState(String state)`
`String clusterState()`
Don't think one more would be handy;
Also, should we declare
`IgniteMXBean#active(boolean active)`
deprecated too.
----------------------------------------------------------------
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] NSAmelchev commented on a change in pull request #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375292287
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/ClusterStateChangeCommand.java
##########
@@ -42,6 +63,26 @@
/** Cluster name. */
private String clusterName;
+ /** Force cluster deactivation even it might have in-mem caches. */
+ private boolean force;
+
+ /**
+ * Checks if it is resonable to deactivate cluster.
+ *
+ * @param taskLauncher Computation task launcher. The task has no params, just needs to be launched for its result.
+ * @return Empty (not-null) message if cluster is ready. Or warning text telling why deactivation is not advised.
+ */
+ @NotNull public static String isClusterReadyForDeactivation(
Review comment:
Incorrect package. Use `org.jetbrains.annotations.NotNull` instead of
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r376385251
##########
File path: modules/core/src/main/java/org/apache/ignite/mxbean/IgniteMXBean.java
##########
@@ -380,23 +376,30 @@
*/
@MXBeanDescription("Pings node with given node ID to see whether it is alive. " +
"Returns whether or not node is alive.")
- @MXBeanParametersNames(
- "nodeId"
- )
- @MXBeanParametersDescriptions(
- "String presentation of node ID. See java.util.UUID class for details."
- )
+ @MXBeanParametersNames("nodeId")
+ @MXBeanParametersDescriptions("String presentation of node ID. See java.util.UUID class for details.")
public boolean pingNode(String nodeId);
+ /** Activates cluster. */
+ @MXBeanDescription("Execute activation process.")
+ @MXBeanParametersNames("active")
+ public void activate();
+
+ /** Deactivates cluster. */
+ @MXBeanDescription("Execute deactivation process.")
+ @MXBeanParametersNames("force")
Review comment:
> > But this is an JMX. I don't think it is unsafe to add single param.
>
> Why do you think so?
An automated, external system like Zabbix of something must use exposed bean to be afraid of it. Manual method invocation in the console is quite obvious. Just requires to say extra ‘yes’ or ‘no’ for the new argument. Direct bean usage within the code isn't reasonable except the tests.
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r377070501
##########
File path: modules/core/src/main/java/org/apache/ignite/mxbean/IgniteMXBean.java
##########
@@ -388,11 +388,31 @@
)
public boolean pingNode(String nodeId);
+ /** Activates cluster. */
+ @MXBeanDescription(
Review comment:
@nizhikov , how do you think, can we keep IgniteMXBean as it is now in this PR? With one new
`clusterState(String state, boolean force)`
Unfortunately, we can't use two different default-methods in the interfaces IgniteMXBean and Ignite:
` default public void active(boolean active){
active(active, false);
}
and
default public void active(boolean active){
active(active, true);
}`
Jaca compilator forces us to choose one in IgniteKernal:
`class org.apache.ignite.internal.IgniteKernal inherits unrelated defaults for active(boolean) from types org.apache.ignite.Ignite and org.apache.ignite.mxbean.IgniteMXBean`
I think this happens because there are no real implementations of default methods within interfaces. They are just code sample for in-lining to implementation. So, we have two different implementations.
----------------------------------------------------------------
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] NSAmelchev commented on a change in pull request #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375450576
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java
##########
@@ -3949,6 +3957,28 @@ private void checkNearCacheStarted(IgniteCacheProxy<?, ?> cache) throws IgniteCh
cluster().active(active);
}
+ /** {@inheritDoc} */
+ @Override public void activate() {
+ cluster().state(ClusterState.ACTIVE);
+ }
+
+ /** {@inheritDoc} */
+ @Override public void deactivate(boolean force) {
+ // Check if cluster ir ready for deactivation.
+ if ((cluster().state() == ClusterState.ACTIVE || cluster().state() == ClusterState.ACTIVE_READ_ONLY)
Review comment:
I suggest:
1. Change condition to `cluster().state() != ClusterState.INACTIVE && !force`
2. Do not execute task. Implement some method like this `ctx.state().isCacheDataCanBeLostOnDeactivation()` which will check attributes and in-memry caches
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375358728
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/ClusterStateChangeCommand.java
##########
@@ -42,6 +63,26 @@
/** Cluster name. */
private String clusterName;
+ /** Force cluster deactivation even it might have in-mem caches. */
+ private boolean force;
+
+ /**
+ * Checks if it is resonable to deactivate cluster.
+ *
+ * @param taskLauncher Computation task launcher. The task has no params, just needs to be launched for its result.
+ * @return Empty (not-null) message if cluster is ready. Or warning text telling why deactivation is not advised.
+ */
+ @NotNull public static String isClusterReadyForDeactivation(
Review comment:
Not actual. Code changed.
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375365356
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/ClusterStateChangeCommand.java
##########
@@ -102,4 +167,53 @@
@Override public String name() {
return SET_STATE.toCommandName();
}
+
+ /** Searches for any non-persistent cache. */
+ private static class FindNotPersistentCachesJob extends ComputeJobAdapter {
+
Review comment:
Not actual. Code changed.
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375368400
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/ClusterStateChangeCommand.java
##########
@@ -102,4 +167,53 @@
@Override public String name() {
return SET_STATE.toCommandName();
}
+
+ /** Searches for any non-persistent cache. */
+ private static class FindNotPersistentCachesJob extends ComputeJobAdapter {
+
+ @IgniteInstanceResource
+ private Ignite ignite;
+
+ /** */
+ @SuppressWarnings("unchecked")
+ @Override public Boolean execute() throws IgniteException {
+ //Find any node with disabled memory reusage on deactivation/activation.
+ boolean cacheDataCanBeLost = ignite.cluster().forPredicate(node->
+ (Boolean)node.attributes().getOrDefault(ATTR_MEMORY_ERASURE_ON_DEACTIVATION, true))
+ .nodes().stream().findAny().isPresent();
+
+ if (cacheDataCanBeLost) {
+ //Find data region to set persistent flag.
+ for (String cacheName : ignite.cacheNames()) {
Review comment:
Implemented.
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375373084
##########
File path: modules/core/src/main/java/org/apache/ignite/mxbean/IgniteMXBean.java
##########
@@ -388,11 +388,31 @@
)
public boolean pingNode(String nodeId);
+ /** Activates cluster. */
+ @MXBeanDescription(
+ "Execute activation process."
+ )
+ @MXBeanParametersNames(
+ "active"
+ )
+ public void activate();
+
+ /** Deactivates cluster. */
+ @MXBeanDescription(
+ "Execute deactivation process."
+ )
+ @MXBeanParametersNames(
+ "force"
+ )
+ public void deactivate(boolean force);
+
/**
* @param active Activate/DeActivate flag.
+ * @deprecated Use {@link #activate()} and {@link #deactivate(boolean)} instead.
*/
+ @Deprecated
@MXBeanDescription(
- "Execute activate or deactivate process."
+ "Execute activate or deactivate process. Deprecated. Use activate() / deactivate(). Deactivation may require flag '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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r376379912
##########
File path: modules/core/src/main/java/org/apache/ignite/mxbean/IgniteMXBean.java
##########
@@ -380,23 +376,30 @@
*/
@MXBeanDescription("Pings node with given node ID to see whether it is alive. " +
"Returns whether or not node is alive.")
- @MXBeanParametersNames(
- "nodeId"
- )
- @MXBeanParametersDescriptions(
- "String presentation of node ID. See java.util.UUID class for details."
- )
+ @MXBeanParametersNames("nodeId")
+ @MXBeanParametersDescriptions("String presentation of node ID. See java.util.UUID class for details.")
public boolean pingNode(String nodeId);
+ /** Activates cluster. */
+ @MXBeanDescription("Execute activation process.")
+ @MXBeanParametersNames("active")
+ public void activate();
+
+ /** Deactivates cluster. */
+ @MXBeanDescription("Execute deactivation process.")
+ @MXBeanParametersNames("force")
Review comment:
@nizhikov , please review IgniteMXBean. We have to put flag 'force' somewhere for the deactivation. Current choice is
`IgniteMXBean#clusterState(String state) -> clusterState(String state, boolean force)`
Usually one has to support backward compatibility of an Interface. But this is an JMX. I don't think it is unsafe to add single param. Otherwise we need to implement additional method like
` clusterState(String state, boolean force)
or
deactivate(boolean force)`
whereas the mxbean already has several methods for the cluster state:
`void active(boolean active)`
`boolean active()`
`void clusterState(String state)`
`String clusterState()`
Don't think one more would be handy;
Also, should we declare
`IgniteMXBean#active(boolean active)`
deprecated too.
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375359023
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java
##########
@@ -3946,7 +3952,27 @@ private void checkNearCacheStarted(IgniteCacheProxy<?, ?> cache) throws IgniteCh
/** {@inheritDoc} */
@Override public void active(boolean active) {
- cluster().active(active);
+ if (active)
+ activate();
+ else
+ deactivate(false);
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] NSAmelchev commented on a change in pull request #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375310891
##########
File path: modules/core/src/main/java/org/apache/ignite/mxbean/IgniteMXBean.java
##########
@@ -388,11 +388,31 @@
)
public boolean pingNode(String nodeId);
+ /** Activates cluster. */
+ @MXBeanDescription(
Review comment:
Seems we can make one-line them.
----------------------------------------------------------------
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] NSAmelchev commented on a change in pull request #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375452692
##########
File path: modules/core/src/main/java/org/apache/ignite/mxbean/IgniteMXBean.java
##########
@@ -380,23 +376,30 @@
*/
@MXBeanDescription("Pings node with given node ID to see whether it is alive. " +
"Returns whether or not node is alive.")
- @MXBeanParametersNames(
- "nodeId"
- )
- @MXBeanParametersDescriptions(
- "String presentation of node ID. See java.util.UUID class for details."
- )
+ @MXBeanParametersNames("nodeId")
+ @MXBeanParametersDescriptions("String presentation of node ID. See java.util.UUID class for details.")
public boolean pingNode(String nodeId);
+ /** Activates cluster. */
+ @MXBeanDescription("Execute activation process.")
+ @MXBeanParametersNames("active")
+ public void activate();
+
+ /** Deactivates cluster. */
+ @MXBeanDescription("Execute deactivation process.")
+ @MXBeanParametersNames("force")
Review comment:
We should add MXBeanParametersDescriptions
----------------------------------------------------------------
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] NSAmelchev commented on a change in pull request #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375286713
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java
##########
@@ -3946,7 +3952,27 @@ private void checkNearCacheStarted(IgniteCacheProxy<?, ?> cache) throws IgniteCh
/** {@inheritDoc} */
@Override public void active(boolean active) {
- cluster().active(active);
+ if (active)
+ activate();
+ else
+ deactivate(false);
Review comment:
Why we should change behavior for the deprecated method?
----------------------------------------------------------------
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] NSAmelchev commented on a change in pull request #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375450576
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java
##########
@@ -3949,6 +3957,28 @@ private void checkNearCacheStarted(IgniteCacheProxy<?, ?> cache) throws IgniteCh
cluster().active(active);
}
+ /** {@inheritDoc} */
+ @Override public void activate() {
+ cluster().state(ClusterState.ACTIVE);
+ }
+
+ /** {@inheritDoc} */
+ @Override public void deactivate(boolean force) {
+ // Check if cluster ir ready for deactivation.
+ if ((cluster().state() == ClusterState.ACTIVE || cluster().state() == ClusterState.ACTIVE_READ_ONLY)
Review comment:
I suggest:
1. Change condition to `cluster().state() != ClusterState.INACTIVE && !force`
2. Do not execute task. Implement some method like this `ctx.state().isUserDataCanBeLostOnDeactivation()` which will check attributes and in-memry caches
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375738551
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/visor/cluster/VisorCheckDeactivationTask.java
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.visor.cluster;
+
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.processors.cache.CacheType;
+import org.apache.ignite.internal.processors.task.GridInternal;
+import org.apache.ignite.internal.visor.VisorJob;
+import org.apache.ignite.internal.visor.VisorOneNodeTask;
+
+import static org.apache.ignite.internal.IgniteNodeAttributes.ATTR_KEEP_MEMORY_ON_DEACTIVATION;
+
+/**
+ * Task for checking if deactiation is safe. Returns false when deactivation can lead to data loss.
+ */
+@GridInternal
+public class VisorCheckDeactivationTask extends VisorOneNodeTask<Void, Boolean> {
+ /** Serial version uid. */
+ private static final long serialVersionUID = 0L;
+
+ /** */
+ public static final String WARN_DEACTIVATION_IN_MEM_CACHES =
Review comment:
Done.
----------------------------------------------------------------
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] NSAmelchev commented on a change in pull request #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375291183
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java
##########
@@ -1959,6 +1962,9 @@ private void fillNodeAttributes(boolean notifyEnabled) throws IgniteCheckedExcep
ctx.addNodeAttribute(ATTR_EVENT_DRIVEN_SERVICE_PROCESSOR_ENABLED,
ctx.service() instanceof IgniteServiceProcessor);
+
+ //Allows to predict behavior on deactivation.
+ add(ATTR_MEMORY_ERASURE_ON_DEACTIVATION, !getBoolean(IGNITE_REUSE_MEMORY_ON_DEACTIVATE));
Review comment:
Can we change the var name and do not inverse var?
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r376392073
##########
File path: modules/core/src/main/java/org/apache/ignite/mxbean/IgniteMXBean.java
##########
@@ -380,23 +376,30 @@
*/
@MXBeanDescription("Pings node with given node ID to see whether it is alive. " +
"Returns whether or not node is alive.")
- @MXBeanParametersNames(
- "nodeId"
- )
- @MXBeanParametersDescriptions(
- "String presentation of node ID. See java.util.UUID class for details."
- )
+ @MXBeanParametersNames("nodeId")
+ @MXBeanParametersDescriptions("String presentation of node ID. See java.util.UUID class for details.")
public boolean pingNode(String nodeId);
+ /** Activates cluster. */
+ @MXBeanDescription("Execute activation process.")
+ @MXBeanParametersNames("active")
+ public void activate();
+
+ /** Deactivates cluster. */
+ @MXBeanDescription("Execute deactivation process.")
+ @MXBeanParametersNames("force")
Review comment:
> JMX bean is a public API, so we should preserve backward compatibility.
Ok.
And
>> Also, should we declare
>> `IgniteMXBean#active(boolean active)`
>> deprecated too?
----------------------------------------------------------------
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] NSAmelchev commented on a change in pull request #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375436085
##########
File path: modules/core/src/main/java/org/apache/ignite/mxbean/IgniteMXBean.java
##########
@@ -424,12 +427,8 @@
* @throws JMException Thrown if undeploy failed.
*/
@MXBeanDescription("Makes the best attempt to undeploy a task from the whole grid.")
- @MXBeanParametersNames(
- "taskName"
- )
- @MXBeanParametersDescriptions(
- "Name of the task to undeploy."
- )
+ @MXBeanParametersNames("taskName")
Review comment:
Lets do not change code that is not related to the issue
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375710485
##########
File path: modules/core/src/main/java/org/apache/ignite/mxbean/IgniteMXBean.java
##########
@@ -424,12 +427,8 @@
* @throws JMException Thrown if undeploy failed.
*/
@MXBeanDescription("Makes the best attempt to undeploy a task from the whole grid.")
- @MXBeanParametersNames(
- "taskName"
- )
- @MXBeanParametersDescriptions(
- "Name of the task to undeploy."
- )
+ @MXBeanParametersNames("taskName")
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375359487
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java
##########
@@ -3946,7 +3952,27 @@ private void checkNearCacheStarted(IgniteCacheProxy<?, ?> cache) throws IgniteCh
/** {@inheritDoc} */
@Override public void active(boolean active) {
- cluster().active(active);
+ if (active)
+ activate();
+ else
+ deactivate(false);
+ }
+
+ /** {@inheritDoc} */
+ @Override public void activate() {
+ cluster().state(ClusterState.ACTIVE);
+ }
+
+ /** {@inheritDoc} */
+ @Override public void deactivate(boolean force) {
+ //Check if cluster ir ready for deactivation.
+ if (cluster().state() == ClusterState.ACTIVE && !force) {
+ String msg = ClusterStateChangeCommand.isClusterReadyForDeactivation((cls -> compute().execute(cls, null)));
Review comment:
Implemented.
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r376387283
##########
File path: modules/core/src/main/java/org/apache/ignite/mxbean/IgniteMXBean.java
##########
@@ -380,23 +376,30 @@
*/
@MXBeanDescription("Pings node with given node ID to see whether it is alive. " +
"Returns whether or not node is alive.")
- @MXBeanParametersNames(
- "nodeId"
- )
- @MXBeanParametersDescriptions(
- "String presentation of node ID. See java.util.UUID class for details."
- )
+ @MXBeanParametersNames("nodeId")
+ @MXBeanParametersDescriptions("String presentation of node ID. See java.util.UUID class for details.")
public boolean pingNode(String nodeId);
+ /** Activates cluster. */
+ @MXBeanDescription("Execute activation process.")
+ @MXBeanParametersNames("active")
+ public void activate();
+
+ /** Deactivates cluster. */
+ @MXBeanDescription("Execute deactivation process.")
+ @MXBeanParametersNames("force")
Review comment:
JMX bean is a public API, so we should preserve backward compatibility.
----------------------------------------------------------------
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] NSAmelchev commented on a change in pull request #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375283384
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/ClusterStateChangeCommand.java
##########
@@ -68,14 +110,29 @@
/** {@inheritDoc} */
@Override public Object execute(GridClientConfiguration clientCfg, Logger log) throws Exception {
try (GridClient client = Command.startClient(clientCfg)) {
+
+ //Search for in-memory-only caches. Fail if possible data loss.
Review comment:
Missed space `// Search`. Check your comments.
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375370875
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/DeactivateCommand.java
##########
@@ -59,25 +69,51 @@
* @param clientCfg Client configuration.
* @throws Exception If failed to deactivate.
*/
- @Override public Object execute(GridClientConfiguration clientCfg, Logger logger) throws Exception {
- logger.warning("Command deprecated. Use " + SET_STATE.toString() + " instead.");
+ @Override public Object execute(GridClientConfiguration clientCfg, Logger log) throws Exception {
+ log.warning("Command deprecated. Use " + SET_STATE.toString() + " instead.");
try (GridClient client = Command.startClient(clientCfg)) {
+
+ //Search for in-memory-only caches. Warn of possible data loss.
+ if (!force) {
+ String msg = ClusterStateChangeCommand.isClusterReadyForDeactivation((cls) -> {
+ try {
+ return TaskExecutor.executeTask(client, cls, null, clientCfg);
+ }
+ catch (GridClientException e) {
+ throw new RuntimeException("Failed to launch task for checking check if cluster is ready for deactivation.", e);
+ }
+ });
+ if (!msg.isEmpty())
+ throw new IllegalStateException(msg + " Type --force to proceed.");
+ }
+
GridClientClusterState state = client.state();
state.active(false);
- logger.info("Cluster deactivated");
+ log.info("Cluster deactivated.");
}
catch (Exception e) {
- logger.severe("Failed to deactivate cluster.");
+ log.severe("Failed to deactivate cluster.");
throw e;
}
return null;
}
+ /** {@inheritDoc} */
+ @Override public void parseArguments(CommandArgIterator argIter) {
+ if (argIter.hasNextArg()) {
+ String arg = argIter.peekNextArg();
+ if ("--force".equalsIgnoreCase(arg)) {
Review comment:
Done.
----------------------------------------------------------------
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] NSAmelchev commented on a change in pull request #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375281032
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/DeactivateCommand.java
##########
@@ -59,25 +69,51 @@
* @param clientCfg Client configuration.
* @throws Exception If failed to deactivate.
*/
- @Override public Object execute(GridClientConfiguration clientCfg, Logger logger) throws Exception {
- logger.warning("Command deprecated. Use " + SET_STATE.toString() + " instead.");
+ @Override public Object execute(GridClientConfiguration clientCfg, Logger log) throws Exception {
+ log.warning("Command deprecated. Use " + SET_STATE.toString() + " instead.");
try (GridClient client = Command.startClient(clientCfg)) {
+
+ //Search for in-memory-only caches. Warn of possible data loss.
+ if (!force) {
+ String msg = ClusterStateChangeCommand.isClusterReadyForDeactivation((cls) -> {
+ try {
+ return TaskExecutor.executeTask(client, cls, null, clientCfg);
+ }
+ catch (GridClientException e) {
+ throw new RuntimeException("Failed to launch task for checking check if cluster is ready for deactivation.", e);
+ }
+ });
+ if (!msg.isEmpty())
+ throw new IllegalStateException(msg + " Type --force to proceed.");
+ }
+
GridClientClusterState state = client.state();
state.active(false);
- logger.info("Cluster deactivated");
+ log.info("Cluster deactivated.");
}
catch (Exception e) {
- logger.severe("Failed to deactivate cluster.");
+ log.severe("Failed to deactivate cluster.");
throw e;
}
return null;
}
+ /** {@inheritDoc} */
+ @Override public void parseArguments(CommandArgIterator argIter) {
+ if (argIter.hasNextArg()) {
+ String arg = argIter.peekNextArg();
+ if ("--force".equalsIgnoreCase(arg)) {
Review comment:
Can we define the force command as an static var?
----------------------------------------------------------------
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] NSAmelchev commented on a change in pull request #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375282443
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/DeactivateCommand.java
##########
@@ -59,25 +65,51 @@
* @param clientCfg Client configuration.
* @throws Exception If failed to deactivate.
*/
- @Override public Object execute(GridClientConfiguration clientCfg, Logger logger) throws Exception {
- logger.warning("Command deprecated. Use " + SET_STATE.toString() + " instead.");
+ @Override public Object execute(GridClientConfiguration clientCfg, Logger log) throws Exception {
+ log.warning("Command deprecated. Use " + SET_STATE.toString() + " instead.");
try (GridClient client = Command.startClient(clientCfg)) {
+
+ //Search for in-memory-only caches. Warn of possible data loss.
+ if (!force) {
+ String msg = ClusterStateChangeCommand.isClusterReadyForDeactivation((cls) -> {
+ try {
+ return TaskExecutor.executeTask(client, cls, null, clientCfg);
+ }
+ catch (GridClientException e) {
+ throw new RuntimeException("Failed to launch task for checking check if cluster is ready for deactivation.", e);
+ }
+ });
+ if (!msg.isEmpty())
+ throw new IllegalStateException(msg + " Type --force to proceed.");
+ }
+
GridClientClusterState state = client.state();
state.active(false);
- logger.info("Cluster deactivated");
+ log.info("Cluster deactivated.");
}
catch (Exception e) {
- logger.severe("Failed to deactivate cluster.");
+ log.severe("Failed to deactivate cluster.");
throw e;
}
return null;
}
+ /** {@inheritDoc} */
+ @Override public void parseArguments(CommandArgIterator argIter) {
+ if (argIter.hasNextArg()) {
+ String arg = argIter.peekNextArg();
+ if ("--force".equalsIgnoreCase(arg)) {
+ force = true;
+ argIter.nextArg("");
Review comment:
Missed break line.
----------------------------------------------------------------
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] NSAmelchev commented on a change in pull request #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375451168
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/ClusterStateChangeCommand.java
##########
@@ -36,12 +37,18 @@
* Command to change cluster state.
*/
public class ClusterStateChangeCommand implements Command<ClusterState> {
+ /** */
+ private static final String FLAG_FORCE = "--force";
Review comment:
I suggest name it `FORCE_COMMAND`
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375727493
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java
##########
@@ -3949,6 +3957,28 @@ private void checkNearCacheStarted(IgniteCacheProxy<?, ?> cache) throws IgniteCh
cluster().active(active);
}
+ /** {@inheritDoc} */
+ @Override public void activate() {
+ cluster().state(ClusterState.ACTIVE);
+ }
+
+ /** {@inheritDoc} */
+ @Override public void deactivate(boolean force) {
+ // Check if cluster ir ready for deactivation.
+ if ((cluster().state() == ClusterState.ACTIVE || cluster().state() == ClusterState.ACTIVE_READ_ONLY)
Review comment:
Impelemted
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375730812
##########
File path: modules/core/src/main/java/org/apache/ignite/mxbean/IgniteMXBean.java
##########
@@ -380,23 +376,30 @@
*/
@MXBeanDescription("Pings node with given node ID to see whether it is alive. " +
"Returns whether or not node is alive.")
- @MXBeanParametersNames(
- "nodeId"
- )
- @MXBeanParametersDescriptions(
- "String presentation of node ID. See java.util.UUID class for details."
- )
+ @MXBeanParametersNames("nodeId")
+ @MXBeanParametersDescriptions("String presentation of node ID. See java.util.UUID class for details.")
public boolean pingNode(String nodeId);
+ /** Activates cluster. */
+ @MXBeanDescription("Execute activation process.")
+ @MXBeanParametersNames("active")
Review comment:
Yep. 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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r376381816
##########
File path: modules/core/src/main/java/org/apache/ignite/mxbean/IgniteMXBean.java
##########
@@ -380,23 +376,30 @@
*/
@MXBeanDescription("Pings node with given node ID to see whether it is alive. " +
"Returns whether or not node is alive.")
- @MXBeanParametersNames(
- "nodeId"
- )
- @MXBeanParametersDescriptions(
- "String presentation of node ID. See java.util.UUID class for details."
- )
+ @MXBeanParametersNames("nodeId")
+ @MXBeanParametersDescriptions("String presentation of node ID. See java.util.UUID class for details.")
public boolean pingNode(String nodeId);
+ /** Activates cluster. */
+ @MXBeanDescription("Execute activation process.")
+ @MXBeanParametersNames("active")
+ public void activate();
+
+ /** Deactivates cluster. */
+ @MXBeanDescription("Execute deactivation process.")
+ @MXBeanParametersNames("force")
Review comment:
> But this is an JMX. I don't think it is unsafe to add single param.
Why do you think so?
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r377070501
##########
File path: modules/core/src/main/java/org/apache/ignite/mxbean/IgniteMXBean.java
##########
@@ -388,11 +388,31 @@
)
public boolean pingNode(String nodeId);
+ /** Activates cluster. */
+ @MXBeanDescription(
Review comment:
@nizhikov , how do you think, can we keep IgniteMXBean as it is now in this PR? With one new
`clusterState(String state, boolean force)`
Unfortunately, we can't use two different default-methods in the interfaces IgniteMXBean and Ignite:
` default public void active(boolean active){
active(active, false);
}
and
default public void active(boolean active){
active(active, true);
}`
Jaca compilator forces us to choose one in IgniteKernal:
`class org.apache.ignite.internal.IgniteKernal inherits unrelated defaults for active(boolean) from types org.apache.ignite.Ignite and org.apache.ignite.mxbean.IgniteMXBean`
I think this happens because there are no real implementations of default methods within interfaces. They are just code sample for in-lining to the implementation. So, we have two different implementations.
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375360780
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java
##########
@@ -3946,7 +3952,27 @@ private void checkNearCacheStarted(IgniteCacheProxy<?, ?> cache) throws IgniteCh
/** {@inheritDoc} */
@Override public void active(boolean active) {
- cluster().active(active);
+ if (active)
+ activate();
+ else
+ deactivate(false);
+ }
+
+ /** {@inheritDoc} */
+ @Override public void activate() {
+ cluster().state(ClusterState.ACTIVE);
+ }
+
+ /** {@inheritDoc} */
+ @Override public void deactivate(boolean force) {
+ //Check if cluster ir ready for deactivation.
+ if (cluster().state() == ClusterState.ACTIVE && !force) {
+ String msg = ClusterStateChangeCommand.isClusterReadyForDeactivation((cls -> compute().execute(cls, null)));
Review comment:
Implemented.
----------------------------------------------------------------
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] NSAmelchev commented on a change in pull request #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375282621
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/ClusterStateChangeCommand.java
##########
@@ -91,6 +148,14 @@
catch (IllegalArgumentException e) {
throw new IllegalArgumentException("Can't parse new cluster state. State: " + s, e);
}
+
+ if (argIter.hasNextArg()) {
+ String arg = argIter.peekNextArg();
+ if ("--force".equalsIgnoreCase(arg)) {
Review comment:
Missed break line.
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375364938
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/ClusterStateChangeCommand.java
##########
@@ -91,6 +148,14 @@
catch (IllegalArgumentException e) {
throw new IllegalArgumentException("Can't parse new cluster state. State: " + s, e);
}
+
+ if (argIter.hasNextArg()) {
+ String arg = argIter.peekNextArg();
+ if ("--force".equalsIgnoreCase(arg)) {
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r376379912
##########
File path: modules/core/src/main/java/org/apache/ignite/mxbean/IgniteMXBean.java
##########
@@ -380,23 +376,30 @@
*/
@MXBeanDescription("Pings node with given node ID to see whether it is alive. " +
"Returns whether or not node is alive.")
- @MXBeanParametersNames(
- "nodeId"
- )
- @MXBeanParametersDescriptions(
- "String presentation of node ID. See java.util.UUID class for details."
- )
+ @MXBeanParametersNames("nodeId")
+ @MXBeanParametersDescriptions("String presentation of node ID. See java.util.UUID class for details.")
public boolean pingNode(String nodeId);
+ /** Activates cluster. */
+ @MXBeanDescription("Execute activation process.")
+ @MXBeanParametersNames("active")
+ public void activate();
+
+ /** Deactivates cluster. */
+ @MXBeanDescription("Execute deactivation process.")
+ @MXBeanParametersNames("force")
Review comment:
@nizhikov , please review IgniteMXBean. We have to put flag 'force' somewhere for the deactivation. Current choice is
`IgniteMXBean#clusterState(String state) -> clusterState(String state, boolean force)`
Usually one has to support backward compatibility of an Interface. But this is an JMX. I don't think it is unsafe to add single param. Otherwise we need to implement additional method like
` clusterState(String state, boolean force)
or
deactivate(boolean force)`
whereas the mxbean already has several methods for the cluster state:
`
void active(boolean active)
boolean active()
void clusterState(String state)
String clusterState()
`
Don't think one more would be handy;
Also, should we declare
`IgniteMXBean#active(boolean active)`
deprecated too.
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375359122
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java
##########
@@ -3946,7 +3952,27 @@ private void checkNearCacheStarted(IgniteCacheProxy<?, ?> cache) throws IgniteCh
/** {@inheritDoc} */
@Override public void active(boolean active) {
- cluster().active(active);
+ if (active)
+ activate();
+ else
+ deactivate(false);
+ }
+
+ /** {@inheritDoc} */
+ @Override public void activate() {
+ cluster().state(ClusterState.ACTIVE);
+ }
+
+ /** {@inheritDoc} */
+ @Override public void deactivate(boolean force) {
+ //Check if cluster ir ready for deactivation.
+ if (cluster().state() == ClusterState.ACTIVE && !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] NSAmelchev commented on a change in pull request #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375435183
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/DeactivateCommand.java
##########
@@ -33,12 +34,19 @@
*/
@Deprecated
public class DeactivateCommand implements Command<Void> {
+ /** */
+ private static final String FLAG_FORCE = "--force";
Review comment:
Lets reuse var from the ClusterStateChangeCommand class
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r378087444
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/GridClusterStateProcessor.java
##########
@@ -1654,6 +1659,14 @@ public boolean isBaselineAutoAdjustEnabled() {
return distributedBaselineConfiguration.isBaselineAutoAdjustEnabled();
}
+ /**
+ * @return {@code False} if cluster deactivation can erase user's data and objects.
+ * {@code True} If deactivation is safe.
+ */
+ public boolean isDeactivationSafe() {
Review comment:
Please, use `ctx.cache().cacheDescriptors()` here and remove `GridCacheProcessor#inMemoryCaches` method.
```
return ctx.cache().cacheDescriptors().values().stream()
.filter(desc -> !isPersistentCache(desc.cacheConfiguration(), ctx.config().getDataStorageConfiguration()))
.findFirst()
.isPresent();
```
----------------------------------------------------------------
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] NSAmelchev commented on a change in pull request #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375316632
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/ClusterStateChangeCommand.java
##########
@@ -102,4 +167,53 @@
@Override public String name() {
return SET_STATE.toCommandName();
}
+
+ /** Searches for any non-persistent cache. */
+ private static class FindNotPersistentCachesJob extends ComputeJobAdapter {
+
+ @IgniteInstanceResource
+ private Ignite ignite;
+
+ /** */
+ @SuppressWarnings("unchecked")
+ @Override public Boolean execute() throws IgniteException {
+ //Find any node with disabled memory reusage on deactivation/activation.
+ boolean cacheDataCanBeLost = ignite.cluster().forPredicate(node->
+ (Boolean)node.attributes().getOrDefault(ATTR_MEMORY_ERASURE_ON_DEACTIVATION, true))
+ .nodes().stream().findAny().isPresent();
+
+ if (cacheDataCanBeLost) {
+ //Find data region to set persistent flag.
+ for (String cacheName : ignite.cacheNames()) {
Review comment:
Lets use the same approach as for JMX
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375363696
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/ClusterStateChangeCommand.java
##########
@@ -68,14 +110,29 @@
/** {@inheritDoc} */
@Override public Object execute(GridClientConfiguration clientCfg, Logger log) throws Exception {
try (GridClient client = Command.startClient(clientCfg)) {
+
+ //Search for in-memory-only caches. Fail if possible data loss.
Review comment:
Fixed. Checked other similar.
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375372945
##########
File path: modules/core/src/main/java/org/apache/ignite/mxbean/IgniteMXBean.java
##########
@@ -388,11 +388,31 @@
)
public boolean pingNode(String nodeId);
+ /** Activates cluster. */
+ @MXBeanDescription(
Review comment:
Fixed. Also the others.
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375359302
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java
##########
@@ -3946,7 +3952,27 @@ private void checkNearCacheStarted(IgniteCacheProxy<?, ?> cache) throws IgniteCh
/** {@inheritDoc} */
@Override public void active(boolean active) {
- cluster().active(active);
+ if (active)
+ activate();
+ else
+ deactivate(false);
+ }
+
+ /** {@inheritDoc} */
+ @Override public void activate() {
+ cluster().state(ClusterState.ACTIVE);
+ }
+
+ /** {@inheritDoc} */
+ @Override public void deactivate(boolean force) {
+ //Check if cluster ir ready for deactivation.
+ if (cluster().state() == ClusterState.ACTIVE && !force) {
+ String msg = ClusterStateChangeCommand.isClusterReadyForDeactivation((cls -> compute().execute(cls, null)));
Review comment:
Not actual. Code changed.
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r376394271
##########
File path: modules/core/src/main/java/org/apache/ignite/mxbean/IgniteMXBean.java
##########
@@ -380,23 +376,30 @@
*/
@MXBeanDescription("Pings node with given node ID to see whether it is alive. " +
"Returns whether or not node is alive.")
- @MXBeanParametersNames(
- "nodeId"
- )
- @MXBeanParametersDescriptions(
- "String presentation of node ID. See java.util.UUID class for details."
- )
+ @MXBeanParametersNames("nodeId")
+ @MXBeanParametersDescriptions("String presentation of node ID. See java.util.UUID class for details.")
public boolean pingNode(String nodeId);
+ /** Activates cluster. */
+ @MXBeanDescription("Execute activation process.")
+ @MXBeanParametersNames("active")
+ public void activate();
+
+ /** Deactivates cluster. */
+ @MXBeanDescription("Execute deactivation process.")
+ @MXBeanParametersNames("force")
Review comment:
I think it should be discussed on the dev-list
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375371188
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/DeactivateCommand.java
##########
@@ -59,25 +65,51 @@
* @param clientCfg Client configuration.
* @throws Exception If failed to deactivate.
*/
- @Override public Object execute(GridClientConfiguration clientCfg, Logger logger) throws Exception {
- logger.warning("Command deprecated. Use " + SET_STATE.toString() + " instead.");
+ @Override public Object execute(GridClientConfiguration clientCfg, Logger log) throws Exception {
+ log.warning("Command deprecated. Use " + SET_STATE.toString() + " instead.");
try (GridClient client = Command.startClient(clientCfg)) {
+
+ //Search for in-memory-only caches. Warn of possible data loss.
+ if (!force) {
+ String msg = ClusterStateChangeCommand.isClusterReadyForDeactivation((cls) -> {
+ try {
+ return TaskExecutor.executeTask(client, cls, null, clientCfg);
+ }
+ catch (GridClientException e) {
+ throw new RuntimeException("Failed to launch task for checking check if cluster is ready for deactivation.", e);
+ }
+ });
+ if (!msg.isEmpty())
+ throw new IllegalStateException(msg + " Type --force to proceed.");
+ }
+
GridClientClusterState state = client.state();
state.active(false);
- logger.info("Cluster deactivated");
+ log.info("Cluster deactivated.");
}
catch (Exception e) {
- logger.severe("Failed to deactivate cluster.");
+ log.severe("Failed to deactivate cluster.");
throw e;
}
return null;
}
+ /** {@inheritDoc} */
+ @Override public void parseArguments(CommandArgIterator argIter) {
+ if (argIter.hasNextArg()) {
+ String arg = argIter.peekNextArg();
+ if ("--force".equalsIgnoreCase(arg)) {
+ force = true;
+ argIter.nextArg("");
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] NSAmelchev commented on a change in pull request #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375454887
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/visor/cluster/VisorCheckDeactivationTask.java
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.visor.cluster;
+
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.processors.cache.CacheType;
+import org.apache.ignite.internal.processors.task.GridInternal;
+import org.apache.ignite.internal.visor.VisorJob;
+import org.apache.ignite.internal.visor.VisorOneNodeTask;
+
+import static org.apache.ignite.internal.IgniteNodeAttributes.ATTR_KEEP_MEMORY_ON_DEACTIVATION;
+
+/**
+ * Task for checking if deactiation is safe. Returns false when deactivation can lead to data loss.
+ */
+@GridInternal
+public class VisorCheckDeactivationTask extends VisorOneNodeTask<Void, Boolean> {
+ /** Serial version uid. */
+ private static final long serialVersionUID = 0L;
+
+ /** */
+ public static final String WARN_DEACTIVATION_IN_MEM_CACHES =
Review comment:
I think we can move it to the GridClusterStateProcessor. Add javadoc, please
----------------------------------------------------------------
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 #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375727740
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/ClusterStateChangeCommand.java
##########
@@ -36,12 +37,18 @@
* Command to change cluster state.
*/
public class ClusterStateChangeCommand implements Command<ClusterState> {
+ /** */
+ private static final String FLAG_FORCE = "--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] NSAmelchev commented on a change in pull request #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375452889
##########
File path: modules/core/src/main/java/org/apache/ignite/mxbean/IgniteMXBean.java
##########
@@ -380,23 +376,30 @@
*/
@MXBeanDescription("Pings node with given node ID to see whether it is alive. " +
"Returns whether or not node is alive.")
- @MXBeanParametersNames(
- "nodeId"
- )
- @MXBeanParametersDescriptions(
- "String presentation of node ID. See java.util.UUID class for details."
- )
+ @MXBeanParametersNames("nodeId")
+ @MXBeanParametersDescriptions("String presentation of node ID. See java.util.UUID class for details.")
public boolean pingNode(String nodeId);
+ /** Activates cluster. */
+ @MXBeanDescription("Execute activation process.")
+ @MXBeanParametersNames("active")
Review comment:
There is no parametrs
----------------------------------------------------------------
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] NSAmelchev commented on a change in pull request #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375311642
##########
File path: modules/core/src/main/java/org/apache/ignite/mxbean/IgniteMXBean.java
##########
@@ -388,11 +388,31 @@
)
public boolean pingNode(String nodeId);
+ /** Activates cluster. */
+ @MXBeanDescription(
+ "Execute activation process."
+ )
+ @MXBeanParametersNames(
+ "active"
+ )
+ public void activate();
+
+ /** Deactivates cluster. */
+ @MXBeanDescription(
+ "Execute deactivation process."
+ )
+ @MXBeanParametersNames(
+ "force"
+ )
+ public void deactivate(boolean force);
+
/**
* @param active Activate/DeActivate flag.
+ * @deprecated Use {@link #activate()} and {@link #deactivate(boolean)} instead.
*/
+ @Deprecated
@MXBeanDescription(
- "Execute activate or deactivate process."
+ "Execute activate or deactivate process. Deprecated. Use activate() / deactivate(). Deactivation may require flag 'force'."
Review comment:
More than 120 chars in one line
----------------------------------------------------------------
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] NSAmelchev commented on a change in pull request #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375284235
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/ClusterStateChangeCommand.java
##########
@@ -102,4 +167,53 @@
@Override public String name() {
return SET_STATE.toCommandName();
}
+
+ /** Searches for any non-persistent cache. */
+ private static class FindNotPersistentCachesJob extends ComputeJobAdapter {
+
Review comment:
Unnecessary break line and missed javadoc
----------------------------------------------------------------
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] NSAmelchev commented on a change in pull request #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375285463
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java
##########
@@ -3946,7 +3952,27 @@ private void checkNearCacheStarted(IgniteCacheProxy<?, ?> cache) throws IgniteCh
/** {@inheritDoc} */
@Override public void active(boolean active) {
- cluster().active(active);
+ if (active)
+ activate();
+ else
+ deactivate(false);
+ }
+
+ /** {@inheritDoc} */
+ @Override public void activate() {
+ cluster().state(ClusterState.ACTIVE);
+ }
+
+ /** {@inheritDoc} */
+ @Override public void deactivate(boolean force) {
+ //Check if cluster ir ready for deactivation.
+ if (cluster().state() == ClusterState.ACTIVE && !force) {
+ String msg = ClusterStateChangeCommand.isClusterReadyForDeactivation((cls -> compute().execute(cls, null)));
Review comment:
Add break lines, please. See Coding Guidelines
----------------------------------------------------------------
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] NSAmelchev commented on a change in pull request #7358:
IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem
data loss.
Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #7358: IGNITE-12614 : Disallow silent deactivation of cluster to prevent in-mem data loss.
URL: https://github.com/apache/ignite/pull/7358#discussion_r375305928
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java
##########
@@ -3946,7 +3952,27 @@ private void checkNearCacheStarted(IgniteCacheProxy<?, ?> cache) throws IgniteCh
/** {@inheritDoc} */
@Override public void active(boolean active) {
- cluster().active(active);
+ if (active)
+ activate();
+ else
+ deactivate(false);
+ }
+
+ /** {@inheritDoc} */
+ @Override public void activate() {
+ cluster().state(ClusterState.ACTIVE);
+ }
+
+ /** {@inheritDoc} */
+ @Override public void deactivate(boolean force) {
+ //Check if cluster ir ready for deactivation.
+ if (cluster().state() == ClusterState.ACTIVE && !force) {
+ String msg = ClusterStateChangeCommand.isClusterReadyForDeactivation((cls -> compute().execute(cls, null)));
Review comment:
I suggest check without compute call:
`!ctx.cache().inMemoryCaches().isEmpty()`
The `memoryCaches` method should be introduced. See `persistentCaches() `for example
----------------------------------------------------------------
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