Various shutdown guaranties

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

Various shutdown guaranties

V.Pyatkov
Hi

We need to have ability to calling shutdown with various guaranties.
For example:
Need to reboot a node, but after that node should be available for
historical rebalance (all partitions in MOVING state should have gone to
OWNING).

Implemented a circled reboot of cluster, but all data should be available on
that time (at least one copy of partition should be available in cluster).

Need to wait not only data available, but all jobs (before this behavior
available through a stop(false) method invocation).

All these reason required various behavior before shutting down node.
I propose slightly modify public API and add here method which shown on
shutdown behavior directly:
Ignite.close(Shutdown)

/public enum Shutdownn {
    /**
     * Stop immediately as soon components are ready.
     */
    IMMEDIATE,
    /**
     * Stop node when all partitions completed moving from/to this node to
another.
     */
    NORMAL,
    /**
     * Node will stop if and only if it does not store any unique
partitions, that does not have copies on cluster.
     */
    GRACEFUL,
    /**
     * Node stops graceful and wait all jobs before shutdown.
     */
    ALL
}/

Method close without parameter Ignite.close() will get shutdown behavior
configured for cluster wide. It will be implemented through distributed meta
storage and additional utilities for configuration.
Also, will be added a method to configure shutdown on start, this is look as
IgniteConfiguration.setShutdown(Shutdown).
If shutting down did not configure all be worked as before according to
IMMEDIATE behavior.
All other close method will be marked as deprecated.

I will be waiting for your opinions.



--
Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
Reply | Threaded
Open this post in threaded view
|

Re: Various shutdown guaranties

Ivan Rakov
Vlad, thanks for starting this discussion.

I'll try to clarify the motivation for this change as I see it.
In general, Ignite clusters are vulnerable to the data loss. Of course, we
have configurable PartitionLossPolicy, which allows to handle data loss
safely and mitigate its consequences. But being able to avoid critical
situations is always better than being able to recover from it.

The most common issue from my perspective is absence of a way to perform
rolling cluster restart safely. Scenario:
1. Backup count is 1
2. Admin wants to perform rolling restart in order to deploy new version of
business code that uses Ignite in embedded mode
3. Admin shuts down first node, replaces needed binaries and returns the
node back to the topology
4. Node joins the cluster successfully
5. Admin shuts down second node
6. Data loss happens: the second node was the only owner of a certain
partition, which was being rebalanced from the second node to the first

We can prevent such situations by introducing "safe shutdown by default"
mode, which blocks stopping node while it remains the only owner for at
least one partition. It should be applied to "common" ways of stopping
nodes - Ignite.close() and kill <pid>.
I think, option to be enabled or disabled in runtime should be a
requirement for this behavior. Safe shutdown mode has weird side-effects.
For example, admin won't be able to stop the whole cluster: stop of last
node will be blocked, because the last node is the only present owner of
all its partitions. Sure, kill -9 will resolve it, but it's still a
usability issue.

With the described dynamic property scenario will be changed as follows:
1. Admin enables "safe shutdown" mode
2. Admin shuts down first node, replaces needed binaries and returns the
node back to the topology
3. Admin shuts down second node (with either ignite.close() or kill <pid>),
shutdown is blocked until the first node returns to the topology and
completes the rebalancing process
4. Admin proceeds the rolling restart procedure
5. Admin disables "safe shutdown" mode

This logic will also simplify the rolling restart scenario in K8S. Pod with
Ignite node won't be terminated until its termination will cause data loss.

Aside from waiting for backups, Ignition interface provide lots of options
to perform various node stop:
- Whether or not to cancel pending compute jobs
- Whether or not to perform instant halt() instead of any graceful stop
logic
- Whether or not to wait for some timeout before halt()
- Whether or not the stopped grid should be restarted
All these "stop" methods provide very custom logic. I don't see a need to
make them part of dynamic cluster-wide configuration. They still can be
invoked directly via Java API. Later we can extract some of them to dynamic
cluster-wide parameters of default stop if it will become necessary. That's
why I think we should create an enum for default shutdown policy, but only
with two options so far (we can add more later): DEFAULT and
WAIT_FOR_BACKUPS.
Regarding the "NORMAL" option that you propose (where the node is not
stopped until the rebalance is finished): I don't think that we should add
it. It doesn't ensure any strict guarantees: the data still can be lost
with it.

To sum it up, I propose:
1. Add a new method to Ignition interface to make it possible to stop with
"wait for backups" logic directly via Java API, like Ignition.stop(boolean
cancel, boolean waitForBackups)
2. Introduce "defaultShutdownPolicy" as a dynamic cluster configuration,
two values are available so far: DEFAULT and WAIT_FOR_BACKUPS
3. This property is stored in the distributed metastorage (thus persisted),
can be changed via Java API and ./control.sh
4. Behavior configured with this property will be applied only on common
ways of stopping the node - Ignite.close() and kill <pid>.
5. *Don't* add new options to the static IgniteConfiguration to avoid
conflicts between dynamic and static configuration

--
Best Regards,
Ivan Rakov

On Mon, Jun 8, 2020 at 6:44 PM V.Pyatkov <[hidden email]> wrote:

> Hi
>
> We need to have ability to calling shutdown with various guaranties.
> For example:
> Need to reboot a node, but after that node should be available for
> historical rebalance (all partitions in MOVING state should have gone to
> OWNING).
>
> Implemented a circled reboot of cluster, but all data should be available
> on
> that time (at least one copy of partition should be available in cluster).
>
> Need to wait not only data available, but all jobs (before this behavior
> available through a stop(false) method invocation).
>
> All these reason required various behavior before shutting down node.
> I propose slightly modify public API and add here method which shown on
> shutdown behavior directly:
> Ignite.close(Shutdown)
>
> /public enum Shutdownn {
>     /**
>      * Stop immediately as soon components are ready.
>      */
>     IMMEDIATE,
>     /**
>      * Stop node when all partitions completed moving from/to this node to
> another.
>      */
>     NORMAL,
>     /**
>      * Node will stop if and only if it does not store any unique
> partitions, that does not have copies on cluster.
>      */
>     GRACEFUL,
>     /**
>      * Node stops graceful and wait all jobs before shutdown.
>      */
>     ALL
> }/
>
> Method close without parameter Ignite.close() will get shutdown behavior
> configured for cluster wide. It will be implemented through distributed
> meta
> storage and additional utilities for configuration.
> Also, will be added a method to configure shutdown on start, this is look
> as
> IgniteConfiguration.setShutdown(Shutdown).
> If shutting down did not configure all be worked as before according to
> IMMEDIATE behavior.
> All other close method will be marked as deprecated.
>
> I will be waiting for your opinions.
>
>
>
> --
> Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
>
Reply | Threaded
Open this post in threaded view
|

Re: Various shutdown guaranties

Alexei Scherbakov
Ivan Rakov,

Your proposal overall looks good to me. My comments:

1. I would avoid adding such a method, because it will be impossible to
change it in the future if some more shutdown policies will be introduced
later.
Also shutdown policy must be always consistent on the grid or unintentional
data loss is possible if two nodes are stopping simultaneously with
different policies.

This behavior can be achieved by changing policy globally when stopping a
node:
ignite.cluster().setShutdownPolicy(DEFAULT);
ignore.stop();

2. defaultShutdownPolicy with DEFAULT value is a mess. WAIT_FOR_BACKUPS is
not very clear either.
Let's use shutdownPolicy=DEFAULT|GRACEFUL, as was proposed by me earlier.

3. OK

4. OK

5. Let's keep a static property for simplifying setting of initial
behavior.
In most cases the policy will never be changed during grid's lifetime.
No need for an explicit call to API on grid start.
A joining node should check a local configuration value to match the grid.
If a dynamic value is already present in a metastore, it should override
static value with a warning.




пн, 8 июн. 2020 г. в 19:06, Ivan Rakov <[hidden email]>:

> Vlad, thanks for starting this discussion.
>
> I'll try to clarify the motivation for this change as I see it.
> In general, Ignite clusters are vulnerable to the data loss. Of course, we
> have configurable PartitionLossPolicy, which allows to handle data loss
> safely and mitigate its consequences. But being able to avoid critical
> situations is always better than being able to recover from it.
>
> The most common issue from my perspective is absence of a way to perform
> rolling cluster restart safely. Scenario:
> 1. Backup count is 1
> 2. Admin wants to perform rolling restart in order to deploy new version of
> business code that uses Ignite in embedded mode
> 3. Admin shuts down first node, replaces needed binaries and returns the
> node back to the topology
> 4. Node joins the cluster successfully
> 5. Admin shuts down second node
> 6. Data loss happens: the second node was the only owner of a certain
> partition, which was being rebalanced from the second node to the first
>
> We can prevent such situations by introducing "safe shutdown by default"
> mode, which blocks stopping node while it remains the only owner for at
> least one partition. It should be applied to "common" ways of stopping
> nodes - Ignite.close() and kill <pid>.
> I think, option to be enabled or disabled in runtime should be a
> requirement for this behavior. Safe shutdown mode has weird side-effects.
> For example, admin won't be able to stop the whole cluster: stop of last
> node will be blocked, because the last node is the only present owner of
> all its partitions. Sure, kill -9 will resolve it, but it's still a
> usability issue.
>
> With the described dynamic property scenario will be changed as follows:
> 1. Admin enables "safe shutdown" mode
> 2. Admin shuts down first node, replaces needed binaries and returns the
> node back to the topology
> 3. Admin shuts down second node (with either ignite.close() or kill <pid>),
> shutdown is blocked until the first node returns to the topology and
> completes the rebalancing process
> 4. Admin proceeds the rolling restart procedure
> 5. Admin disables "safe shutdown" mode
>
> This logic will also simplify the rolling restart scenario in K8S. Pod with
> Ignite node won't be terminated until its termination will cause data loss.
>
> Aside from waiting for backups, Ignition interface provide lots of options
> to perform various node stop:
> - Whether or not to cancel pending compute jobs
> - Whether or not to perform instant halt() instead of any graceful stop
> logic
> - Whether or not to wait for some timeout before halt()
> - Whether or not the stopped grid should be restarted
> All these "stop" methods provide very custom logic. I don't see a need to
> make them part of dynamic cluster-wide configuration. They still can be
> invoked directly via Java API. Later we can extract some of them to dynamic
> cluster-wide parameters of default stop if it will become necessary. That's
> why I think we should create an enum for default shutdown policy, but only
> with two options so far (we can add more later): DEFAULT and
> WAIT_FOR_BACKUPS.
> Regarding the "NORMAL" option that you propose (where the node is not
> stopped until the rebalance is finished): I don't think that we should add
> it. It doesn't ensure any strict guarantees: the data still can be lost
> with it.
>
> To sum it up, I propose:
> 1. Add a new method to Ignition interface to make it possible to stop with
> "wait for backups" logic directly via Java API, like Ignition.stop(boolean
> cancel, boolean waitForBackups)
> 2. Introduce "defaultShutdownPolicy" as a dynamic cluster configuration,
> two values are available so far: DEFAULT and WAIT_FOR_BACKUPS
> 3. This property is stored in the distributed metastorage (thus persisted),
> can be changed via Java API and ./control.sh
> 4. Behavior configured with this property will be applied only on common
> ways of stopping the node - Ignite.close() and kill <pid>.
> 5. *Don't* add new options to the static IgniteConfiguration to avoid
> conflicts between dynamic and static configuration
>
> --
> Best Regards,
> Ivan Rakov
>
> On Mon, Jun 8, 2020 at 6:44 PM V.Pyatkov <[hidden email]> wrote:
>
> > Hi
> >
> > We need to have ability to calling shutdown with various guaranties.
> > For example:
> > Need to reboot a node, but after that node should be available for
> > historical rebalance (all partitions in MOVING state should have gone to
> > OWNING).
> >
> > Implemented a circled reboot of cluster, but all data should be available
> > on
> > that time (at least one copy of partition should be available in
> cluster).
> >
> > Need to wait not only data available, but all jobs (before this behavior
> > available through a stop(false) method invocation).
> >
> > All these reason required various behavior before shutting down node.
> > I propose slightly modify public API and add here method which shown on
> > shutdown behavior directly:
> > Ignite.close(Shutdown)
> >
> > /public enum Shutdownn {
> >     /**
> >      * Stop immediately as soon components are ready.
> >      */
> >     IMMEDIATE,
> >     /**
> >      * Stop node when all partitions completed moving from/to this node
> to
> > another.
> >      */
> >     NORMAL,
> >     /**
> >      * Node will stop if and only if it does not store any unique
> > partitions, that does not have copies on cluster.
> >      */
> >     GRACEFUL,
> >     /**
> >      * Node stops graceful and wait all jobs before shutdown.
> >      */
> >     ALL
> > }/
> >
> > Method close without parameter Ignite.close() will get shutdown behavior
> > configured for cluster wide. It will be implemented through distributed
> > meta
> > storage and additional utilities for configuration.
> > Also, will be added a method to configure shutdown on start, this is look
> > as
> > IgniteConfiguration.setShutdown(Shutdown).
> > If shutting down did not configure all be worked as before according to
> > IMMEDIATE behavior.
> > All other close method will be marked as deprecated.
> >
> > I will be waiting for your opinions.
> >
> >
> >
> > --
> > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> >
>


--

Best regards,
Alexei Scherbakov
Reply | Threaded
Open this post in threaded view
|

Re: Various shutdown guaranties

Ivan Rakov-2
Alex,

Also shutdown policy must be always consistent on the grid or unintentional
> data loss is possible if two nodes are stopping simultaneously with
> different policies.

 Totally agree.

Let's use shutdownPolicy=DEFAULT|GRACEFUL, as was proposed by me earlier.

 I'm ok with GRACEFUL instead of WAIT_FOR_BACKUPS.

5. Let's keep a static property for simplifying setting of initial
> behavior.
> In most cases the policy will never be changed during grid's lifetime.
> No need for an explicit call to API on grid start.
> A joining node should check a local configuration value to match the grid.
> If a dynamic value is already present in a metastore, it should override
> static value with a warning.

To sum it up:
- ShutdownPolicy can be set with static configuration
(IgniteConfiguration#setShutdownPolicy), on join we validate that
statically configured policies on different server nodes are the same
- It's possible to override statically configured value by adding
distributed metastorage value, which can be done by
calling ignite.cluster().setShutdownPolicy(plc) or control.sh method
- Dynamic property is persisted

Generally, I don't mind if we have both dynamic and static configuration
properties. Necessity to call ignite.cluster().setShutdownPolicy(plc); on
every new cluster creation is a usability issue itself.
What bothers me here are the possible conflicts between static and dynamic
configuration. User may be surprised if he has shutdown policy X in
IgniteConfiguration, but the cluster behaves according to policy Y (because
several months ago another admin had called
IgniteCluster#setShutdownPolicy).
We can handle it by adding a separate enum field to the shutdown policy:

> public enum ShutdownPolicy {
>   /* Default value of dynamic shutdown policy property. If it's set, the
> shutdown policy is resolved according to value of static {@link
> IgniteConfiguration#shutdownPolicy} configuration parameter. */
>   USE_STATIC_CONFIGURATION,
>
>   /* Node leaves the cluster even if it's the last owner of some
> partitions. Only partitions of caches with backups > 0 are taken into
> account. */
>   IMMEDIATE,
>
>   /* Shutdown is blocked until node is safe to leave without the data
> loss. */
>   GRACEFUL
> }
>
This way:
1) User may easily understand whether the static parameter is overridden by
dynamic. If ignite.cluster().getShutdownPolicy() return anything except
USE_STATIC_CONFIGURATION, behavior is overridden.
2) User may clear previous overriding by calling
ignite.cluster().setShutdownPolicy(USE_STATIC_CONFIGURATION). After that,
behavior will be resolved based in IgniteConfiguration#shutdownPolicy again.
If we agree on this mechanism, I propose to use IMMEDIATE name instead of
DEFAULT for non-safe policy in order to don't confuse user.
Meanwhile, static configuration will accept the same enum, but
USE_STATIC_CONFIGURATION will be restricted:

> public class IgniteConfiguration {
>   public static final ShutdownPolicy DFLT_STATIC_SHUTDOWN_POLICY =
> IMMEDIATE;
>   private ShutdownPolicy shutdownPolicy = DFLT_STATIC_SHUTDOWN_POLICY;
>   ...
>   public void setShutdownPolicy(ShutdownPolicy shutdownPlc) {
>     if (shutdownPlc ==  USE_STATIC_CONFIGURATION)
>       throw new IllegalArgumentException("USE_STATIC_CONFIGURATION can
> only be passed as dynamic property value via
> ignite.cluster().setShutdownPolicy");
>     ...
>   }
> ...
> }
>

What do you think?


On Tue, Jun 9, 2020 at 11:46 AM Alexei Scherbakov <
[hidden email]> wrote:

> Ivan Rakov,
>
> Your proposal overall looks good to me. My comments:
>
> 1. I would avoid adding such a method, because it will be impossible to
> change it in the future if some more shutdown policies will be introduced
> later.
> Also shutdown policy must be always consistent on the grid or unintentional
> data loss is possible if two nodes are stopping simultaneously with
> different policies.
>
> This behavior can be achieved by changing policy globally when stopping a
> node:
> ignite.cluster().setShutdownPolicy(DEFAULT);
> ignore.stop();
>
> 2. defaultShutdownPolicy with DEFAULT value is a mess. WAIT_FOR_BACKUPS is
> not very clear either.
> Let's use shutdownPolicy=DEFAULT|GRACEFUL, as was proposed by me earlier.
>
> 3. OK
>
> 4. OK
>
> 5. Let's keep a static property for simplifying setting of initial
> behavior.
> In most cases the policy will never be changed during grid's lifetime.
> No need for an explicit call to API on grid start.
> A joining node should check a local configuration value to match the grid.
> If a dynamic value is already present in a metastore, it should override
> static value with a warning.
>
>
>
>
> пн, 8 июн. 2020 г. в 19:06, Ivan Rakov <[hidden email]>:
>
> > Vlad, thanks for starting this discussion.
> >
> > I'll try to clarify the motivation for this change as I see it.
> > In general, Ignite clusters are vulnerable to the data loss. Of course,
> we
> > have configurable PartitionLossPolicy, which allows to handle data loss
> > safely and mitigate its consequences. But being able to avoid critical
> > situations is always better than being able to recover from it.
> >
> > The most common issue from my perspective is absence of a way to perform
> > rolling cluster restart safely. Scenario:
> > 1. Backup count is 1
> > 2. Admin wants to perform rolling restart in order to deploy new version
> of
> > business code that uses Ignite in embedded mode
> > 3. Admin shuts down first node, replaces needed binaries and returns the
> > node back to the topology
> > 4. Node joins the cluster successfully
> > 5. Admin shuts down second node
> > 6. Data loss happens: the second node was the only owner of a certain
> > partition, which was being rebalanced from the second node to the first
> >
> > We can prevent such situations by introducing "safe shutdown by default"
> > mode, which blocks stopping node while it remains the only owner for at
> > least one partition. It should be applied to "common" ways of stopping
> > nodes - Ignite.close() and kill <pid>.
> > I think, option to be enabled or disabled in runtime should be a
> > requirement for this behavior. Safe shutdown mode has weird side-effects.
> > For example, admin won't be able to stop the whole cluster: stop of last
> > node will be blocked, because the last node is the only present owner of
> > all its partitions. Sure, kill -9 will resolve it, but it's still a
> > usability issue.
> >
> > With the described dynamic property scenario will be changed as follows:
> > 1. Admin enables "safe shutdown" mode
> > 2. Admin shuts down first node, replaces needed binaries and returns the
> > node back to the topology
> > 3. Admin shuts down second node (with either ignite.close() or kill
> <pid>),
> > shutdown is blocked until the first node returns to the topology and
> > completes the rebalancing process
> > 4. Admin proceeds the rolling restart procedure
> > 5. Admin disables "safe shutdown" mode
> >
> > This logic will also simplify the rolling restart scenario in K8S. Pod
> with
> > Ignite node won't be terminated until its termination will cause data
> loss.
> >
> > Aside from waiting for backups, Ignition interface provide lots of
> options
> > to perform various node stop:
> > - Whether or not to cancel pending compute jobs
> > - Whether or not to perform instant halt() instead of any graceful stop
> > logic
> > - Whether or not to wait for some timeout before halt()
> > - Whether or not the stopped grid should be restarted
> > All these "stop" methods provide very custom logic. I don't see a need to
> > make them part of dynamic cluster-wide configuration. They still can be
> > invoked directly via Java API. Later we can extract some of them to
> dynamic
> > cluster-wide parameters of default stop if it will become necessary.
> That's
> > why I think we should create an enum for default shutdown policy, but
> only
> > with two options so far (we can add more later): DEFAULT and
> > WAIT_FOR_BACKUPS.
> > Regarding the "NORMAL" option that you propose (where the node is not
> > stopped until the rebalance is finished): I don't think that we should
> add
> > it. It doesn't ensure any strict guarantees: the data still can be lost
> > with it.
> >
> > To sum it up, I propose:
> > 1. Add a new method to Ignition interface to make it possible to stop
> with
> > "wait for backups" logic directly via Java API, like
> Ignition.stop(boolean
> > cancel, boolean waitForBackups)
> > 2. Introduce "defaultShutdownPolicy" as a dynamic cluster configuration,
> > two values are available so far: DEFAULT and WAIT_FOR_BACKUPS
> > 3. This property is stored in the distributed metastorage (thus
> persisted),
> > can be changed via Java API and ./control.sh
> > 4. Behavior configured with this property will be applied only on common
> > ways of stopping the node - Ignite.close() and kill <pid>.
> > 5. *Don't* add new options to the static IgniteConfiguration to avoid
> > conflicts between dynamic and static configuration
> >
> > --
> > Best Regards,
> > Ivan Rakov
> >
> > On Mon, Jun 8, 2020 at 6:44 PM V.Pyatkov <[hidden email]> wrote:
> >
> > > Hi
> > >
> > > We need to have ability to calling shutdown with various guaranties.
> > > For example:
> > > Need to reboot a node, but after that node should be available for
> > > historical rebalance (all partitions in MOVING state should have gone
> to
> > > OWNING).
> > >
> > > Implemented a circled reboot of cluster, but all data should be
> available
> > > on
> > > that time (at least one copy of partition should be available in
> > cluster).
> > >
> > > Need to wait not only data available, but all jobs (before this
> behavior
> > > available through a stop(false) method invocation).
> > >
> > > All these reason required various behavior before shutting down node.
> > > I propose slightly modify public API and add here method which shown on
> > > shutdown behavior directly:
> > > Ignite.close(Shutdown)
> > >
> > > /public enum Shutdownn {
> > >     /**
> > >      * Stop immediately as soon components are ready.
> > >      */
> > >     IMMEDIATE,
> > >     /**
> > >      * Stop node when all partitions completed moving from/to this node
> > to
> > > another.
> > >      */
> > >     NORMAL,
> > >     /**
> > >      * Node will stop if and only if it does not store any unique
> > > partitions, that does not have copies on cluster.
> > >      */
> > >     GRACEFUL,
> > >     /**
> > >      * Node stops graceful and wait all jobs before shutdown.
> > >      */
> > >     ALL
> > > }/
> > >
> > > Method close without parameter Ignite.close() will get shutdown
> behavior
> > > configured for cluster wide. It will be implemented through distributed
> > > meta
> > > storage and additional utilities for configuration.
> > > Also, will be added a method to configure shutdown on start, this is
> look
> > > as
> > > IgniteConfiguration.setShutdown(Shutdown).
> > > If shutting down did not configure all be worked as before according to
> > > IMMEDIATE behavior.
> > > All other close method will be marked as deprecated.
> > >
> > > I will be waiting for your opinions.
> > >
> > >
> > >
> > > --
> > > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> > >
> >
>
>
> --
>
> Best regards,
> Alexei Scherbakov
>


--
Best Regards,
Ivan Rakov
Reply | Threaded
Open this post in threaded view
|

Re: Various shutdown guaranties

Ivan Rakov
In reply to this post by Alexei Scherbakov
Alex,

Also shutdown policy must be always consistent on the grid or unintentional
> data loss is possible if two nodes are stopping simultaneously with
> different policies.

 Totally agree.

Let's use shutdownPolicy=DEFAULT|GRACEFUL, as was proposed by me earlier.

 I'm ok with GRACEFUL instead of WAIT_FOR_BACKUPS.

5. Let's keep a static property for simplifying setting of initial
> behavior.
> In most cases the policy will never be changed during grid's lifetime.
> No need for an explicit call to API on grid start.
> A joining node should check a local configuration value to match the grid.
> If a dynamic value is already present in a metastore, it should override
> static value with a warning.

To sum it up:
- ShutdownPolicy can be set with static configuration
(IgniteConfiguration#setShutdownPolicy), on join we validate that
statically configured policies on different server nodes are the same
- It's possible to override statically configured value by adding
distributed metastorage value, which can be done by
calling ignite.cluster().setShutdownPolicy(plc) or control.sh method
- Dynamic property is persisted

Generally, I don't mind if we have both dynamic and static configuration
properties. Necessity to call ignite.cluster().setShutdownPolicy(plc); on
every new cluster creation is a usability issue itself.
What bothers me here are the possible conflicts between static and dynamic
configuration. User may be surprised if he has shutdown policy X in
IgniteConfiguration, but the cluster behaves according to policy Y (because
several months ago another admin had called
IgniteCluster#setShutdownPolicy).
We can handle it by adding a separate enum field to the shutdown policy:

> public enum ShutdownPolicy {
>   /* Default value of dynamic shutdown policy property. If it's set, the
> shutdown policy is resolved according to value of static {@link
> IgniteConfiguration#shutdownPolicy} configuration parameter. */
>   USE_STATIC_CONFIGURATION,
>
>   /* Node leaves the cluster even if it's the last owner of some
> partitions. Only partitions of caches with backups > 0 are taken into
> account. */
>   IMMEDIATE,
>
>   /* Shutdown is blocked until node is safe to leave without the data
> loss. */
>   GRACEFUL
> }
>
This way:
1) User may easily understand whether the static parameter is overridden by
dynamic. If ignite.cluster().getShutdownPolicy() return anything except
USE_STATIC_CONFIGURATION, behavior is overridden.
2) User may clear previous overriding by calling
ignite.cluster().setShutdownPolicy(USE_STATIC_CONFIGURATION). After that,
behavior will be resolved based in IgniteConfiguration#shutdownPolicy again.
If we agree on this mechanism, I propose to use IMMEDIATE name instead of
DEFAULT for non-safe policy in order to don't confuse user.
Meanwhile, static configuration will accept the same enum, but
USE_STATIC_CONFIGURATION will be restricted:

> public class IgniteConfiguration {
>   public static final ShutdownPolicy DFLT_STATIC_SHUTDOWN_POLICY =
> IMMEDIATE;
>   private ShutdownPolicy shutdownPolicy = DFLT_STATIC_SHUTDOWN_POLICY;
>   ...
>   public void setShutdownPolicy(ShutdownPolicy shutdownPlc) {
>     if (shutdownPlc ==  USE_STATIC_CONFIGURATION)
>       throw new IllegalArgumentException("USE_STATIC_CONFIGURATION can
> only be passed as dynamic property value via
> ignite.cluster().setShutdownPolicy");
>     ...
>   }
> ...
> }
>

What do you think?

On Tue, Jun 9, 2020 at 11:46 AM Alexei Scherbakov <
[hidden email]> wrote:

> Ivan Rakov,
>
> Your proposal overall looks good to me. My comments:
>
> 1. I would avoid adding such a method, because it will be impossible to
> change it in the future if some more shutdown policies will be introduced
> later.
> Also shutdown policy must be always consistent on the grid or unintentional
> data loss is possible if two nodes are stopping simultaneously with
> different policies.
>
> This behavior can be achieved by changing policy globally when stopping a
> node:
> ignite.cluster().setShutdownPolicy(DEFAULT);
> ignore.stop();
>
> 2. defaultShutdownPolicy with DEFAULT value is a mess. WAIT_FOR_BACKUPS is
> not very clear either.
> Let's use shutdownPolicy=DEFAULT|GRACEFUL, as was proposed by me earlier.
>
> 3. OK
>
> 4. OK
>
> 5. Let's keep a static property for simplifying setting of initial
> behavior.
> In most cases the policy will never be changed during grid's lifetime.
> No need for an explicit call to API on grid start.
> A joining node should check a local configuration value to match the grid.
> If a dynamic value is already present in a metastore, it should override
> static value with a warning.
>
>
>
>
> пн, 8 июн. 2020 г. в 19:06, Ivan Rakov <[hidden email]>:
>
> > Vlad, thanks for starting this discussion.
> >
> > I'll try to clarify the motivation for this change as I see it.
> > In general, Ignite clusters are vulnerable to the data loss. Of course,
> we
> > have configurable PartitionLossPolicy, which allows to handle data loss
> > safely and mitigate its consequences. But being able to avoid critical
> > situations is always better than being able to recover from it.
> >
> > The most common issue from my perspective is absence of a way to perform
> > rolling cluster restart safely. Scenario:
> > 1. Backup count is 1
> > 2. Admin wants to perform rolling restart in order to deploy new version
> of
> > business code that uses Ignite in embedded mode
> > 3. Admin shuts down first node, replaces needed binaries and returns the
> > node back to the topology
> > 4. Node joins the cluster successfully
> > 5. Admin shuts down second node
> > 6. Data loss happens: the second node was the only owner of a certain
> > partition, which was being rebalanced from the second node to the first
> >
> > We can prevent such situations by introducing "safe shutdown by default"
> > mode, which blocks stopping node while it remains the only owner for at
> > least one partition. It should be applied to "common" ways of stopping
> > nodes - Ignite.close() and kill <pid>.
> > I think, option to be enabled or disabled in runtime should be a
> > requirement for this behavior. Safe shutdown mode has weird side-effects.
> > For example, admin won't be able to stop the whole cluster: stop of last
> > node will be blocked, because the last node is the only present owner of
> > all its partitions. Sure, kill -9 will resolve it, but it's still a
> > usability issue.
> >
> > With the described dynamic property scenario will be changed as follows:
> > 1. Admin enables "safe shutdown" mode
> > 2. Admin shuts down first node, replaces needed binaries and returns the
> > node back to the topology
> > 3. Admin shuts down second node (with either ignite.close() or kill
> <pid>),
> > shutdown is blocked until the first node returns to the topology and
> > completes the rebalancing process
> > 4. Admin proceeds the rolling restart procedure
> > 5. Admin disables "safe shutdown" mode
> >
> > This logic will also simplify the rolling restart scenario in K8S. Pod
> with
> > Ignite node won't be terminated until its termination will cause data
> loss.
> >
> > Aside from waiting for backups, Ignition interface provide lots of
> options
> > to perform various node stop:
> > - Whether or not to cancel pending compute jobs
> > - Whether or not to perform instant halt() instead of any graceful stop
> > logic
> > - Whether or not to wait for some timeout before halt()
> > - Whether or not the stopped grid should be restarted
> > All these "stop" methods provide very custom logic. I don't see a need to
> > make them part of dynamic cluster-wide configuration. They still can be
> > invoked directly via Java API. Later we can extract some of them to
> dynamic
> > cluster-wide parameters of default stop if it will become necessary.
> That's
> > why I think we should create an enum for default shutdown policy, but
> only
> > with two options so far (we can add more later): DEFAULT and
> > WAIT_FOR_BACKUPS.
> > Regarding the "NORMAL" option that you propose (where the node is not
> > stopped until the rebalance is finished): I don't think that we should
> add
> > it. It doesn't ensure any strict guarantees: the data still can be lost
> > with it.
> >
> > To sum it up, I propose:
> > 1. Add a new method to Ignition interface to make it possible to stop
> with
> > "wait for backups" logic directly via Java API, like
> Ignition.stop(boolean
> > cancel, boolean waitForBackups)
> > 2. Introduce "defaultShutdownPolicy" as a dynamic cluster configuration,
> > two values are available so far: DEFAULT and WAIT_FOR_BACKUPS
> > 3. This property is stored in the distributed metastorage (thus
> persisted),
> > can be changed via Java API and ./control.sh
> > 4. Behavior configured with this property will be applied only on common
> > ways of stopping the node - Ignite.close() and kill <pid>.
> > 5. *Don't* add new options to the static IgniteConfiguration to avoid
> > conflicts between dynamic and static configuration
> >
> > --
> > Best Regards,
> > Ivan Rakov
> >
> > On Mon, Jun 8, 2020 at 6:44 PM V.Pyatkov <[hidden email]> wrote:
> >
> > > Hi
> > >
> > > We need to have ability to calling shutdown with various guaranties.
> > > For example:
> > > Need to reboot a node, but after that node should be available for
> > > historical rebalance (all partitions in MOVING state should have gone
> to
> > > OWNING).
> > >
> > > Implemented a circled reboot of cluster, but all data should be
> available
> > > on
> > > that time (at least one copy of partition should be available in
> > cluster).
> > >
> > > Need to wait not only data available, but all jobs (before this
> behavior
> > > available through a stop(false) method invocation).
> > >
> > > All these reason required various behavior before shutting down node.
> > > I propose slightly modify public API and add here method which shown on
> > > shutdown behavior directly:
> > > Ignite.close(Shutdown)
> > >
> > > /public enum Shutdownn {
> > >     /**
> > >      * Stop immediately as soon components are ready.
> > >      */
> > >     IMMEDIATE,
> > >     /**
> > >      * Stop node when all partitions completed moving from/to this node
> > to
> > > another.
> > >      */
> > >     NORMAL,
> > >     /**
> > >      * Node will stop if and only if it does not store any unique
> > > partitions, that does not have copies on cluster.
> > >      */
> > >     GRACEFUL,
> > >     /**
> > >      * Node stops graceful and wait all jobs before shutdown.
> > >      */
> > >     ALL
> > > }/
> > >
> > > Method close without parameter Ignite.close() will get shutdown
> behavior
> > > configured for cluster wide. It will be implemented through distributed
> > > meta
> > > storage and additional utilities for configuration.
> > > Also, will be added a method to configure shutdown on start, this is
> look
> > > as
> > > IgniteConfiguration.setShutdown(Shutdown).
> > > If shutting down did not configure all be worked as before according to
> > > IMMEDIATE behavior.
> > > All other close method will be marked as deprecated.
> > >
> > > I will be waiting for your opinions.
> > >
> > >
> > >
> > > --
> > > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> > >
> >
>
>
> --
>
> Best regards,
> Alexei Scherbakov
>
Reply | Threaded
Open this post in threaded view
|

Re: Various shutdown guaranties

Ivan Rakov
Something went wrong with gmail formatting. Resending my reply.

Alex,

Also shutdown policy must be always consistent on the grid or unintentional
> data loss is possible if two nodes are stopping simultaneously with
> different policies.

 Totally agree.

Let's use shutdownPolicy=DEFAULT|GRACEFUL, as was proposed by me earlier.

 I'm ok with GRACEFUL instead of WAIT_FOR_BACKUPS.

5. Let's keep a static property for simplifying setting of initial
> behavior.
> In most cases the policy will never be changed during grid's lifetime.
> No need for an explicit call to API on grid start.
> A joining node should check a local configuration value to match the grid.
> If a dynamic value is already present in a metastore, it should override
> static value with a warning.

To sum it up:
- ShutdownPolicy can be set with static configuration
(IgniteConfiguration#setShutdownPolicy), on join we validate that
statically configured policies on different server nodes are the same
- It's possible to override statically configured value by adding
distributed metastorage value, which can be done by
calling ignite.cluster().setShutdownPolicy(plc) or control.sh method
- Dynamic property is persisted

Generally, I don't mind if we have both dynamic and static configuration
properties. Necessity to call ignite.cluster().setShutdownPolicy(plc); on
every new cluster creation is a usability issue itself.
What bothers me here are the possible conflicts between static and dynamic
configuration. User may be surprised if he has shutdown policy X in
IgniteConfiguration, but the cluster behaves according to policy Y (because
several months ago another admin had called
IgniteCluster#setShutdownPolicy).
We can handle it by adding a separate enum field to the shutdown policy:

> public enum ShutdownPolicy {
>   /* Default value of dynamic shutdown policy property. If it's set, the
> shutdown policy is resolved according to value of static {@link
> IgniteConfiguration#shutdownPolicy} configuration parameter. */
>   USE_STATIC_CONFIGURATION,
>
>   /* Node leaves the cluster even if it's the last owner of some
> partitions. Only partitions of caches with backups > 0 are taken into
> account. */
>   IMMEDIATE,
>
>   /* Shutdown is blocked until node is safe to leave without the data
> loss. */
>   GRACEFUL
> }
>
This way:
1) User may easily understand whether the static parameter is overridden by
dynamic. If ignite.cluster().getShutdownPolicy() return anything except
USE_STATIC_CONFIGURATION, behavior is overridden.
2) User may clear previous overriding by calling
ignite.cluster().setShutdownPolicy(USE_STATIC_CONFIGURATION). After that,
behavior will be resolved based in IgniteConfiguration#shutdownPolicy again.
If we agree on this mechanism, I propose to use IMMEDIATE name instead of
DEFAULT for non-safe policy in order to don't confuse user.
Meanwhile, static configuration will accept the same enum, but
USE_STATIC_CONFIGURATION will be restricted:

> public class IgniteConfiguration {
>   public static final ShutdownPolicy DFLT_STATIC_SHUTDOWN_POLICY =
> IMMEDIATE;
>   private ShutdownPolicy shutdownPolicy = DFLT_STATIC_SHUTDOWN_POLICY;
>   ...
>   public void setShutdownPolicy(ShutdownPolicy shutdownPlc) {
>     if (shutdownPlc ==  USE_STATIC_CONFIGURATION)
>       throw new IllegalArgumentException("USE_STATIC_CONFIGURATION can
> only be passed as dynamic property value via
> ignite.cluster().setShutdownPolicy");
>     ...
>   }
> ...
> }
>

What do you think?

On Tue, Jun 9, 2020 at 3:09 PM Ivan Rakov <[hidden email]> wrote:

> Alex,
>
> Also shutdown policy must be always consistent on the grid or unintentional
>> data loss is possible if two nodes are stopping simultaneously with
>> different policies.
>
>  Totally agree.
>
> Let's use shutdownPolicy=DEFAULT|GRACEFUL, as was proposed by me earlier.
>
>  I'm ok with GRACEFUL instead of WAIT_FOR_BACKUPS.
>
> 5. Let's keep a static property for simplifying setting of initial
>> behavior.
>> In most cases the policy will never be changed during grid's lifetime.
>> No need for an explicit call to API on grid start.
>> A joining node should check a local configuration value to match the grid.
>> If a dynamic value is already present in a metastore, it should override
>> static value with a warning.
>
> To sum it up:
> - ShutdownPolicy can be set with static configuration
> (IgniteConfiguration#setShutdownPolicy), on join we validate that
> statically configured policies on different server nodes are the same
> - It's possible to override statically configured value by adding
> distributed metastorage value, which can be done by
> calling ignite.cluster().setShutdownPolicy(plc) or control.sh method
> - Dynamic property is persisted
>
> Generally, I don't mind if we have both dynamic and static configuration
> properties. Necessity to call ignite.cluster().setShutdownPolicy(plc); on
> every new cluster creation is a usability issue itself.
> What bothers me here are the possible conflicts between static and dynamic
> configuration. User may be surprised if he has shutdown policy X in
> IgniteConfiguration, but the cluster behaves according to policy Y (because
> several months ago another admin had called
> IgniteCluster#setShutdownPolicy).
> We can handle it by adding a separate enum field to the shutdown policy:
>
>> public enum ShutdownPolicy {
>>   /* Default value of dynamic shutdown policy property. If it's set, the
>> shutdown policy is resolved according to value of static {@link
>> IgniteConfiguration#shutdownPolicy} configuration parameter. */
>>   USE_STATIC_CONFIGURATION,
>>
>>   /* Node leaves the cluster even if it's the last owner of some
>> partitions. Only partitions of caches with backups > 0 are taken into
>> account. */
>>   IMMEDIATE,
>>
>>   /* Shutdown is blocked until node is safe to leave without the data
>> loss. */
>>   GRACEFUL
>> }
>>
> This way:
> 1) User may easily understand whether the static parameter is overridden
> by dynamic. If ignite.cluster().getShutdownPolicy() return anything except
> USE_STATIC_CONFIGURATION, behavior is overridden.
> 2) User may clear previous overriding by calling
> ignite.cluster().setShutdownPolicy(USE_STATIC_CONFIGURATION). After that,
> behavior will be resolved based in IgniteConfiguration#shutdownPolicy again.
> If we agree on this mechanism, I propose to use IMMEDIATE name instead of
> DEFAULT for non-safe policy in order to don't confuse user.
> Meanwhile, static configuration will accept the same enum, but
> USE_STATIC_CONFIGURATION will be restricted:
>
>> public class IgniteConfiguration {
>>   public static final ShutdownPolicy DFLT_STATIC_SHUTDOWN_POLICY =
>> IMMEDIATE;
>>   private ShutdownPolicy shutdownPolicy = DFLT_STATIC_SHUTDOWN_POLICY;
>>   ...
>>   public void setShutdownPolicy(ShutdownPolicy shutdownPlc) {
>>     if (shutdownPlc ==  USE_STATIC_CONFIGURATION)
>>       throw new IllegalArgumentException("USE_STATIC_CONFIGURATION can
>> only be passed as dynamic property value via
>> ignite.cluster().setShutdownPolicy");
>>     ...
>>   }
>> ...
>> }
>>
>
> What do you think?
>
> On Tue, Jun 9, 2020 at 11:46 AM Alexei Scherbakov <
> [hidden email]> wrote:
>
>> Ivan Rakov,
>>
>> Your proposal overall looks good to me. My comments:
>>
>> 1. I would avoid adding such a method, because it will be impossible to
>> change it in the future if some more shutdown policies will be introduced
>> later.
>> Also shutdown policy must be always consistent on the grid or
>> unintentional
>> data loss is possible if two nodes are stopping simultaneously with
>> different policies.
>>
>> This behavior can be achieved by changing policy globally when stopping a
>> node:
>> ignite.cluster().setShutdownPolicy(DEFAULT);
>> ignore.stop();
>>
>> 2. defaultShutdownPolicy with DEFAULT value is a mess. WAIT_FOR_BACKUPS is
>> not very clear either.
>> Let's use shutdownPolicy=DEFAULT|GRACEFUL, as was proposed by me earlier.
>>
>> 3. OK
>>
>> 4. OK
>>
>> 5. Let's keep a static property for simplifying setting of initial
>> behavior.
>> In most cases the policy will never be changed during grid's lifetime.
>> No need for an explicit call to API on grid start.
>> A joining node should check a local configuration value to match the grid.
>> If a dynamic value is already present in a metastore, it should override
>> static value with a warning.
>>
>>
>>
>>
>> пн, 8 июн. 2020 г. в 19:06, Ivan Rakov <[hidden email]>:
>>
>> > Vlad, thanks for starting this discussion.
>> >
>> > I'll try to clarify the motivation for this change as I see it.
>> > In general, Ignite clusters are vulnerable to the data loss. Of course,
>> we
>> > have configurable PartitionLossPolicy, which allows to handle data loss
>> > safely and mitigate its consequences. But being able to avoid critical
>> > situations is always better than being able to recover from it.
>> >
>> > The most common issue from my perspective is absence of a way to perform
>> > rolling cluster restart safely. Scenario:
>> > 1. Backup count is 1
>> > 2. Admin wants to perform rolling restart in order to deploy new
>> version of
>> > business code that uses Ignite in embedded mode
>> > 3. Admin shuts down first node, replaces needed binaries and returns the
>> > node back to the topology
>> > 4. Node joins the cluster successfully
>> > 5. Admin shuts down second node
>> > 6. Data loss happens: the second node was the only owner of a certain
>> > partition, which was being rebalanced from the second node to the first
>> >
>> > We can prevent such situations by introducing "safe shutdown by default"
>> > mode, which blocks stopping node while it remains the only owner for at
>> > least one partition. It should be applied to "common" ways of stopping
>> > nodes - Ignite.close() and kill <pid>.
>> > I think, option to be enabled or disabled in runtime should be a
>> > requirement for this behavior. Safe shutdown mode has weird
>> side-effects.
>> > For example, admin won't be able to stop the whole cluster: stop of last
>> > node will be blocked, because the last node is the only present owner of
>> > all its partitions. Sure, kill -9 will resolve it, but it's still a
>> > usability issue.
>> >
>> > With the described dynamic property scenario will be changed as follows:
>> > 1. Admin enables "safe shutdown" mode
>> > 2. Admin shuts down first node, replaces needed binaries and returns the
>> > node back to the topology
>> > 3. Admin shuts down second node (with either ignite.close() or kill
>> <pid>),
>> > shutdown is blocked until the first node returns to the topology and
>> > completes the rebalancing process
>> > 4. Admin proceeds the rolling restart procedure
>> > 5. Admin disables "safe shutdown" mode
>> >
>> > This logic will also simplify the rolling restart scenario in K8S. Pod
>> with
>> > Ignite node won't be terminated until its termination will cause data
>> loss.
>> >
>> > Aside from waiting for backups, Ignition interface provide lots of
>> options
>> > to perform various node stop:
>> > - Whether or not to cancel pending compute jobs
>> > - Whether or not to perform instant halt() instead of any graceful stop
>> > logic
>> > - Whether or not to wait for some timeout before halt()
>> > - Whether or not the stopped grid should be restarted
>> > All these "stop" methods provide very custom logic. I don't see a need
>> to
>> > make them part of dynamic cluster-wide configuration. They still can be
>> > invoked directly via Java API. Later we can extract some of them to
>> dynamic
>> > cluster-wide parameters of default stop if it will become necessary.
>> That's
>> > why I think we should create an enum for default shutdown policy, but
>> only
>> > with two options so far (we can add more later): DEFAULT and
>> > WAIT_FOR_BACKUPS.
>> > Regarding the "NORMAL" option that you propose (where the node is not
>> > stopped until the rebalance is finished): I don't think that we should
>> add
>> > it. It doesn't ensure any strict guarantees: the data still can be lost
>> > with it.
>> >
>> > To sum it up, I propose:
>> > 1. Add a new method to Ignition interface to make it possible to stop
>> with
>> > "wait for backups" logic directly via Java API, like
>> Ignition.stop(boolean
>> > cancel, boolean waitForBackups)
>> > 2. Introduce "defaultShutdownPolicy" as a dynamic cluster configuration,
>> > two values are available so far: DEFAULT and WAIT_FOR_BACKUPS
>> > 3. This property is stored in the distributed metastorage (thus
>> persisted),
>> > can be changed via Java API and ./control.sh
>> > 4. Behavior configured with this property will be applied only on common
>> > ways of stopping the node - Ignite.close() and kill <pid>.
>> > 5. *Don't* add new options to the static IgniteConfiguration to avoid
>> > conflicts between dynamic and static configuration
>> >
>> > --
>> > Best Regards,
>> > Ivan Rakov
>> >
>> > On Mon, Jun 8, 2020 at 6:44 PM V.Pyatkov <[hidden email]> wrote:
>> >
>> > > Hi
>> > >
>> > > We need to have ability to calling shutdown with various guaranties.
>> > > For example:
>> > > Need to reboot a node, but after that node should be available for
>> > > historical rebalance (all partitions in MOVING state should have gone
>> to
>> > > OWNING).
>> > >
>> > > Implemented a circled reboot of cluster, but all data should be
>> available
>> > > on
>> > > that time (at least one copy of partition should be available in
>> > cluster).
>> > >
>> > > Need to wait not only data available, but all jobs (before this
>> behavior
>> > > available through a stop(false) method invocation).
>> > >
>> > > All these reason required various behavior before shutting down node.
>> > > I propose slightly modify public API and add here method which shown
>> on
>> > > shutdown behavior directly:
>> > > Ignite.close(Shutdown)
>> > >
>> > > /public enum Shutdownn {
>> > >     /**
>> > >      * Stop immediately as soon components are ready.
>> > >      */
>> > >     IMMEDIATE,
>> > >     /**
>> > >      * Stop node when all partitions completed moving from/to this
>> node
>> > to
>> > > another.
>> > >      */
>> > >     NORMAL,
>> > >     /**
>> > >      * Node will stop if and only if it does not store any unique
>> > > partitions, that does not have copies on cluster.
>> > >      */
>> > >     GRACEFUL,
>> > >     /**
>> > >      * Node stops graceful and wait all jobs before shutdown.
>> > >      */
>> > >     ALL
>> > > }/
>> > >
>> > > Method close without parameter Ignite.close() will get shutdown
>> behavior
>> > > configured for cluster wide. It will be implemented through
>> distributed
>> > > meta
>> > > storage and additional utilities for configuration.
>> > > Also, will be added a method to configure shutdown on start, this is
>> look
>> > > as
>> > > IgniteConfiguration.setShutdown(Shutdown).
>> > > If shutting down did not configure all be worked as before according
>> to
>> > > IMMEDIATE behavior.
>> > > All other close method will be marked as deprecated.
>> > >
>> > > I will be waiting for your opinions.
>> > >
>> > >
>> > >
>> > > --
>> > > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
>> > >
>> >
>>
>>
>> --
>>
>> Best regards,
>> Alexei Scherbakov
>>
>
Reply | Threaded
Open this post in threaded view
|

Re: Various shutdown guaranties

Alexei Scherbakov
Ivan,

Using an additional enum on public API for resetting dynamic value looks a
little bit dirty for me.
I'm not sure there is a problem at all, because user can always query the
current policy, and a javadoc can describe such behavior clearly.
If you really insist maybe use null to reset policy value:

ignite.cluster().setShutdownPolicy(null); // Clear dynamic value and switch
to statically configured.

On top of this, we already have a bunch of over properties, which are set
statically and can be changed dynamically later,  for example [1]
I think all such properties should behave the same way as shutdown policy
and we need a ticket for this.
In such a case we probably should go with something like

ignite.cluster().resetDynamicProperValuey(propName); // Resets a property
to statically configured default value.

Right now I would prefer for shutdown policy behave as other dynamic
properties to make things consistent and fix them all later to be
resettable to static configuration value.

[1] org.apache.ignite.IgniteCluster#setTxTimeoutOnPartitionMapExchange(timeout)



вт, 9 июн. 2020 г. в 15:12, Ivan Rakov <[hidden email]>:

> Something went wrong with gmail formatting. Resending my reply.
>
> Alex,
>
> Also shutdown policy must be always consistent on the grid or unintentional
> > data loss is possible if two nodes are stopping simultaneously with
> > different policies.
>
>  Totally agree.
>
> Let's use shutdownPolicy=DEFAULT|GRACEFUL, as was proposed by me earlier.
>
>  I'm ok with GRACEFUL instead of WAIT_FOR_BACKUPS.
>
> 5. Let's keep a static property for simplifying setting of initial
> > behavior.
> > In most cases the policy will never be changed during grid's lifetime.
> > No need for an explicit call to API on grid start.
> > A joining node should check a local configuration value to match the
> grid.
> > If a dynamic value is already present in a metastore, it should override
> > static value with a warning.
>
> To sum it up:
> - ShutdownPolicy can be set with static configuration
> (IgniteConfiguration#setShutdownPolicy), on join we validate that
> statically configured policies on different server nodes are the same
> - It's possible to override statically configured value by adding
> distributed metastorage value, which can be done by
> calling ignite.cluster().setShutdownPolicy(plc) or control.sh method
> - Dynamic property is persisted
>
> Generally, I don't mind if we have both dynamic and static configuration
> properties. Necessity to call ignite.cluster().setShutdownPolicy(plc); on
> every new cluster creation is a usability issue itself.
> What bothers me here are the possible conflicts between static and dynamic
> configuration. User may be surprised if he has shutdown policy X in
> IgniteConfiguration, but the cluster behaves according to policy Y (because
> several months ago another admin had called
> IgniteCluster#setShutdownPolicy).
> We can handle it by adding a separate enum field to the shutdown policy:
>
> > public enum ShutdownPolicy {
> >   /* Default value of dynamic shutdown policy property. If it's set, the
> > shutdown policy is resolved according to value of static {@link
> > IgniteConfiguration#shutdownPolicy} configuration parameter. */
> >   USE_STATIC_CONFIGURATION,
> >
> >   /* Node leaves the cluster even if it's the last owner of some
> > partitions. Only partitions of caches with backups > 0 are taken into
> > account. */
> >   IMMEDIATE,
> >
> >   /* Shutdown is blocked until node is safe to leave without the data
> > loss. */
> >   GRACEFUL
> > }
> >
> This way:
> 1) User may easily understand whether the static parameter is overridden by
> dynamic. If ignite.cluster().getShutdownPolicy() return anything except
> USE_STATIC_CONFIGURATION, behavior is overridden.
> 2) User may clear previous overriding by calling
> ignite.cluster().setShutdownPolicy(USE_STATIC_CONFIGURATION). After that,
> behavior will be resolved based in IgniteConfiguration#shutdownPolicy
> again.
> If we agree on this mechanism, I propose to use IMMEDIATE name instead of
> DEFAULT for non-safe policy in order to don't confuse user.
> Meanwhile, static configuration will accept the same enum, but
> USE_STATIC_CONFIGURATION will be restricted:
>
> > public class IgniteConfiguration {
> >   public static final ShutdownPolicy DFLT_STATIC_SHUTDOWN_POLICY =
> > IMMEDIATE;
> >   private ShutdownPolicy shutdownPolicy = DFLT_STATIC_SHUTDOWN_POLICY;
> >   ...
> >   public void setShutdownPolicy(ShutdownPolicy shutdownPlc) {
> >     if (shutdownPlc ==  USE_STATIC_CONFIGURATION)
> >       throw new IllegalArgumentException("USE_STATIC_CONFIGURATION can
> > only be passed as dynamic property value via
> > ignite.cluster().setShutdownPolicy");
> >     ...
> >   }
> > ...
> > }
> >
>
> What do you think?
>
> On Tue, Jun 9, 2020 at 3:09 PM Ivan Rakov <[hidden email]> wrote:
>
> > Alex,
> >
> > Also shutdown policy must be always consistent on the grid or
> unintentional
> >> data loss is possible if two nodes are stopping simultaneously with
> >> different policies.
> >
> >  Totally agree.
> >
> > Let's use shutdownPolicy=DEFAULT|GRACEFUL, as was proposed by me earlier.
> >
> >  I'm ok with GRACEFUL instead of WAIT_FOR_BACKUPS.
> >
> > 5. Let's keep a static property for simplifying setting of initial
> >> behavior.
> >> In most cases the policy will never be changed during grid's lifetime.
> >> No need for an explicit call to API on grid start.
> >> A joining node should check a local configuration value to match the
> grid.
> >> If a dynamic value is already present in a metastore, it should override
> >> static value with a warning.
> >
> > To sum it up:
> > - ShutdownPolicy can be set with static configuration
> > (IgniteConfiguration#setShutdownPolicy), on join we validate that
> > statically configured policies on different server nodes are the same
> > - It's possible to override statically configured value by adding
> > distributed metastorage value, which can be done by
> > calling ignite.cluster().setShutdownPolicy(plc) or control.sh method
> > - Dynamic property is persisted
> >
> > Generally, I don't mind if we have both dynamic and static configuration
> > properties. Necessity to call ignite.cluster().setShutdownPolicy(plc); on
> > every new cluster creation is a usability issue itself.
> > What bothers me here are the possible conflicts between static and
> dynamic
> > configuration. User may be surprised if he has shutdown policy X in
> > IgniteConfiguration, but the cluster behaves according to policy Y
> (because
> > several months ago another admin had called
> > IgniteCluster#setShutdownPolicy).
> > We can handle it by adding a separate enum field to the shutdown policy:
> >
> >> public enum ShutdownPolicy {
> >>   /* Default value of dynamic shutdown policy property. If it's set, the
> >> shutdown policy is resolved according to value of static {@link
> >> IgniteConfiguration#shutdownPolicy} configuration parameter. */
> >>   USE_STATIC_CONFIGURATION,
> >>
> >>   /* Node leaves the cluster even if it's the last owner of some
> >> partitions. Only partitions of caches with backups > 0 are taken into
> >> account. */
> >>   IMMEDIATE,
> >>
> >>   /* Shutdown is blocked until node is safe to leave without the data
> >> loss. */
> >>   GRACEFUL
> >> }
> >>
> > This way:
> > 1) User may easily understand whether the static parameter is overridden
> > by dynamic. If ignite.cluster().getShutdownPolicy() return anything
> except
> > USE_STATIC_CONFIGURATION, behavior is overridden.
> > 2) User may clear previous overriding by calling
> > ignite.cluster().setShutdownPolicy(USE_STATIC_CONFIGURATION). After that,
> > behavior will be resolved based in IgniteConfiguration#shutdownPolicy
> again.
> > If we agree on this mechanism, I propose to use IMMEDIATE name instead of
> > DEFAULT for non-safe policy in order to don't confuse user.
> > Meanwhile, static configuration will accept the same enum, but
> > USE_STATIC_CONFIGURATION will be restricted:
> >
> >> public class IgniteConfiguration {
> >>   public static final ShutdownPolicy DFLT_STATIC_SHUTDOWN_POLICY =
> >> IMMEDIATE;
> >>   private ShutdownPolicy shutdownPolicy = DFLT_STATIC_SHUTDOWN_POLICY;
> >>   ...
> >>   public void setShutdownPolicy(ShutdownPolicy shutdownPlc) {
> >>     if (shutdownPlc ==  USE_STATIC_CONFIGURATION)
> >>       throw new IllegalArgumentException("USE_STATIC_CONFIGURATION can
> >> only be passed as dynamic property value via
> >> ignite.cluster().setShutdownPolicy");
> >>     ...
> >>   }
> >> ...
> >> }
> >>
> >
> > What do you think?
> >
> > On Tue, Jun 9, 2020 at 11:46 AM Alexei Scherbakov <
> > [hidden email]> wrote:
> >
> >> Ivan Rakov,
> >>
> >> Your proposal overall looks good to me. My comments:
> >>
> >> 1. I would avoid adding such a method, because it will be impossible to
> >> change it in the future if some more shutdown policies will be
> introduced
> >> later.
> >> Also shutdown policy must be always consistent on the grid or
> >> unintentional
> >> data loss is possible if two nodes are stopping simultaneously with
> >> different policies.
> >>
> >> This behavior can be achieved by changing policy globally when stopping
> a
> >> node:
> >> ignite.cluster().setShutdownPolicy(DEFAULT);
> >> ignore.stop();
> >>
> >> 2. defaultShutdownPolicy with DEFAULT value is a mess. WAIT_FOR_BACKUPS
> is
> >> not very clear either.
> >> Let's use shutdownPolicy=DEFAULT|GRACEFUL, as was proposed by me
> earlier.
> >>
> >> 3. OK
> >>
> >> 4. OK
> >>
> >> 5. Let's keep a static property for simplifying setting of initial
> >> behavior.
> >> In most cases the policy will never be changed during grid's lifetime.
> >> No need for an explicit call to API on grid start.
> >> A joining node should check a local configuration value to match the
> grid.
> >> If a dynamic value is already present in a metastore, it should override
> >> static value with a warning.
> >>
> >>
> >>
> >>
> >> пн, 8 июн. 2020 г. в 19:06, Ivan Rakov <[hidden email]>:
> >>
> >> > Vlad, thanks for starting this discussion.
> >> >
> >> > I'll try to clarify the motivation for this change as I see it.
> >> > In general, Ignite clusters are vulnerable to the data loss. Of
> course,
> >> we
> >> > have configurable PartitionLossPolicy, which allows to handle data
> loss
> >> > safely and mitigate its consequences. But being able to avoid critical
> >> > situations is always better than being able to recover from it.
> >> >
> >> > The most common issue from my perspective is absence of a way to
> perform
> >> > rolling cluster restart safely. Scenario:
> >> > 1. Backup count is 1
> >> > 2. Admin wants to perform rolling restart in order to deploy new
> >> version of
> >> > business code that uses Ignite in embedded mode
> >> > 3. Admin shuts down first node, replaces needed binaries and returns
> the
> >> > node back to the topology
> >> > 4. Node joins the cluster successfully
> >> > 5. Admin shuts down second node
> >> > 6. Data loss happens: the second node was the only owner of a certain
> >> > partition, which was being rebalanced from the second node to the
> first
> >> >
> >> > We can prevent such situations by introducing "safe shutdown by
> default"
> >> > mode, which blocks stopping node while it remains the only owner for
> at
> >> > least one partition. It should be applied to "common" ways of stopping
> >> > nodes - Ignite.close() and kill <pid>.
> >> > I think, option to be enabled or disabled in runtime should be a
> >> > requirement for this behavior. Safe shutdown mode has weird
> >> side-effects.
> >> > For example, admin won't be able to stop the whole cluster: stop of
> last
> >> > node will be blocked, because the last node is the only present owner
> of
> >> > all its partitions. Sure, kill -9 will resolve it, but it's still a
> >> > usability issue.
> >> >
> >> > With the described dynamic property scenario will be changed as
> follows:
> >> > 1. Admin enables "safe shutdown" mode
> >> > 2. Admin shuts down first node, replaces needed binaries and returns
> the
> >> > node back to the topology
> >> > 3. Admin shuts down second node (with either ignite.close() or kill
> >> <pid>),
> >> > shutdown is blocked until the first node returns to the topology and
> >> > completes the rebalancing process
> >> > 4. Admin proceeds the rolling restart procedure
> >> > 5. Admin disables "safe shutdown" mode
> >> >
> >> > This logic will also simplify the rolling restart scenario in K8S. Pod
> >> with
> >> > Ignite node won't be terminated until its termination will cause data
> >> loss.
> >> >
> >> > Aside from waiting for backups, Ignition interface provide lots of
> >> options
> >> > to perform various node stop:
> >> > - Whether or not to cancel pending compute jobs
> >> > - Whether or not to perform instant halt() instead of any graceful
> stop
> >> > logic
> >> > - Whether or not to wait for some timeout before halt()
> >> > - Whether or not the stopped grid should be restarted
> >> > All these "stop" methods provide very custom logic. I don't see a need
> >> to
> >> > make them part of dynamic cluster-wide configuration. They still can
> be
> >> > invoked directly via Java API. Later we can extract some of them to
> >> dynamic
> >> > cluster-wide parameters of default stop if it will become necessary.
> >> That's
> >> > why I think we should create an enum for default shutdown policy, but
> >> only
> >> > with two options so far (we can add more later): DEFAULT and
> >> > WAIT_FOR_BACKUPS.
> >> > Regarding the "NORMAL" option that you propose (where the node is not
> >> > stopped until the rebalance is finished): I don't think that we should
> >> add
> >> > it. It doesn't ensure any strict guarantees: the data still can be
> lost
> >> > with it.
> >> >
> >> > To sum it up, I propose:
> >> > 1. Add a new method to Ignition interface to make it possible to stop
> >> with
> >> > "wait for backups" logic directly via Java API, like
> >> Ignition.stop(boolean
> >> > cancel, boolean waitForBackups)
> >> > 2. Introduce "defaultShutdownPolicy" as a dynamic cluster
> configuration,
> >> > two values are available so far: DEFAULT and WAIT_FOR_BACKUPS
> >> > 3. This property is stored in the distributed metastorage (thus
> >> persisted),
> >> > can be changed via Java API and ./control.sh
> >> > 4. Behavior configured with this property will be applied only on
> common
> >> > ways of stopping the node - Ignite.close() and kill <pid>.
> >> > 5. *Don't* add new options to the static IgniteConfiguration to avoid
> >> > conflicts between dynamic and static configuration
> >> >
> >> > --
> >> > Best Regards,
> >> > Ivan Rakov
> >> >
> >> > On Mon, Jun 8, 2020 at 6:44 PM V.Pyatkov <[hidden email]>
> wrote:
> >> >
> >> > > Hi
> >> > >
> >> > > We need to have ability to calling shutdown with various guaranties.
> >> > > For example:
> >> > > Need to reboot a node, but after that node should be available for
> >> > > historical rebalance (all partitions in MOVING state should have
> gone
> >> to
> >> > > OWNING).
> >> > >
> >> > > Implemented a circled reboot of cluster, but all data should be
> >> available
> >> > > on
> >> > > that time (at least one copy of partition should be available in
> >> > cluster).
> >> > >
> >> > > Need to wait not only data available, but all jobs (before this
> >> behavior
> >> > > available through a stop(false) method invocation).
> >> > >
> >> > > All these reason required various behavior before shutting down
> node.
> >> > > I propose slightly modify public API and add here method which shown
> >> on
> >> > > shutdown behavior directly:
> >> > > Ignite.close(Shutdown)
> >> > >
> >> > > /public enum Shutdownn {
> >> > >     /**
> >> > >      * Stop immediately as soon components are ready.
> >> > >      */
> >> > >     IMMEDIATE,
> >> > >     /**
> >> > >      * Stop node when all partitions completed moving from/to this
> >> node
> >> > to
> >> > > another.
> >> > >      */
> >> > >     NORMAL,
> >> > >     /**
> >> > >      * Node will stop if and only if it does not store any unique
> >> > > partitions, that does not have copies on cluster.
> >> > >      */
> >> > >     GRACEFUL,
> >> > >     /**
> >> > >      * Node stops graceful and wait all jobs before shutdown.
> >> > >      */
> >> > >     ALL
> >> > > }/
> >> > >
> >> > > Method close without parameter Ignite.close() will get shutdown
> >> behavior
> >> > > configured for cluster wide. It will be implemented through
> >> distributed
> >> > > meta
> >> > > storage and additional utilities for configuration.
> >> > > Also, will be added a method to configure shutdown on start, this is
> >> look
> >> > > as
> >> > > IgniteConfiguration.setShutdown(Shutdown).
> >> > > If shutting down did not configure all be worked as before according
> >> to
> >> > > IMMEDIATE behavior.
> >> > > All other close method will be marked as deprecated.
> >> > >
> >> > > I will be waiting for your opinions.
> >> > >
> >> > >
> >> > >
> >> > > --
> >> > > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> >> > >
> >> >
> >>
> >>
> >> --
> >>
> >> Best regards,
> >> Alexei Scherbakov
> >>
> >
>


--

Best regards,
Alexei Scherbakov
Reply | Threaded
Open this post in threaded view
|

Re: Various shutdown guaranties

Ivan Rakov
Alex,

I'm not sure there is a problem at all, because user can always query the
> current policy, and a javadoc can describe such behavior clearly.

What will the query method return if the static policy is not overridden?
If we decide to avoid adding dedicated USE_STATIC_CONFIGURATION value,
semantics can be as follows:

> // Returns shutdown policy that is currently used by the cluster
> // If ignite.cluster().setShutdownPolicy() was never called, returns value
> from static configuration {@link IgniteConfiguration#shutdownPolicy}, which
> is consistent across all server nodes
> // If shutdown policy was overridden by user via
> ignite.cluster().setShutdownPolicy(), returns corresponding value

ignite.cluster().getShutdownPolicy();
>

Seems like there will be no need to reset distributed meta storage value.
User can always check which policy is used right now (regardless of whether
it has been overridden) and just set the policy that he needs if he wants
to change it.
The behavior is simple, the only magic is mapping <null> value in
distributed meta storage to value from IgniteConfiguration#shutdownPolicy.
Can we agree on this?

On Tue, Jun 9, 2020 at 3:48 PM Alexei Scherbakov <
[hidden email]> wrote:

> Ivan,
>
> Using an additional enum on public API for resetting dynamic value looks a
> little bit dirty for me.
> I'm not sure there is a problem at all, because user can always query the
> current policy, and a javadoc can describe such behavior clearly.
> If you really insist maybe use null to reset policy value:
>
> ignite.cluster().setShutdownPolicy(null); // Clear dynamic value and switch
> to statically configured.
>
> On top of this, we already have a bunch of over properties, which are set
> statically and can be changed dynamically later,  for example [1]
> I think all such properties should behave the same way as shutdown policy
> and we need a ticket for this.
> In such a case we probably should go with something like
>
> ignite.cluster().resetDynamicProperValuey(propName); // Resets a property
> to statically configured default value.
>
> Right now I would prefer for shutdown policy behave as other dynamic
> properties to make things consistent and fix them all later to be
> resettable to static configuration value.
>
> [1]
> org.apache.ignite.IgniteCluster#setTxTimeoutOnPartitionMapExchange(timeout)
>
>
>
> вт, 9 июн. 2020 г. в 15:12, Ivan Rakov <[hidden email]>:
>
> > Something went wrong with gmail formatting. Resending my reply.
> >
> > Alex,
> >
> > Also shutdown policy must be always consistent on the grid or
> unintentional
> > > data loss is possible if two nodes are stopping simultaneously with
> > > different policies.
> >
> >  Totally agree.
> >
> > Let's use shutdownPolicy=DEFAULT|GRACEFUL, as was proposed by me earlier.
> >
> >  I'm ok with GRACEFUL instead of WAIT_FOR_BACKUPS.
> >
> > 5. Let's keep a static property for simplifying setting of initial
> > > behavior.
> > > In most cases the policy will never be changed during grid's lifetime.
> > > No need for an explicit call to API on grid start.
> > > A joining node should check a local configuration value to match the
> > grid.
> > > If a dynamic value is already present in a metastore, it should
> override
> > > static value with a warning.
> >
> > To sum it up:
> > - ShutdownPolicy can be set with static configuration
> > (IgniteConfiguration#setShutdownPolicy), on join we validate that
> > statically configured policies on different server nodes are the same
> > - It's possible to override statically configured value by adding
> > distributed metastorage value, which can be done by
> > calling ignite.cluster().setShutdownPolicy(plc) or control.sh method
> > - Dynamic property is persisted
> >
> > Generally, I don't mind if we have both dynamic and static configuration
> > properties. Necessity to call ignite.cluster().setShutdownPolicy(plc); on
> > every new cluster creation is a usability issue itself.
> > What bothers me here are the possible conflicts between static and
> dynamic
> > configuration. User may be surprised if he has shutdown policy X in
> > IgniteConfiguration, but the cluster behaves according to policy Y
> (because
> > several months ago another admin had called
> > IgniteCluster#setShutdownPolicy).
> > We can handle it by adding a separate enum field to the shutdown policy:
> >
> > > public enum ShutdownPolicy {
> > >   /* Default value of dynamic shutdown policy property. If it's set,
> the
> > > shutdown policy is resolved according to value of static {@link
> > > IgniteConfiguration#shutdownPolicy} configuration parameter. */
> > >   USE_STATIC_CONFIGURATION,
> > >
> > >   /* Node leaves the cluster even if it's the last owner of some
> > > partitions. Only partitions of caches with backups > 0 are taken into
> > > account. */
> > >   IMMEDIATE,
> > >
> > >   /* Shutdown is blocked until node is safe to leave without the data
> > > loss. */
> > >   GRACEFUL
> > > }
> > >
> > This way:
> > 1) User may easily understand whether the static parameter is overridden
> by
> > dynamic. If ignite.cluster().getShutdownPolicy() return anything except
> > USE_STATIC_CONFIGURATION, behavior is overridden.
> > 2) User may clear previous overriding by calling
> > ignite.cluster().setShutdownPolicy(USE_STATIC_CONFIGURATION). After that,
> > behavior will be resolved based in IgniteConfiguration#shutdownPolicy
> > again.
> > If we agree on this mechanism, I propose to use IMMEDIATE name instead of
> > DEFAULT for non-safe policy in order to don't confuse user.
> > Meanwhile, static configuration will accept the same enum, but
> > USE_STATIC_CONFIGURATION will be restricted:
> >
> > > public class IgniteConfiguration {
> > >   public static final ShutdownPolicy DFLT_STATIC_SHUTDOWN_POLICY =
> > > IMMEDIATE;
> > >   private ShutdownPolicy shutdownPolicy = DFLT_STATIC_SHUTDOWN_POLICY;
> > >   ...
> > >   public void setShutdownPolicy(ShutdownPolicy shutdownPlc) {
> > >     if (shutdownPlc ==  USE_STATIC_CONFIGURATION)
> > >       throw new IllegalArgumentException("USE_STATIC_CONFIGURATION can
> > > only be passed as dynamic property value via
> > > ignite.cluster().setShutdownPolicy");
> > >     ...
> > >   }
> > > ...
> > > }
> > >
> >
> > What do you think?
> >
> > On Tue, Jun 9, 2020 at 3:09 PM Ivan Rakov <[hidden email]> wrote:
> >
> > > Alex,
> > >
> > > Also shutdown policy must be always consistent on the grid or
> > unintentional
> > >> data loss is possible if two nodes are stopping simultaneously with
> > >> different policies.
> > >
> > >  Totally agree.
> > >
> > > Let's use shutdownPolicy=DEFAULT|GRACEFUL, as was proposed by me
> earlier.
> > >
> > >  I'm ok with GRACEFUL instead of WAIT_FOR_BACKUPS.
> > >
> > > 5. Let's keep a static property for simplifying setting of initial
> > >> behavior.
> > >> In most cases the policy will never be changed during grid's lifetime.
> > >> No need for an explicit call to API on grid start.
> > >> A joining node should check a local configuration value to match the
> > grid.
> > >> If a dynamic value is already present in a metastore, it should
> override
> > >> static value with a warning.
> > >
> > > To sum it up:
> > > - ShutdownPolicy can be set with static configuration
> > > (IgniteConfiguration#setShutdownPolicy), on join we validate that
> > > statically configured policies on different server nodes are the same
> > > - It's possible to override statically configured value by adding
> > > distributed metastorage value, which can be done by
> > > calling ignite.cluster().setShutdownPolicy(plc) or control.sh method
> > > - Dynamic property is persisted
> > >
> > > Generally, I don't mind if we have both dynamic and static
> configuration
> > > properties. Necessity to call ignite.cluster().setShutdownPolicy(plc);
> on
> > > every new cluster creation is a usability issue itself.
> > > What bothers me here are the possible conflicts between static and
> > dynamic
> > > configuration. User may be surprised if he has shutdown policy X in
> > > IgniteConfiguration, but the cluster behaves according to policy Y
> > (because
> > > several months ago another admin had called
> > > IgniteCluster#setShutdownPolicy).
> > > We can handle it by adding a separate enum field to the shutdown
> policy:
> > >
> > >> public enum ShutdownPolicy {
> > >>   /* Default value of dynamic shutdown policy property. If it's set,
> the
> > >> shutdown policy is resolved according to value of static {@link
> > >> IgniteConfiguration#shutdownPolicy} configuration parameter. */
> > >>   USE_STATIC_CONFIGURATION,
> > >>
> > >>   /* Node leaves the cluster even if it's the last owner of some
> > >> partitions. Only partitions of caches with backups > 0 are taken into
> > >> account. */
> > >>   IMMEDIATE,
> > >>
> > >>   /* Shutdown is blocked until node is safe to leave without the data
> > >> loss. */
> > >>   GRACEFUL
> > >> }
> > >>
> > > This way:
> > > 1) User may easily understand whether the static parameter is
> overridden
> > > by dynamic. If ignite.cluster().getShutdownPolicy() return anything
> > except
> > > USE_STATIC_CONFIGURATION, behavior is overridden.
> > > 2) User may clear previous overriding by calling
> > > ignite.cluster().setShutdownPolicy(USE_STATIC_CONFIGURATION). After
> that,
> > > behavior will be resolved based in IgniteConfiguration#shutdownPolicy
> > again.
> > > If we agree on this mechanism, I propose to use IMMEDIATE name instead
> of
> > > DEFAULT for non-safe policy in order to don't confuse user.
> > > Meanwhile, static configuration will accept the same enum, but
> > > USE_STATIC_CONFIGURATION will be restricted:
> > >
> > >> public class IgniteConfiguration {
> > >>   public static final ShutdownPolicy DFLT_STATIC_SHUTDOWN_POLICY =
> > >> IMMEDIATE;
> > >>   private ShutdownPolicy shutdownPolicy = DFLT_STATIC_SHUTDOWN_POLICY;
> > >>   ...
> > >>   public void setShutdownPolicy(ShutdownPolicy shutdownPlc) {
> > >>     if (shutdownPlc ==  USE_STATIC_CONFIGURATION)
> > >>       throw new IllegalArgumentException("USE_STATIC_CONFIGURATION can
> > >> only be passed as dynamic property value via
> > >> ignite.cluster().setShutdownPolicy");
> > >>     ...
> > >>   }
> > >> ...
> > >> }
> > >>
> > >
> > > What do you think?
> > >
> > > On Tue, Jun 9, 2020 at 11:46 AM Alexei Scherbakov <
> > > [hidden email]> wrote:
> > >
> > >> Ivan Rakov,
> > >>
> > >> Your proposal overall looks good to me. My comments:
> > >>
> > >> 1. I would avoid adding such a method, because it will be impossible
> to
> > >> change it in the future if some more shutdown policies will be
> > introduced
> > >> later.
> > >> Also shutdown policy must be always consistent on the grid or
> > >> unintentional
> > >> data loss is possible if two nodes are stopping simultaneously with
> > >> different policies.
> > >>
> > >> This behavior can be achieved by changing policy globally when
> stopping
> > a
> > >> node:
> > >> ignite.cluster().setShutdownPolicy(DEFAULT);
> > >> ignore.stop();
> > >>
> > >> 2. defaultShutdownPolicy with DEFAULT value is a mess.
> WAIT_FOR_BACKUPS
> > is
> > >> not very clear either.
> > >> Let's use shutdownPolicy=DEFAULT|GRACEFUL, as was proposed by me
> > earlier.
> > >>
> > >> 3. OK
> > >>
> > >> 4. OK
> > >>
> > >> 5. Let's keep a static property for simplifying setting of initial
> > >> behavior.
> > >> In most cases the policy will never be changed during grid's lifetime.
> > >> No need for an explicit call to API on grid start.
> > >> A joining node should check a local configuration value to match the
> > grid.
> > >> If a dynamic value is already present in a metastore, it should
> override
> > >> static value with a warning.
> > >>
> > >>
> > >>
> > >>
> > >> пн, 8 июн. 2020 г. в 19:06, Ivan Rakov <[hidden email]>:
> > >>
> > >> > Vlad, thanks for starting this discussion.
> > >> >
> > >> > I'll try to clarify the motivation for this change as I see it.
> > >> > In general, Ignite clusters are vulnerable to the data loss. Of
> > course,
> > >> we
> > >> > have configurable PartitionLossPolicy, which allows to handle data
> > loss
> > >> > safely and mitigate its consequences. But being able to avoid
> critical
> > >> > situations is always better than being able to recover from it.
> > >> >
> > >> > The most common issue from my perspective is absence of a way to
> > perform
> > >> > rolling cluster restart safely. Scenario:
> > >> > 1. Backup count is 1
> > >> > 2. Admin wants to perform rolling restart in order to deploy new
> > >> version of
> > >> > business code that uses Ignite in embedded mode
> > >> > 3. Admin shuts down first node, replaces needed binaries and returns
> > the
> > >> > node back to the topology
> > >> > 4. Node joins the cluster successfully
> > >> > 5. Admin shuts down second node
> > >> > 6. Data loss happens: the second node was the only owner of a
> certain
> > >> > partition, which was being rebalanced from the second node to the
> > first
> > >> >
> > >> > We can prevent such situations by introducing "safe shutdown by
> > default"
> > >> > mode, which blocks stopping node while it remains the only owner for
> > at
> > >> > least one partition. It should be applied to "common" ways of
> stopping
> > >> > nodes - Ignite.close() and kill <pid>.
> > >> > I think, option to be enabled or disabled in runtime should be a
> > >> > requirement for this behavior. Safe shutdown mode has weird
> > >> side-effects.
> > >> > For example, admin won't be able to stop the whole cluster: stop of
> > last
> > >> > node will be blocked, because the last node is the only present
> owner
> > of
> > >> > all its partitions. Sure, kill -9 will resolve it, but it's still a
> > >> > usability issue.
> > >> >
> > >> > With the described dynamic property scenario will be changed as
> > follows:
> > >> > 1. Admin enables "safe shutdown" mode
> > >> > 2. Admin shuts down first node, replaces needed binaries and returns
> > the
> > >> > node back to the topology
> > >> > 3. Admin shuts down second node (with either ignite.close() or kill
> > >> <pid>),
> > >> > shutdown is blocked until the first node returns to the topology and
> > >> > completes the rebalancing process
> > >> > 4. Admin proceeds the rolling restart procedure
> > >> > 5. Admin disables "safe shutdown" mode
> > >> >
> > >> > This logic will also simplify the rolling restart scenario in K8S.
> Pod
> > >> with
> > >> > Ignite node won't be terminated until its termination will cause
> data
> > >> loss.
> > >> >
> > >> > Aside from waiting for backups, Ignition interface provide lots of
> > >> options
> > >> > to perform various node stop:
> > >> > - Whether or not to cancel pending compute jobs
> > >> > - Whether or not to perform instant halt() instead of any graceful
> > stop
> > >> > logic
> > >> > - Whether or not to wait for some timeout before halt()
> > >> > - Whether or not the stopped grid should be restarted
> > >> > All these "stop" methods provide very custom logic. I don't see a
> need
> > >> to
> > >> > make them part of dynamic cluster-wide configuration. They still can
> > be
> > >> > invoked directly via Java API. Later we can extract some of them to
> > >> dynamic
> > >> > cluster-wide parameters of default stop if it will become necessary.
> > >> That's
> > >> > why I think we should create an enum for default shutdown policy,
> but
> > >> only
> > >> > with two options so far (we can add more later): DEFAULT and
> > >> > WAIT_FOR_BACKUPS.
> > >> > Regarding the "NORMAL" option that you propose (where the node is
> not
> > >> > stopped until the rebalance is finished): I don't think that we
> should
> > >> add
> > >> > it. It doesn't ensure any strict guarantees: the data still can be
> > lost
> > >> > with it.
> > >> >
> > >> > To sum it up, I propose:
> > >> > 1. Add a new method to Ignition interface to make it possible to
> stop
> > >> with
> > >> > "wait for backups" logic directly via Java API, like
> > >> Ignition.stop(boolean
> > >> > cancel, boolean waitForBackups)
> > >> > 2. Introduce "defaultShutdownPolicy" as a dynamic cluster
> > configuration,
> > >> > two values are available so far: DEFAULT and WAIT_FOR_BACKUPS
> > >> > 3. This property is stored in the distributed metastorage (thus
> > >> persisted),
> > >> > can be changed via Java API and ./control.sh
> > >> > 4. Behavior configured with this property will be applied only on
> > common
> > >> > ways of stopping the node - Ignite.close() and kill <pid>.
> > >> > 5. *Don't* add new options to the static IgniteConfiguration to
> avoid
> > >> > conflicts between dynamic and static configuration
> > >> >
> > >> > --
> > >> > Best Regards,
> > >> > Ivan Rakov
> > >> >
> > >> > On Mon, Jun 8, 2020 at 6:44 PM V.Pyatkov <[hidden email]>
> > wrote:
> > >> >
> > >> > > Hi
> > >> > >
> > >> > > We need to have ability to calling shutdown with various
> guaranties.
> > >> > > For example:
> > >> > > Need to reboot a node, but after that node should be available for
> > >> > > historical rebalance (all partitions in MOVING state should have
> > gone
> > >> to
> > >> > > OWNING).
> > >> > >
> > >> > > Implemented a circled reboot of cluster, but all data should be
> > >> available
> > >> > > on
> > >> > > that time (at least one copy of partition should be available in
> > >> > cluster).
> > >> > >
> > >> > > Need to wait not only data available, but all jobs (before this
> > >> behavior
> > >> > > available through a stop(false) method invocation).
> > >> > >
> > >> > > All these reason required various behavior before shutting down
> > node.
> > >> > > I propose slightly modify public API and add here method which
> shown
> > >> on
> > >> > > shutdown behavior directly:
> > >> > > Ignite.close(Shutdown)
> > >> > >
> > >> > > /public enum Shutdownn {
> > >> > >     /**
> > >> > >      * Stop immediately as soon components are ready.
> > >> > >      */
> > >> > >     IMMEDIATE,
> > >> > >     /**
> > >> > >      * Stop node when all partitions completed moving from/to this
> > >> node
> > >> > to
> > >> > > another.
> > >> > >      */
> > >> > >     NORMAL,
> > >> > >     /**
> > >> > >      * Node will stop if and only if it does not store any unique
> > >> > > partitions, that does not have copies on cluster.
> > >> > >      */
> > >> > >     GRACEFUL,
> > >> > >     /**
> > >> > >      * Node stops graceful and wait all jobs before shutdown.
> > >> > >      */
> > >> > >     ALL
> > >> > > }/
> > >> > >
> > >> > > Method close without parameter Ignite.close() will get shutdown
> > >> behavior
> > >> > > configured for cluster wide. It will be implemented through
> > >> distributed
> > >> > > meta
> > >> > > storage and additional utilities for configuration.
> > >> > > Also, will be added a method to configure shutdown on start, this
> is
> > >> look
> > >> > > as
> > >> > > IgniteConfiguration.setShutdown(Shutdown).
> > >> > > If shutting down did not configure all be worked as before
> according
> > >> to
> > >> > > IMMEDIATE behavior.
> > >> > > All other close method will be marked as deprecated.
> > >> > >
> > >> > > I will be waiting for your opinions.
> > >> > >
> > >> > >
> > >> > >
> > >> > > --
> > >> > > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> > >> > >
> > >> >
> > >>
> > >>
> > >> --
> > >>
> > >> Best regards,
> > >> Alexei Scherbakov
> > >>
> > >
> >
>
>
> --
>
> Best regards,
> Alexei Scherbakov
>
Reply | Threaded
Open this post in threaded view
|

Re: Various shutdown guaranties

V.Pyatkov
Hi,

ignite.cluster().setShutdownPolicy(null); // Clear dynamic value and switch
to statically configured.

I do not understand why we need it. if user want to change configuration to
any other value he set it explicitly.
We can to add warning on start when static option does not math to dynamic
(dynamic always prefer if it initiated).

shutdownPolicy=IMMEDIATE|GRACEFUL

Looks better that DEFAULT and WAIT_FOR_BACKUP.

I general I consider job cancellation need to added in these policies'
enumeration.
But we can do it in the future.



--
Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
Reply | Threaded
Open this post in threaded view
|

Re: Various shutdown guaranties

Ivan Rakov
Vlad,

+1, that's what I mean.
We don't need either <null> or dedicated USE_STATIC_CONFIGURATION in case
the user will be able to retrieve current shutdown policy and apply the one
he needs.
My only requirement is that ignite.cluster().getShutdownPolicy() should
return a statically configured value {@link
IgniteConfiguration#shutdownPolicy} in case no override has been specified.
So, static configuration will be applied only on cluster start, like it
currently works for SQL schemas.

On Tue, Jun 9, 2020 at 7:09 PM V.Pyatkov <[hidden email]> wrote:

> Hi,
>
> ignite.cluster().setShutdownPolicy(null); // Clear dynamic value and switch
> to statically configured.
>
> I do not understand why we need it. if user want to change configuration to
> any other value he set it explicitly.
> We can to add warning on start when static option does not math to dynamic
> (dynamic always prefer if it initiated).
>
> shutdownPolicy=IMMEDIATE|GRACEFUL
>
> Looks better that DEFAULT and WAIT_FOR_BACKUP.
>
> I general I consider job cancellation need to added in these policies'
> enumeration.
> But we can do it in the future.
>
>
>
> --
> Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
>
Reply | Threaded
Open this post in threaded view
|

Re: Various shutdown guaranties

Alexei Scherbakov
+1, this is exactly what I want.

I'm fine with either IMMEDIATE or DEFAULT.

вт, 9 июн. 2020 г. в 19:41, Ivan Rakov <[hidden email]>:

> Vlad,
>
> +1, that's what I mean.
> We don't need either <null> or dedicated USE_STATIC_CONFIGURATION in case
> the user will be able to retrieve current shutdown policy and apply the one
> he needs.
> My only requirement is that ignite.cluster().getShutdownPolicy() should
> return a statically configured value {@link
> IgniteConfiguration#shutdownPolicy} in case no override has been specified.
> So, static configuration will be applied only on cluster start, like it
> currently works for SQL schemas.
>
> On Tue, Jun 9, 2020 at 7:09 PM V.Pyatkov <[hidden email]> wrote:
>
> > Hi,
> >
> > ignite.cluster().setShutdownPolicy(null); // Clear dynamic value and
> switch
> > to statically configured.
> >
> > I do not understand why we need it. if user want to change configuration
> to
> > any other value he set it explicitly.
> > We can to add warning on start when static option does not math to
> dynamic
> > (dynamic always prefer if it initiated).
> >
> > shutdownPolicy=IMMEDIATE|GRACEFUL
> >
> > Looks better that DEFAULT and WAIT_FOR_BACKUP.
> >
> > I general I consider job cancellation need to added in these policies'
> > enumeration.
> > But we can do it in the future.
> >
> >
> >
> > --
> > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> >
>


--

Best regards,
Alexei Scherbakov