[
https://issues.apache.org/jira/browse/IGNITE-23144?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Mikhail Efremov updated IGNITE-23144:
-------------------------------------
Description:
*Description*
The goal of the ticket is to add {{IgniteSpinBusyLock}} field for correct
thread-safe behavior on node stopping process. The common approach assumes the
follows:
* on {{VaultManager#stopAsync}} we should first touch {{AtomicBoolean}} field
{{stopGuard}} that checks that the stop method is processed only ones;
* then we must acquire {{busyLock}};
* after this acquisition all methods should be interrupted on begin with
{{busyLock#enterBusy}} in case if the method returns {{false}}. There are
options:
** log the situation and return {{null}};
** throw {{NodeStoppingException}}.
The last one approach is more complicated because the exception is checkable
and requires a corresponding try-catch handling on all up-to-hierarchy
call-chain. But it's still most suit and convenient. It may take a time to add
a proper try-catch handling.
Last, but not least is a method {{VaultManager#range}}. On the one hand
probably no one wants to handle {{NodeStoppingException}} on {{next}} method of
a cursor's iterator. Moreover no one expects runtime exception during exception
too (but I have to mention that this approach is used e.g. in
{{RocksDbMvPartitionStorage.BasePartitionTimestampCursor}} implementation). On
the other hand we may assume that there won't such many elements inside: there
is only one usage in {{LocalConfigurationStorage#readAll}}. Then, we may change
the method's return type to a {{Collection}} and read the whole cursor iterator
inside {{range}} (let's rename it to {{getAllInRange}}) under {{busyLock}}
section.
*Motivation*
Busy locks usage make the class safer on node stopping on the one hand, because
the vault manager and e.g. vault service are stopping asynchronously and
concurrent class addressing may lead to an inconsistent results. On the other
hand when node is stopping then we shouldn't serve incoming calls, that makes
stopping process faster.
*Definition of Done*
* {{busyLock}} field of type {{IgniteSpinBusyLock}} is introduced;
* {{stopGuard}} field of type {{AtomicBoolean}} is introduced;
* on {{VaultManager#stopAsync}} a value of {{stopGuard}} is changed from
assumed {{false}} to {{true}};
* on {{VaultManager#stopAsync}} the {{busyLock#block}} is called;
* every public method of {{VaultManager}} should call in a begin
{{busyLock.enterBusy()}} and handle node stopping situation if the call
returned {{false}}.
* if {{busyLock.enterBusy()}} was {{false}} then we should to throw
{{NodeStoppingException}} with a message like {{"Failed to process <methodName>
(the node is stopping) [<arg1>={}, <arg2>={} ... ]."}} where {{argN}} -- an
input argument of a method is named {{methodName}}.
* method {{range}} is renamed to {{getAllInRange}}), returns now {{Collection}}
and inside the body reads the whole cursor and collects elements to a return
collection under {{busyLock}} as other public methods.
was:
*Description*
The goal of the ticket is to add {{IgniteSpinBusyLock}} field for correct
thread-safe behavior on node stopping process. The common approach assumes the
follows:
* on {{VaultManager#stopAsync}} we should first touch {{AtomicBoolean}} field
{{stopGuard}} that checks that the stop method is processed only ones;
* then we must acquire {{busyLock}};
* after this acquisition all methods should be interrupted on begin with
{{busyLock#enterBusy}} in case if the method returns {{false}}. There are
options:
** log the situation and return {{null}};
** throw {{NodeStoppingException}}.
The last one approach is more complicated because the exception is checkable
and requires a corresponding try-catch handling on all up-to-hierarchy
call-chain. Then the first one looks preferable for {{void}} or {{@Nullable}}
return statement.
Hopefully, there is only one such method {{VaultManager#range}} and it has the
only call with try-with-resources handling.
*Motivation*
Busy locks usage make the class safer on node stopping on the one hand, because
the vault manager and e.g. vault service are stopping asynchronously and
concurrent class addressing may lead to an inconsistent results. On the other
hand when node is stopping then we shouldn't serve incoming calls, that makes
stopping process faster.
*Definition of Done*
* {{busyLock}} field of type {{IgniteSpinBusyLock}} is introduced;
* {{stopGuard}} field of type {{AtomicBoolean}} is introduced;
* on {{VaultManager#stopAsync}} a value of {{stopGuard}} is changed from
assumed {{false}} to {{true}};
* on {{VaultManager#stopAsync}} the {{busyLock#block}} is called;
* every public method of {{VaultManager}} should call in a begin
{{busyLock.enterBusy()}} and handle node stopping situation if the call
returned {{false}}.
> Add busy lock to VaultManager
> -----------------------------
>
> Key: IGNITE-23144
> URL: https://issues.apache.org/jira/browse/IGNITE-23144
> Project: Ignite
> Issue Type: Improvement
> Reporter: Roman Puchkovskiy
> Priority: Major
> Labels: ignite-3
>
> *Description*
> The goal of the ticket is to add {{IgniteSpinBusyLock}} field for correct
> thread-safe behavior on node stopping process. The common approach assumes
> the follows:
> * on {{VaultManager#stopAsync}} we should first touch {{AtomicBoolean}} field
> {{stopGuard}} that checks that the stop method is processed only ones;
> * then we must acquire {{busyLock}};
> * after this acquisition all methods should be interrupted on begin with
> {{busyLock#enterBusy}} in case if the method returns {{false}}. There are
> options:
> ** log the situation and return {{null}};
> ** throw {{NodeStoppingException}}.
> The last one approach is more complicated because the exception is checkable
> and requires a corresponding try-catch handling on all up-to-hierarchy
> call-chain. But it's still most suit and convenient. It may take a time to
> add a proper try-catch handling.
> Last, but not least is a method {{VaultManager#range}}. On the one hand
> probably no one wants to handle {{NodeStoppingException}} on {{next}} method
> of a cursor's iterator. Moreover no one expects runtime exception during
> exception too (but I have to mention that this approach is used e.g. in
> {{RocksDbMvPartitionStorage.BasePartitionTimestampCursor}} implementation).
> On the other hand we may assume that there won't such many elements inside:
> there is only one usage in {{LocalConfigurationStorage#readAll}}. Then, we
> may change the method's return type to a {{Collection}} and read the whole
> cursor iterator inside {{range}} (let's rename it to {{getAllInRange}}) under
> {{busyLock}} section.
> *Motivation*
> Busy locks usage make the class safer on node stopping on the one hand,
> because the vault manager and e.g. vault service are stopping asynchronously
> and concurrent class addressing may lead to an inconsistent results. On the
> other hand when node is stopping then we shouldn't serve incoming calls, that
> makes stopping process faster.
> *Definition of Done*
> * {{busyLock}} field of type {{IgniteSpinBusyLock}} is introduced;
> * {{stopGuard}} field of type {{AtomicBoolean}} is introduced;
> * on {{VaultManager#stopAsync}} a value of {{stopGuard}} is changed from
> assumed {{false}} to {{true}};
> * on {{VaultManager#stopAsync}} the {{busyLock#block}} is called;
> * every public method of {{VaultManager}} should call in a begin
> {{busyLock.enterBusy()}} and handle node stopping situation if the call
> returned {{false}}.
> * if {{busyLock.enterBusy()}} was {{false}} then we should to throw
> {{NodeStoppingException}} with a message like {{"Failed to process
> <methodName> (the node is stopping) [<arg1>={}, <arg2>={} ... ]."}} where
> {{argN}} -- an input argument of a method is named {{methodName}}.
> * method {{range}} is renamed to {{getAllInRange}}), returns now
> {{Collection}} and inside the body reads the whole cursor and collects
> elements to a return collection under {{busyLock}} as other public methods.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
