You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Ivan Rakov <iv...@gmail.com> on 2020/04/01 17:20:59 UTC
Re: Data vanished from cluster after INACTIVE/ACTIVE switch
I don't think that making javadocs more descriptive can be considered as
harmful code base enlargement.
I'd recommend to extend the docs, but the last word is yours ;)
On Tue, Mar 31, 2020 at 2:44 PM Vladimir Steshin <vl...@gmail.com> wrote:
> Ivan, hi.
>
> I absolutely agree this particular description is not enough to see the
> deactivation issue. I also vote for brief code.
>
> There are about 15 places in inner logic with this description. I
> propose balance between code base size and comment completeness.
>
> Should we enlarge code even if we got several full descriptions?
>
>
> 30.03.2020 20:02, Ivan Rakov пишет:
> > Vladimir,
> >
> > @param forceDeactivation If {@code true}, cluster deactivation will be
> >> forced.
> > It's true that it's possible to infer semantics of forced deactivation
> from
> > other parts of API. I just wanted to highlight that exactly this
> > description explains something that can be guessed by the parameter name.
> > I suppose to shorten the lookup path and shed a light on deactivation
> > semantics a bit:
> >
> >> @param forceDeactivation If {@code true}, cluster will be deactivated
> even
> >> if running in-memory caches are present. All data in the corresponding
> >> caches will be vanished as a result.
> > Does this make sense?
> >
> > On Fri, Mar 27, 2020 at 12:00 PM Vladimir Steshin <vl...@gmail.com>
> > wrote:
> >
> >> Ivan, hi.
> >>
> >>
> >> 1) >>> Is it correct? If we are on the same page, let's proceed this way
> >>
> >> It is correct.
> >>
> >>
> >> 2) - In many places in the code I can see the following javadoc
> >>
> >>> @param forceDeactivation If {@code true}, cluster deactivation will
> be
> >> forced.
> >>
> >> In the internal params/flags. You can also find /@see
> >> ClusterState#INACTIVE/ and full description with several public APIs (
> >> like /Ignite.active(boolean)/ ):
> >>
> >> //
> >>
> >> /* <p>/
> >>
> >> //
> >>
> >> /* <b>NOTE:</b>/
> >>
> >> //
> >>
> >> /* Deactivation clears in-memory caches (without persistence) including
> >> the system caches./
> >>
> >> Should be enough. Is not?
> >>
> >>
> >> 27.03.2020 10:51, Ivan Rakov пишет:
> >>> Vladimir, Igniters,
> >>>
> >>> Let's emphasize our final plan.
> >>>
> >>> We are going to add --force flags that will be necessary to pass for a
> >>> deactivation if there are in-memory caches to:
> >>> 1) Rest API (already implemented in [1])
> >>> 2) Command line utility (already implemented in [1])
> >>> 3) JMX bean (going to be implemented in [2])
> >>> We are *not* going to change IgniteCluster or any other thick Java API,
> >>> thus we are *not* going to merge [3].
> >>> We plan to *fully rollback* [1] and [2] once cache data survival after
> >>> activation-deactivation cycle will be implemented.
> >>>
> >>> Is it correct? If we are on the same page, let's proceed this way.
> >>> I propose to:
> >>> - Create a JIRA issue for in-memory-data-safe deactivation (possibly,
> >>> without IEP and detailed design so far)
> >>> - Describe in the issue description what exact parts of API should be
> >>> removed under the issue scope.
> >>>
> >>> Also, a few questions on already merged [1]:
> >>> - We have removed GridClientClusterState#state(ClusterState) from Java
> >>> client API. Is it a legitimate thing to do? Don't we have to support
> API
> >>> compatibility for thin clients as well?
> >>> - In many places in the code I can see the following javadoc
> >>>
> >>>> @param forceDeactivation If {@code true}, cluster deactivation will
> >> be forced.
> >>>> As for me, this javadoc doesn't clarify anything. I'd suggest to
> >> describe
> >>> in which cases deactivation won't happen unless it's forced and which
> >>> impact forced deactivation will bring on the system.
> >>>
> >>> [1]: https://issues.apache.org/jira/browse/IGNITE-12701
> >>> [2]: https://issues.apache.org/jira/browse/IGNITE-12779
> >>> [3]: https://issues.apache.org/jira/browse/IGNITE-12614
> >>>
> >>> --
> >>> Ivan
> >>>
> >>> On Tue, Mar 24, 2020 at 7:18 PM Vladimir Steshin <vl...@gmail.com>
> >> wrote:
> >>>> Hi, Igniters.
> >>>>
> >>>> I'd like to remind you that cluster can be deactivated by user with 3
> >>>> utilities: control.sh, *JMX and the REST*. Proposed in [1] solution is
> >>>> not about control.sh. It suggests same approach regardless of the
> >>>> utility user executes. The task touches *only* *API of the user
> calls*,
> >>>> not the internal APIs.
> >>>>
> >>>> The reasons why flag “--yes” and confirmation prompt hasn’t taken into
> >>>> account for control.sh are:
> >>>>
> >>>> -Various commands widely use “--yes” just to start. Even not dangerous
> >>>> ones require “--yes” to begin. “--force” is dedicated for *harmless
> >>>> actions*.
> >>>>
> >>>> -Checking of probable data erasure works after command start and
> >>>> “--force” may not be required at all.
> >>>>
> >>>> -There are also JMX and REST. They have no “—yes” but should work
> alike.
> >>>>
> >>>> To get the deactivation safe I propose to merge last ticket
> with
> >>>> the JMX fixes [2]. In future releases, I believe, we should estimate
> >>>> jobs and fix memory erasure in general. For now, let’s prevent it.
> WDYT?
> >>>>
> >>>>
> >>>> [1] https://issues.apache.org/jira/browse/IGNITE-12614
> >>>>
> >>>> [2] https://issues.apache.org/jira/browse/IGNITE-12779
> >>>>
> >>>>
> >>>> 24.03.2020 15:55, Вячеслав Коптилин пишет:
> >>>>> Hello Nikolay,
> >>>>>
> >>>>> I am talking about the interactive mode of the control utility, which
> >>>>> requires explicit confirmation from the user.
> >>>>> Please take a look at DeactivateCommand#prepareConfirmation and its
> >>>> usages.
> >>>>> It seems to me, this mode has the same aim as the forceDeactivation
> >> flag.
> >>>>> We can change the message returned by
> >>>> DeactivateCommand#confirmationPrompt
> >>>>> as follows:
> >>>>> "Warning: the command will deactivate the cluster nnn and
> clear
> >>>>> in-memory caches (without persistence) including system caches."
> >>>>>
> >>>>> What do you think?
> >>>>>
> >>>>> Thanks,
> >>>>> S.
> >>>>>
> >>>>> вт, 24 мар. 2020 г. в 13:07, Nikolay Izhikov <ni...@apache.org>:
> >>>>>
> >>>>>> Hello, Slava.
> >>>>>>
> >>>>>> Are you talking about this commit [1] (sorry for commit message it’s
> >> due
> >>>>>> to the Github issue)?
> >>>>>>
> >>>>>> The message for this command for now
> >>>>>>
> >>>>>> «Deactivation stopped. Deactivation clears in-memory caches (without
> >>>>>> persistence) including the system caches.»
> >>>>>>
> >>>>>> Is it clear enough?
> >>>>>>
> >>>>>> [1]
> >>>>>>
> >>
> https://github.com/apache/ignite/commit/4921fcf1fecbd8a1ab02099e09cc2adb0b3ff88a
> >>>>>>> 24 марта 2020 г., в 13:02, Вячеслав Коптилин <
> >> slava.koptilin@gmail.com
> >>>>>> написал(а):
> >>>>>>> Hi Nikolay,
> >>>>>>>
> >>>>>>>> 1. We should add —force flag to the command.sh deactivation
> command.
> >>>>>>> I just checked and it seems that the deactivation command
> >>>>>>> (control-utility.sh) already has a confirmation option.
> >>>>>>> Perhaps, we need to clearly state the consequences of using this
> >>>> command
> >>>>>>> with in-memory caches.
> >>>>>>>
> >>>>>>> Thanks,
> >>>>>>> S.
> >>>>>>>
> >>>>>>> вт, 24 мар. 2020 г. в 12:51, Nikolay Izhikov <nizhikov@apache.org
> >:
> >>>>>>>
> >>>>>>>> Hello, Alexey.
> >>>>>>>>
> >>>>>>>> I just repeat our agreement to be on the same page
> >>>>>>>>
> >>>>>>>>> The confirmation should only present in the user-facing
> interfaces.
> >>>>>>>> 1. We should add —force flag to the command.sh deactivation
> command.
> >>>>>>>> 2. We should throw the exception if cluster has in-memory caches
> and
> >>>>>>>> —force=false.
> >>>>>>>> 3. We shouldn’t change Java API for deactivation.
> >>>>>>>>
> >>>>>>>> Is it correct?
> >>>>>>>>
> >>>>>>>>> The DROP TABLE command does not have a "yes I am sure" clause in
> it
> >>>>>>>> I think it because the command itself has a «DROP» word in it’s
> >> name.
> >>>>>>>> Which clearly explains what will happen on it’s execution.
> >>>>>>>>
> >>>>>>>> On the opposite «deactivation» command doesn’t have any sign of
> >>>>>>>> destructive operation.
> >>>>>>>> That’s why we should warn user about it’s consequences.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>> 24 марта 2020 г., в 12:38, Alexey Goncharuk <
> >>>>>> alexey.goncharuk@gmail.com>
> >>>>>>>> написал(а):
> >>>>>>>>> Igniters, Ivan, Nikolay,
> >>>>>>>>>
> >>>>>>>>> I am strongly against adding confirmation flags to any kind of
> >> APIs,
> >>>>>>>>> whether we change the deactivation behavior or not (even though I
> >>>> agree
> >>>>>>>>> that it makes sense to fix the deactivation to not clean up the
> >>>>>> in-memory
> >>>>>>>>> data). The confirmation should only present in the user-facing
> >>>>>>>> interfaces.
> >>>>>>>>> I cannot recall any software interface which has such a flag.
> None
> >> of
> >>>>>> the
> >>>>>>>>> syscalls which delete files (a very destructive operation) have
> >> this
> >>>>>>>> flag.
> >>>>>>>>> The DROP TABLE command does not have a "yes I am sure" clause in
> >> it.
> >>>>>> As I
> >>>>>>>>> already mentioned, when used programmatically, most users will
> >> likely
> >>>>>>>>> simply pass 'true' as the new flag because they already know the
> >>>>>>>> behavior.
> >>>>>>>>> This is a clear sign of a bad design choice.
> >>>>>>>>>
> >>>>>>>>> On top of that, given that it is our intention to change the
> >> behavior
> >>>>>> of
> >>>>>>>>> deactivation to not loose the in-memory data, it does not make
> >> sense
> >>>> to
> >>>>>>>> me
> >>>>>>>>> to change the API.
>
Re: Data vanished from cluster after INACTIVE/ACTIVE switch
Posted by Вячеслав Коптилин <sl...@gmail.com>.
Hello Vladimir,
> onto anolugous
> @param forceDeactivation If {@code true}, cluster deactivation will be
forced. //Deactivation clears in-memory caches (without persistence)
including the system caches.
My idea was about providing a link to one place with a complete description
instead of copy&paste.
> We might include this fix in the last [1].
It is up to you, Vladimir.
Thanks,
S.
пт, 3 апр. 2020 г. в 15:42, Vladimir Steshin <vl...@gmail.com>:
> Slava, hello.
>
>
> All right, since we have in public API several
>
> /* Deactivation clears in-memory caches (without persistence) including
> the system caches./
>
> We should change in the internals
>
> / @param forceDeactivation If {@code true}, cluster deactivation will
> be forced./
>
> onto anolugous
>
> / @param forceDeactivation If {@code true}, cluster deactivation will
> be forced. //Deactivation clears in-memory caches (without persistence)
> including the system caches./
>
> //
>
> We might include this fix in the last [1]. WDYT, can we proceed with [1]
> then ?
>
> [1] https://issues.apache.org/jira/browse/IGNITE-12779
>
>
>
> 02.04.2020 19:58, Вячеслав Коптилин пишет:
> > Hi Vladimir,
> >
> >> There are about 15 places in inner logic with this description.
> >> I propose balance between code base size and comment completeness.
> > I agree with Iva and I also think that this approach is not so good.
> > Perhaps we can add just a link to the one method which will provide a
> > comprehensive description, something like as follows
> > @param forceDeactivation {@code true} if cluster deactivation should be
> > forced. Please take a look at {@link IgniteCluster#state(ClusterState
> > newState, boolean force)} for the details.
> >
> > What do you think?
> >
> > Thanks,
> > Slava.
> >
> > чт, 2 апр. 2020 г. в 18:47, Vladimir Steshin <vl...@gmail.com>:
> >
> >> Ivan, hello.
> >>
> >> Thanks. I vote for keeping the comments as they are now :)
> >>
> >> Igniters, it seems we are agreed to merge [1]. And the ticked s to be
> >> reverted in future with new designed solution of keeping in-memory data
> >> after deactivation.
> >>
> >> Right?
> >>
> >>
> >> [1] https://issues.apache.org/jira/browse/IGNITE-12779
> >>
> >>
> >> 01.04.2020 20:20, Ivan Rakov пишет:
> >>> I don't think that making javadocs more descriptive can be considered
> as
> >>> harmful code base enlargement.
> >>> I'd recommend to extend the docs, but the last word is yours ;)
> >>>
> >>> On Tue, Mar 31, 2020 at 2:44 PM Vladimir Steshin <vl...@gmail.com>
> >> wrote:
> >>>> Ivan, hi.
> >>>>
> >>>> I absolutely agree this particular description is not enough to see
> the
> >>>> deactivation issue. I also vote for brief code.
> >>>>
> >>>> There are about 15 places in inner logic with this description. I
> >>>> propose balance between code base size and comment completeness.
> >>>>
> >>>> Should we enlarge code even if we got several full descriptions?
> >>>>
> >>>>
> >>>> 30.03.2020 20:02, Ivan Rakov пишет:
> >>>>> Vladimir,
> >>>>>
> >>>>> @param forceDeactivation If {@code true}, cluster deactivation will
> be
> >>>>>> forced.
> >>>>> It's true that it's possible to infer semantics of forced
> deactivation
> >>>> from
> >>>>> other parts of API. I just wanted to highlight that exactly this
> >>>>> description explains something that can be guessed by the parameter
> >> name.
> >>>>> I suppose to shorten the lookup path and shed a light on deactivation
> >>>>> semantics a bit:
> >>>>>
> >>>>>> @param forceDeactivation If {@code true}, cluster will be
> deactivated
> >>>> even
> >>>>>> if running in-memory caches are present. All data in the
> corresponding
> >>>>>> caches will be vanished as a result.
> >>>>> Does this make sense?
> >>>>>
> >>>>> On Fri, Mar 27, 2020 at 12:00 PM Vladimir Steshin <
> vladsz83@gmail.com>
> >>>>> wrote:
> >>>>>
> >>>>>> Ivan, hi.
> >>>>>>
> >>>>>>
> >>>>>> 1) >>> Is it correct? If we are on the same page, let's proceed this
> >> way
> >>>>>> It is correct.
> >>>>>>
> >>>>>>
> >>>>>> 2) - In many places in the code I can see the following javadoc
> >>>>>>
> >>>>>>> @param forceDeactivation If {@code true}, cluster deactivation
> >> will
> >>>> be
> >>>>>> forced.
> >>>>>>
> >>>>>> In the internal params/flags. You can also find /@see
> >>>>>> ClusterState#INACTIVE/ and full description with several public
> APIs (
> >>>>>> like /Ignite.active(boolean)/ ):
> >>>>>>
> >>>>>> //
> >>>>>>
> >>>>>> /* <p>/
> >>>>>>
> >>>>>> //
> >>>>>>
> >>>>>> /* <b>NOTE:</b>/
> >>>>>>
> >>>>>> //
> >>>>>>
> >>>>>> /* Deactivation clears in-memory caches (without persistence)
> >> including
> >>>>>> the system caches./
> >>>>>>
> >>>>>> Should be enough. Is not?
> >>>>>>
> >>>>>>
> >>>>>> 27.03.2020 10:51, Ivan Rakov пишет:
> >>>>>>> Vladimir, Igniters,
> >>>>>>>
> >>>>>>> Let's emphasize our final plan.
> >>>>>>>
> >>>>>>> We are going to add --force flags that will be necessary to pass
> for
> >> a
> >>>>>>> deactivation if there are in-memory caches to:
> >>>>>>> 1) Rest API (already implemented in [1])
> >>>>>>> 2) Command line utility (already implemented in [1])
> >>>>>>> 3) JMX bean (going to be implemented in [2])
> >>>>>>> We are *not* going to change IgniteCluster or any other thick Java
> >> API,
> >>>>>>> thus we are *not* going to merge [3].
> >>>>>>> We plan to *fully rollback* [1] and [2] once cache data survival
> >> after
> >>>>>>> activation-deactivation cycle will be implemented.
> >>>>>>>
> >>>>>>> Is it correct? If we are on the same page, let's proceed this way.
> >>>>>>> I propose to:
> >>>>>>> - Create a JIRA issue for in-memory-data-safe deactivation
> (possibly,
> >>>>>>> without IEP and detailed design so far)
> >>>>>>> - Describe in the issue description what exact parts of API should
> be
> >>>>>>> removed under the issue scope.
> >>>>>>>
> >>>>>>> Also, a few questions on already merged [1]:
> >>>>>>> - We have removed GridClientClusterState#state(ClusterState) from
> >> Java
> >>>>>>> client API. Is it a legitimate thing to do? Don't we have to
> support
> >>>> API
> >>>>>>> compatibility for thin clients as well?
> >>>>>>> - In many places in the code I can see the following javadoc
> >>>>>>>
> >>>>>>>> @param forceDeactivation If {@code true}, cluster
> deactivation
> >> will
> >>>>>> be forced.
> >>>>>>>> As for me, this javadoc doesn't clarify anything. I'd suggest to
> >>>>>> describe
> >>>>>>> in which cases deactivation won't happen unless it's forced and
> which
> >>>>>>> impact forced deactivation will bring on the system.
> >>>>>>>
> >>>>>>> [1]: https://issues.apache.org/jira/browse/IGNITE-12701
> >>>>>>> [2]: https://issues.apache.org/jira/browse/IGNITE-12779
> >>>>>>> [3]: https://issues.apache.org/jira/browse/IGNITE-12614
> >>>>>>>
> >>>>>>> --
> >>>>>>> Ivan
> >>>>>>>
> >>>>>>> On Tue, Mar 24, 2020 at 7:18 PM Vladimir Steshin <
> vladsz83@gmail.com
> >>>>>> wrote:
> >>>>>>>> Hi, Igniters.
> >>>>>>>>
> >>>>>>>> I'd like to remind you that cluster can be deactivated by user
> with
> >> 3
> >>>>>>>> utilities: control.sh, *JMX and the REST*. Proposed in [1]
> solution
> >> is
> >>>>>>>> not about control.sh. It suggests same approach regardless of the
> >>>>>>>> utility user executes. The task touches *only* *API of the user
> >>>> calls*,
> >>>>>>>> not the internal APIs.
> >>>>>>>>
> >>>>>>>> The reasons why flag “--yes” and confirmation prompt hasn’t taken
> >> into
> >>>>>>>> account for control.sh are:
> >>>>>>>>
> >>>>>>>> -Various commands widely use “--yes” just to start. Even not
> >> dangerous
> >>>>>>>> ones require “--yes” to begin. “--force” is dedicated for
> *harmless
> >>>>>>>> actions*.
> >>>>>>>>
> >>>>>>>> -Checking of probable data erasure works after command start and
> >>>>>>>> “--force” may not be required at all.
> >>>>>>>>
> >>>>>>>> -There are also JMX and REST. They have no “—yes” but should work
> >>>> alike.
> >>>>>>>> To get the deactivation safe I propose to merge last
> ticket
> >>>> with
> >>>>>>>> the JMX fixes [2]. In future releases, I believe, we should
> estimate
> >>>>>>>> jobs and fix memory erasure in general. For now, let’s prevent it.
> >>>> WDYT?
> >>>>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-12614
> >>>>>>>>
> >>>>>>>> [2] https://issues.apache.org/jira/browse/IGNITE-12779
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> 24.03.2020 15:55, Вячеслав Коптилин пишет:
> >>>>>>>>> Hello Nikolay,
> >>>>>>>>>
> >>>>>>>>> I am talking about the interactive mode of the control utility,
> >> which
> >>>>>>>>> requires explicit confirmation from the user.
> >>>>>>>>> Please take a look at DeactivateCommand#prepareConfirmation and
> its
> >>>>>>>> usages.
> >>>>>>>>> It seems to me, this mode has the same aim as the
> forceDeactivation
> >>>>>> flag.
> >>>>>>>>> We can change the message returned by
> >>>>>>>> DeactivateCommand#confirmationPrompt
> >>>>>>>>> as follows:
> >>>>>>>>> "Warning: the command will deactivate the cluster nnn
> and
> >>>> clear
> >>>>>>>>> in-memory caches (without persistence) including system caches."
> >>>>>>>>>
> >>>>>>>>> What do you think?
> >>>>>>>>>
> >>>>>>>>> Thanks,
> >>>>>>>>> S.
> >>>>>>>>>
> >>>>>>>>> вт, 24 мар. 2020 г. в 13:07, Nikolay Izhikov <
> nizhikov@apache.org
> >>> :
> >>>>>>>>>> Hello, Slava.
> >>>>>>>>>>
> >>>>>>>>>> Are you talking about this commit [1] (sorry for commit message
> >> it’s
> >>>>>> due
> >>>>>>>>>> to the Github issue)?
> >>>>>>>>>>
> >>>>>>>>>> The message for this command for now
> >>>>>>>>>>
> >>>>>>>>>> «Deactivation stopped. Deactivation clears in-memory caches
> >> (without
> >>>>>>>>>> persistence) including the system caches.»
> >>>>>>>>>>
> >>>>>>>>>> Is it clear enough?
> >>>>>>>>>>
> >>>>>>>>>> [1]
> >>>>>>>>>>
> >>
> https://github.com/apache/ignite/commit/4921fcf1fecbd8a1ab02099e09cc2adb0b3ff88a
> >>>>>>>>>>> 24 марта 2020 г., в 13:02, Вячеслав Коптилин <
> >>>>>> slava.koptilin@gmail.com
> >>>>>>>>>> написал(а):
> >>>>>>>>>>> Hi Nikolay,
> >>>>>>>>>>>
> >>>>>>>>>>>> 1. We should add —force flag to the command.sh deactivation
> >>>> command.
> >>>>>>>>>>> I just checked and it seems that the deactivation command
> >>>>>>>>>>> (control-utility.sh) already has a confirmation option.
> >>>>>>>>>>> Perhaps, we need to clearly state the consequences of using
> this
> >>>>>>>> command
> >>>>>>>>>>> with in-memory caches.
> >>>>>>>>>>>
> >>>>>>>>>>> Thanks,
> >>>>>>>>>>> S.
> >>>>>>>>>>>
> >>>>>>>>>>> вт, 24 мар. 2020 г. в 12:51, Nikolay Izhikov <
> >> nizhikov@apache.org
> >>>>> :
> >>>>>>>>>>>> Hello, Alexey.
> >>>>>>>>>>>>
> >>>>>>>>>>>> I just repeat our agreement to be on the same page
> >>>>>>>>>>>>
> >>>>>>>>>>>>> The confirmation should only present in the user-facing
> >>>> interfaces.
> >>>>>>>>>>>> 1. We should add —force flag to the command.sh deactivation
> >>>> command.
> >>>>>>>>>>>> 2. We should throw the exception if cluster has in-memory
> caches
> >>>> and
> >>>>>>>>>>>> —force=false.
> >>>>>>>>>>>> 3. We shouldn’t change Java API for deactivation.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Is it correct?
> >>>>>>>>>>>>
> >>>>>>>>>>>>> The DROP TABLE command does not have a "yes I am sure" clause
> >> in
> >>>> it
> >>>>>>>>>>>> I think it because the command itself has a «DROP» word in
> it’s
> >>>>>> name.
> >>>>>>>>>>>> Which clearly explains what will happen on it’s execution.
> >>>>>>>>>>>>
> >>>>>>>>>>>> On the opposite «deactivation» command doesn’t have any sign
> of
> >>>>>>>>>>>> destructive operation.
> >>>>>>>>>>>> That’s why we should warn user about it’s consequences.
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>>> 24 марта 2020 г., в 12:38, Alexey Goncharuk <
> >>>>>>>>>> alexey.goncharuk@gmail.com>
> >>>>>>>>>>>> написал(а):
> >>>>>>>>>>>>> Igniters, Ivan, Nikolay,
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> I am strongly against adding confirmation flags to any kind
> of
> >>>>>> APIs,
> >>>>>>>>>>>>> whether we change the deactivation behavior or not (even
> >> though I
> >>>>>>>> agree
> >>>>>>>>>>>>> that it makes sense to fix the deactivation to not clean up
> the
> >>>>>>>>>> in-memory
> >>>>>>>>>>>>> data). The confirmation should only present in the
> user-facing
> >>>>>>>>>>>> interfaces.
> >>>>>>>>>>>>> I cannot recall any software interface which has such a flag.
> >>>> None
> >>>>>> of
> >>>>>>>>>> the
> >>>>>>>>>>>>> syscalls which delete files (a very destructive operation)
> have
> >>>>>> this
> >>>>>>>>>>>> flag.
> >>>>>>>>>>>>> The DROP TABLE command does not have a "yes I am sure" clause
> >> in
> >>>>>> it.
> >>>>>>>>>> As I
> >>>>>>>>>>>>> already mentioned, when used programmatically, most users
> will
> >>>>>> likely
> >>>>>>>>>>>>> simply pass 'true' as the new flag because they already know
> >> the
> >>>>>>>>>>>> behavior.
> >>>>>>>>>>>>> This is a clear sign of a bad design choice.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> On top of that, given that it is our intention to change the
> >>>>>> behavior
> >>>>>>>>>> of
> >>>>>>>>>>>>> deactivation to not loose the in-memory data, it does not
> make
> >>>>>> sense
> >>>>>>>> to
> >>>>>>>>>>>> me
> >>>>>>>>>>>>> to change the API.
>
Re: Data vanished from cluster after INACTIVE/ACTIVE switch
Posted by Vladimir Steshin <vl...@gmail.com>.
Slava, hello.
All right, since we have in public API several
/* Deactivation clears in-memory caches (without persistence) including
the system caches./
We should change in the internals
/ @param forceDeactivation If {@code true}, cluster deactivation will
be forced./
onto anolugous
/ @param forceDeactivation If {@code true}, cluster deactivation will
be forced. //Deactivation clears in-memory caches (without persistence)
including the system caches./
//
We might include this fix in the last [1]. WDYT, can we proceed with [1]
then ?
[1] https://issues.apache.org/jira/browse/IGNITE-12779
02.04.2020 19:58, Вячеслав Коптилин пишет:
> Hi Vladimir,
>
>> There are about 15 places in inner logic with this description.
>> I propose balance between code base size and comment completeness.
> I agree with Iva and I also think that this approach is not so good.
> Perhaps we can add just a link to the one method which will provide a
> comprehensive description, something like as follows
> @param forceDeactivation {@code true} if cluster deactivation should be
> forced. Please take a look at {@link IgniteCluster#state(ClusterState
> newState, boolean force)} for the details.
>
> What do you think?
>
> Thanks,
> Slava.
>
> чт, 2 апр. 2020 г. в 18:47, Vladimir Steshin <vl...@gmail.com>:
>
>> Ivan, hello.
>>
>> Thanks. I vote for keeping the comments as they are now :)
>>
>> Igniters, it seems we are agreed to merge [1]. And the ticked s to be
>> reverted in future with new designed solution of keeping in-memory data
>> after deactivation.
>>
>> Right?
>>
>>
>> [1] https://issues.apache.org/jira/browse/IGNITE-12779
>>
>>
>> 01.04.2020 20:20, Ivan Rakov пишет:
>>> I don't think that making javadocs more descriptive can be considered as
>>> harmful code base enlargement.
>>> I'd recommend to extend the docs, but the last word is yours ;)
>>>
>>> On Tue, Mar 31, 2020 at 2:44 PM Vladimir Steshin <vl...@gmail.com>
>> wrote:
>>>> Ivan, hi.
>>>>
>>>> I absolutely agree this particular description is not enough to see the
>>>> deactivation issue. I also vote for brief code.
>>>>
>>>> There are about 15 places in inner logic with this description. I
>>>> propose balance between code base size and comment completeness.
>>>>
>>>> Should we enlarge code even if we got several full descriptions?
>>>>
>>>>
>>>> 30.03.2020 20:02, Ivan Rakov пишет:
>>>>> Vladimir,
>>>>>
>>>>> @param forceDeactivation If {@code true}, cluster deactivation will be
>>>>>> forced.
>>>>> It's true that it's possible to infer semantics of forced deactivation
>>>> from
>>>>> other parts of API. I just wanted to highlight that exactly this
>>>>> description explains something that can be guessed by the parameter
>> name.
>>>>> I suppose to shorten the lookup path and shed a light on deactivation
>>>>> semantics a bit:
>>>>>
>>>>>> @param forceDeactivation If {@code true}, cluster will be deactivated
>>>> even
>>>>>> if running in-memory caches are present. All data in the corresponding
>>>>>> caches will be vanished as a result.
>>>>> Does this make sense?
>>>>>
>>>>> On Fri, Mar 27, 2020 at 12:00 PM Vladimir Steshin <vl...@gmail.com>
>>>>> wrote:
>>>>>
>>>>>> Ivan, hi.
>>>>>>
>>>>>>
>>>>>> 1) >>> Is it correct? If we are on the same page, let's proceed this
>> way
>>>>>> It is correct.
>>>>>>
>>>>>>
>>>>>> 2) - In many places in the code I can see the following javadoc
>>>>>>
>>>>>>> @param forceDeactivation If {@code true}, cluster deactivation
>> will
>>>> be
>>>>>> forced.
>>>>>>
>>>>>> In the internal params/flags. You can also find /@see
>>>>>> ClusterState#INACTIVE/ and full description with several public APIs (
>>>>>> like /Ignite.active(boolean)/ ):
>>>>>>
>>>>>> //
>>>>>>
>>>>>> /* <p>/
>>>>>>
>>>>>> //
>>>>>>
>>>>>> /* <b>NOTE:</b>/
>>>>>>
>>>>>> //
>>>>>>
>>>>>> /* Deactivation clears in-memory caches (without persistence)
>> including
>>>>>> the system caches./
>>>>>>
>>>>>> Should be enough. Is not?
>>>>>>
>>>>>>
>>>>>> 27.03.2020 10:51, Ivan Rakov пишет:
>>>>>>> Vladimir, Igniters,
>>>>>>>
>>>>>>> Let's emphasize our final plan.
>>>>>>>
>>>>>>> We are going to add --force flags that will be necessary to pass for
>> a
>>>>>>> deactivation if there are in-memory caches to:
>>>>>>> 1) Rest API (already implemented in [1])
>>>>>>> 2) Command line utility (already implemented in [1])
>>>>>>> 3) JMX bean (going to be implemented in [2])
>>>>>>> We are *not* going to change IgniteCluster or any other thick Java
>> API,
>>>>>>> thus we are *not* going to merge [3].
>>>>>>> We plan to *fully rollback* [1] and [2] once cache data survival
>> after
>>>>>>> activation-deactivation cycle will be implemented.
>>>>>>>
>>>>>>> Is it correct? If we are on the same page, let's proceed this way.
>>>>>>> I propose to:
>>>>>>> - Create a JIRA issue for in-memory-data-safe deactivation (possibly,
>>>>>>> without IEP and detailed design so far)
>>>>>>> - Describe in the issue description what exact parts of API should be
>>>>>>> removed under the issue scope.
>>>>>>>
>>>>>>> Also, a few questions on already merged [1]:
>>>>>>> - We have removed GridClientClusterState#state(ClusterState) from
>> Java
>>>>>>> client API. Is it a legitimate thing to do? Don't we have to support
>>>> API
>>>>>>> compatibility for thin clients as well?
>>>>>>> - In many places in the code I can see the following javadoc
>>>>>>>
>>>>>>>> @param forceDeactivation If {@code true}, cluster deactivation
>> will
>>>>>> be forced.
>>>>>>>> As for me, this javadoc doesn't clarify anything. I'd suggest to
>>>>>> describe
>>>>>>> in which cases deactivation won't happen unless it's forced and which
>>>>>>> impact forced deactivation will bring on the system.
>>>>>>>
>>>>>>> [1]: https://issues.apache.org/jira/browse/IGNITE-12701
>>>>>>> [2]: https://issues.apache.org/jira/browse/IGNITE-12779
>>>>>>> [3]: https://issues.apache.org/jira/browse/IGNITE-12614
>>>>>>>
>>>>>>> --
>>>>>>> Ivan
>>>>>>>
>>>>>>> On Tue, Mar 24, 2020 at 7:18 PM Vladimir Steshin <vladsz83@gmail.com
>>>>>> wrote:
>>>>>>>> Hi, Igniters.
>>>>>>>>
>>>>>>>> I'd like to remind you that cluster can be deactivated by user with
>> 3
>>>>>>>> utilities: control.sh, *JMX and the REST*. Proposed in [1] solution
>> is
>>>>>>>> not about control.sh. It suggests same approach regardless of the
>>>>>>>> utility user executes. The task touches *only* *API of the user
>>>> calls*,
>>>>>>>> not the internal APIs.
>>>>>>>>
>>>>>>>> The reasons why flag “--yes” and confirmation prompt hasn’t taken
>> into
>>>>>>>> account for control.sh are:
>>>>>>>>
>>>>>>>> -Various commands widely use “--yes” just to start. Even not
>> dangerous
>>>>>>>> ones require “--yes” to begin. “--force” is dedicated for *harmless
>>>>>>>> actions*.
>>>>>>>>
>>>>>>>> -Checking of probable data erasure works after command start and
>>>>>>>> “--force” may not be required at all.
>>>>>>>>
>>>>>>>> -There are also JMX and REST. They have no “—yes” but should work
>>>> alike.
>>>>>>>> To get the deactivation safe I propose to merge last ticket
>>>> with
>>>>>>>> the JMX fixes [2]. In future releases, I believe, we should estimate
>>>>>>>> jobs and fix memory erasure in general. For now, let’s prevent it.
>>>> WDYT?
>>>>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-12614
>>>>>>>>
>>>>>>>> [2] https://issues.apache.org/jira/browse/IGNITE-12779
>>>>>>>>
>>>>>>>>
>>>>>>>> 24.03.2020 15:55, Вячеслав Коптилин пишет:
>>>>>>>>> Hello Nikolay,
>>>>>>>>>
>>>>>>>>> I am talking about the interactive mode of the control utility,
>> which
>>>>>>>>> requires explicit confirmation from the user.
>>>>>>>>> Please take a look at DeactivateCommand#prepareConfirmation and its
>>>>>>>> usages.
>>>>>>>>> It seems to me, this mode has the same aim as the forceDeactivation
>>>>>> flag.
>>>>>>>>> We can change the message returned by
>>>>>>>> DeactivateCommand#confirmationPrompt
>>>>>>>>> as follows:
>>>>>>>>> "Warning: the command will deactivate the cluster nnn and
>>>> clear
>>>>>>>>> in-memory caches (without persistence) including system caches."
>>>>>>>>>
>>>>>>>>> What do you think?
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> S.
>>>>>>>>>
>>>>>>>>> вт, 24 мар. 2020 г. в 13:07, Nikolay Izhikov <nizhikov@apache.org
>>> :
>>>>>>>>>> Hello, Slava.
>>>>>>>>>>
>>>>>>>>>> Are you talking about this commit [1] (sorry for commit message
>> it’s
>>>>>> due
>>>>>>>>>> to the Github issue)?
>>>>>>>>>>
>>>>>>>>>> The message for this command for now
>>>>>>>>>>
>>>>>>>>>> «Deactivation stopped. Deactivation clears in-memory caches
>> (without
>>>>>>>>>> persistence) including the system caches.»
>>>>>>>>>>
>>>>>>>>>> Is it clear enough?
>>>>>>>>>>
>>>>>>>>>> [1]
>>>>>>>>>>
>> https://github.com/apache/ignite/commit/4921fcf1fecbd8a1ab02099e09cc2adb0b3ff88a
>>>>>>>>>>> 24 марта 2020 г., в 13:02, Вячеслав Коптилин <
>>>>>> slava.koptilin@gmail.com
>>>>>>>>>> написал(а):
>>>>>>>>>>> Hi Nikolay,
>>>>>>>>>>>
>>>>>>>>>>>> 1. We should add —force flag to the command.sh deactivation
>>>> command.
>>>>>>>>>>> I just checked and it seems that the deactivation command
>>>>>>>>>>> (control-utility.sh) already has a confirmation option.
>>>>>>>>>>> Perhaps, we need to clearly state the consequences of using this
>>>>>>>> command
>>>>>>>>>>> with in-memory caches.
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> S.
>>>>>>>>>>>
>>>>>>>>>>> вт, 24 мар. 2020 г. в 12:51, Nikolay Izhikov <
>> nizhikov@apache.org
>>>>> :
>>>>>>>>>>>> Hello, Alexey.
>>>>>>>>>>>>
>>>>>>>>>>>> I just repeat our agreement to be on the same page
>>>>>>>>>>>>
>>>>>>>>>>>>> The confirmation should only present in the user-facing
>>>> interfaces.
>>>>>>>>>>>> 1. We should add —force flag to the command.sh deactivation
>>>> command.
>>>>>>>>>>>> 2. We should throw the exception if cluster has in-memory caches
>>>> and
>>>>>>>>>>>> —force=false.
>>>>>>>>>>>> 3. We shouldn’t change Java API for deactivation.
>>>>>>>>>>>>
>>>>>>>>>>>> Is it correct?
>>>>>>>>>>>>
>>>>>>>>>>>>> The DROP TABLE command does not have a "yes I am sure" clause
>> in
>>>> it
>>>>>>>>>>>> I think it because the command itself has a «DROP» word in it’s
>>>>>> name.
>>>>>>>>>>>> Which clearly explains what will happen on it’s execution.
>>>>>>>>>>>>
>>>>>>>>>>>> On the opposite «deactivation» command doesn’t have any sign of
>>>>>>>>>>>> destructive operation.
>>>>>>>>>>>> That’s why we should warn user about it’s consequences.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>> 24 марта 2020 г., в 12:38, Alexey Goncharuk <
>>>>>>>>>> alexey.goncharuk@gmail.com>
>>>>>>>>>>>> написал(а):
>>>>>>>>>>>>> Igniters, Ivan, Nikolay,
>>>>>>>>>>>>>
>>>>>>>>>>>>> I am strongly against adding confirmation flags to any kind of
>>>>>> APIs,
>>>>>>>>>>>>> whether we change the deactivation behavior or not (even
>> though I
>>>>>>>> agree
>>>>>>>>>>>>> that it makes sense to fix the deactivation to not clean up the
>>>>>>>>>> in-memory
>>>>>>>>>>>>> data). The confirmation should only present in the user-facing
>>>>>>>>>>>> interfaces.
>>>>>>>>>>>>> I cannot recall any software interface which has such a flag.
>>>> None
>>>>>> of
>>>>>>>>>> the
>>>>>>>>>>>>> syscalls which delete files (a very destructive operation) have
>>>>>> this
>>>>>>>>>>>> flag.
>>>>>>>>>>>>> The DROP TABLE command does not have a "yes I am sure" clause
>> in
>>>>>> it.
>>>>>>>>>> As I
>>>>>>>>>>>>> already mentioned, when used programmatically, most users will
>>>>>> likely
>>>>>>>>>>>>> simply pass 'true' as the new flag because they already know
>> the
>>>>>>>>>>>> behavior.
>>>>>>>>>>>>> This is a clear sign of a bad design choice.
>>>>>>>>>>>>>
>>>>>>>>>>>>> On top of that, given that it is our intention to change the
>>>>>> behavior
>>>>>>>>>> of
>>>>>>>>>>>>> deactivation to not loose the in-memory data, it does not make
>>>>>> sense
>>>>>>>> to
>>>>>>>>>>>> me
>>>>>>>>>>>>> to change the API.
Re: Data vanished from cluster after INACTIVE/ACTIVE switch
Posted by Вячеслав Коптилин <sl...@gmail.com>.
Hi Vladimir,
> There are about 15 places in inner logic with this description.
> I propose balance between code base size and comment completeness.
I agree with Iva and I also think that this approach is not so good.
Perhaps we can add just a link to the one method which will provide a
comprehensive description, something like as follows
@param forceDeactivation {@code true} if cluster deactivation should be
forced. Please take a look at {@link IgniteCluster#state(ClusterState
newState, boolean force)} for the details.
What do you think?
Thanks,
Slava.
чт, 2 апр. 2020 г. в 18:47, Vladimir Steshin <vl...@gmail.com>:
> Ivan, hello.
>
> Thanks. I vote for keeping the comments as they are now :)
>
> Igniters, it seems we are agreed to merge [1]. And the ticked s to be
> reverted in future with new designed solution of keeping in-memory data
> after deactivation.
>
> Right?
>
>
> [1] https://issues.apache.org/jira/browse/IGNITE-12779
>
>
> 01.04.2020 20:20, Ivan Rakov пишет:
> > I don't think that making javadocs more descriptive can be considered as
> > harmful code base enlargement.
> > I'd recommend to extend the docs, but the last word is yours ;)
> >
> > On Tue, Mar 31, 2020 at 2:44 PM Vladimir Steshin <vl...@gmail.com>
> wrote:
> >
> >> Ivan, hi.
> >>
> >> I absolutely agree this particular description is not enough to see the
> >> deactivation issue. I also vote for brief code.
> >>
> >> There are about 15 places in inner logic with this description. I
> >> propose balance between code base size and comment completeness.
> >>
> >> Should we enlarge code even if we got several full descriptions?
> >>
> >>
> >> 30.03.2020 20:02, Ivan Rakov пишет:
> >>> Vladimir,
> >>>
> >>> @param forceDeactivation If {@code true}, cluster deactivation will be
> >>>> forced.
> >>> It's true that it's possible to infer semantics of forced deactivation
> >> from
> >>> other parts of API. I just wanted to highlight that exactly this
> >>> description explains something that can be guessed by the parameter
> name.
> >>> I suppose to shorten the lookup path and shed a light on deactivation
> >>> semantics a bit:
> >>>
> >>>> @param forceDeactivation If {@code true}, cluster will be deactivated
> >> even
> >>>> if running in-memory caches are present. All data in the corresponding
> >>>> caches will be vanished as a result.
> >>> Does this make sense?
> >>>
> >>> On Fri, Mar 27, 2020 at 12:00 PM Vladimir Steshin <vl...@gmail.com>
> >>> wrote:
> >>>
> >>>> Ivan, hi.
> >>>>
> >>>>
> >>>> 1) >>> Is it correct? If we are on the same page, let's proceed this
> way
> >>>>
> >>>> It is correct.
> >>>>
> >>>>
> >>>> 2) - In many places in the code I can see the following javadoc
> >>>>
> >>>>> @param forceDeactivation If {@code true}, cluster deactivation
> will
> >> be
> >>>> forced.
> >>>>
> >>>> In the internal params/flags. You can also find /@see
> >>>> ClusterState#INACTIVE/ and full description with several public APIs (
> >>>> like /Ignite.active(boolean)/ ):
> >>>>
> >>>> //
> >>>>
> >>>> /* <p>/
> >>>>
> >>>> //
> >>>>
> >>>> /* <b>NOTE:</b>/
> >>>>
> >>>> //
> >>>>
> >>>> /* Deactivation clears in-memory caches (without persistence)
> including
> >>>> the system caches./
> >>>>
> >>>> Should be enough. Is not?
> >>>>
> >>>>
> >>>> 27.03.2020 10:51, Ivan Rakov пишет:
> >>>>> Vladimir, Igniters,
> >>>>>
> >>>>> Let's emphasize our final plan.
> >>>>>
> >>>>> We are going to add --force flags that will be necessary to pass for
> a
> >>>>> deactivation if there are in-memory caches to:
> >>>>> 1) Rest API (already implemented in [1])
> >>>>> 2) Command line utility (already implemented in [1])
> >>>>> 3) JMX bean (going to be implemented in [2])
> >>>>> We are *not* going to change IgniteCluster or any other thick Java
> API,
> >>>>> thus we are *not* going to merge [3].
> >>>>> We plan to *fully rollback* [1] and [2] once cache data survival
> after
> >>>>> activation-deactivation cycle will be implemented.
> >>>>>
> >>>>> Is it correct? If we are on the same page, let's proceed this way.
> >>>>> I propose to:
> >>>>> - Create a JIRA issue for in-memory-data-safe deactivation (possibly,
> >>>>> without IEP and detailed design so far)
> >>>>> - Describe in the issue description what exact parts of API should be
> >>>>> removed under the issue scope.
> >>>>>
> >>>>> Also, a few questions on already merged [1]:
> >>>>> - We have removed GridClientClusterState#state(ClusterState) from
> Java
> >>>>> client API. Is it a legitimate thing to do? Don't we have to support
> >> API
> >>>>> compatibility for thin clients as well?
> >>>>> - In many places in the code I can see the following javadoc
> >>>>>
> >>>>>> @param forceDeactivation If {@code true}, cluster deactivation
> will
> >>>> be forced.
> >>>>>> As for me, this javadoc doesn't clarify anything. I'd suggest to
> >>>> describe
> >>>>> in which cases deactivation won't happen unless it's forced and which
> >>>>> impact forced deactivation will bring on the system.
> >>>>>
> >>>>> [1]: https://issues.apache.org/jira/browse/IGNITE-12701
> >>>>> [2]: https://issues.apache.org/jira/browse/IGNITE-12779
> >>>>> [3]: https://issues.apache.org/jira/browse/IGNITE-12614
> >>>>>
> >>>>> --
> >>>>> Ivan
> >>>>>
> >>>>> On Tue, Mar 24, 2020 at 7:18 PM Vladimir Steshin <vladsz83@gmail.com
> >
> >>>> wrote:
> >>>>>> Hi, Igniters.
> >>>>>>
> >>>>>> I'd like to remind you that cluster can be deactivated by user with
> 3
> >>>>>> utilities: control.sh, *JMX and the REST*. Proposed in [1] solution
> is
> >>>>>> not about control.sh. It suggests same approach regardless of the
> >>>>>> utility user executes. The task touches *only* *API of the user
> >> calls*,
> >>>>>> not the internal APIs.
> >>>>>>
> >>>>>> The reasons why flag “--yes” and confirmation prompt hasn’t taken
> into
> >>>>>> account for control.sh are:
> >>>>>>
> >>>>>> -Various commands widely use “--yes” just to start. Even not
> dangerous
> >>>>>> ones require “--yes” to begin. “--force” is dedicated for *harmless
> >>>>>> actions*.
> >>>>>>
> >>>>>> -Checking of probable data erasure works after command start and
> >>>>>> “--force” may not be required at all.
> >>>>>>
> >>>>>> -There are also JMX and REST. They have no “—yes” but should work
> >> alike.
> >>>>>> To get the deactivation safe I propose to merge last ticket
> >> with
> >>>>>> the JMX fixes [2]. In future releases, I believe, we should estimate
> >>>>>> jobs and fix memory erasure in general. For now, let’s prevent it.
> >> WDYT?
> >>>>>>
> >>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-12614
> >>>>>>
> >>>>>> [2] https://issues.apache.org/jira/browse/IGNITE-12779
> >>>>>>
> >>>>>>
> >>>>>> 24.03.2020 15:55, Вячеслав Коптилин пишет:
> >>>>>>> Hello Nikolay,
> >>>>>>>
> >>>>>>> I am talking about the interactive mode of the control utility,
> which
> >>>>>>> requires explicit confirmation from the user.
> >>>>>>> Please take a look at DeactivateCommand#prepareConfirmation and its
> >>>>>> usages.
> >>>>>>> It seems to me, this mode has the same aim as the forceDeactivation
> >>>> flag.
> >>>>>>> We can change the message returned by
> >>>>>> DeactivateCommand#confirmationPrompt
> >>>>>>> as follows:
> >>>>>>> "Warning: the command will deactivate the cluster nnn and
> >> clear
> >>>>>>> in-memory caches (without persistence) including system caches."
> >>>>>>>
> >>>>>>> What do you think?
> >>>>>>>
> >>>>>>> Thanks,
> >>>>>>> S.
> >>>>>>>
> >>>>>>> вт, 24 мар. 2020 г. в 13:07, Nikolay Izhikov <nizhikov@apache.org
> >:
> >>>>>>>
> >>>>>>>> Hello, Slava.
> >>>>>>>>
> >>>>>>>> Are you talking about this commit [1] (sorry for commit message
> it’s
> >>>> due
> >>>>>>>> to the Github issue)?
> >>>>>>>>
> >>>>>>>> The message for this command for now
> >>>>>>>>
> >>>>>>>> «Deactivation stopped. Deactivation clears in-memory caches
> (without
> >>>>>>>> persistence) including the system caches.»
> >>>>>>>>
> >>>>>>>> Is it clear enough?
> >>>>>>>>
> >>>>>>>> [1]
> >>>>>>>>
> >>
> https://github.com/apache/ignite/commit/4921fcf1fecbd8a1ab02099e09cc2adb0b3ff88a
> >>>>>>>>> 24 марта 2020 г., в 13:02, Вячеслав Коптилин <
> >>>> slava.koptilin@gmail.com
> >>>>>>>> написал(а):
> >>>>>>>>> Hi Nikolay,
> >>>>>>>>>
> >>>>>>>>>> 1. We should add —force flag to the command.sh deactivation
> >> command.
> >>>>>>>>> I just checked and it seems that the deactivation command
> >>>>>>>>> (control-utility.sh) already has a confirmation option.
> >>>>>>>>> Perhaps, we need to clearly state the consequences of using this
> >>>>>> command
> >>>>>>>>> with in-memory caches.
> >>>>>>>>>
> >>>>>>>>> Thanks,
> >>>>>>>>> S.
> >>>>>>>>>
> >>>>>>>>> вт, 24 мар. 2020 г. в 12:51, Nikolay Izhikov <
> nizhikov@apache.org
> >>> :
> >>>>>>>>>> Hello, Alexey.
> >>>>>>>>>>
> >>>>>>>>>> I just repeat our agreement to be on the same page
> >>>>>>>>>>
> >>>>>>>>>>> The confirmation should only present in the user-facing
> >> interfaces.
> >>>>>>>>>> 1. We should add —force flag to the command.sh deactivation
> >> command.
> >>>>>>>>>> 2. We should throw the exception if cluster has in-memory caches
> >> and
> >>>>>>>>>> —force=false.
> >>>>>>>>>> 3. We shouldn’t change Java API for deactivation.
> >>>>>>>>>>
> >>>>>>>>>> Is it correct?
> >>>>>>>>>>
> >>>>>>>>>>> The DROP TABLE command does not have a "yes I am sure" clause
> in
> >> it
> >>>>>>>>>> I think it because the command itself has a «DROP» word in it’s
> >>>> name.
> >>>>>>>>>> Which clearly explains what will happen on it’s execution.
> >>>>>>>>>>
> >>>>>>>>>> On the opposite «deactivation» command doesn’t have any sign of
> >>>>>>>>>> destructive operation.
> >>>>>>>>>> That’s why we should warn user about it’s consequences.
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>> 24 марта 2020 г., в 12:38, Alexey Goncharuk <
> >>>>>>>> alexey.goncharuk@gmail.com>
> >>>>>>>>>> написал(а):
> >>>>>>>>>>> Igniters, Ivan, Nikolay,
> >>>>>>>>>>>
> >>>>>>>>>>> I am strongly against adding confirmation flags to any kind of
> >>>> APIs,
> >>>>>>>>>>> whether we change the deactivation behavior or not (even
> though I
> >>>>>> agree
> >>>>>>>>>>> that it makes sense to fix the deactivation to not clean up the
> >>>>>>>> in-memory
> >>>>>>>>>>> data). The confirmation should only present in the user-facing
> >>>>>>>>>> interfaces.
> >>>>>>>>>>> I cannot recall any software interface which has such a flag.
> >> None
> >>>> of
> >>>>>>>> the
> >>>>>>>>>>> syscalls which delete files (a very destructive operation) have
> >>>> this
> >>>>>>>>>> flag.
> >>>>>>>>>>> The DROP TABLE command does not have a "yes I am sure" clause
> in
> >>>> it.
> >>>>>>>> As I
> >>>>>>>>>>> already mentioned, when used programmatically, most users will
> >>>> likely
> >>>>>>>>>>> simply pass 'true' as the new flag because they already know
> the
> >>>>>>>>>> behavior.
> >>>>>>>>>>> This is a clear sign of a bad design choice.
> >>>>>>>>>>>
> >>>>>>>>>>> On top of that, given that it is our intention to change the
> >>>> behavior
> >>>>>>>> of
> >>>>>>>>>>> deactivation to not loose the in-memory data, it does not make
> >>>> sense
> >>>>>> to
> >>>>>>>>>> me
> >>>>>>>>>>> to change the API.
>
Re: Data vanished from cluster after INACTIVE/ACTIVE switch
Posted by Vladimir Steshin <vl...@gmail.com>.
Ivan, hello.
Thanks. I vote for keeping the comments as they are now :)
Igniters, it seems we are agreed to merge [1]. And the ticked s to be
reverted in future with new designed solution of keeping in-memory data
after deactivation.
Right?
[1] https://issues.apache.org/jira/browse/IGNITE-12779
01.04.2020 20:20, Ivan Rakov пишет:
> I don't think that making javadocs more descriptive can be considered as
> harmful code base enlargement.
> I'd recommend to extend the docs, but the last word is yours ;)
>
> On Tue, Mar 31, 2020 at 2:44 PM Vladimir Steshin <vl...@gmail.com> wrote:
>
>> Ivan, hi.
>>
>> I absolutely agree this particular description is not enough to see the
>> deactivation issue. I also vote for brief code.
>>
>> There are about 15 places in inner logic with this description. I
>> propose balance between code base size and comment completeness.
>>
>> Should we enlarge code even if we got several full descriptions?
>>
>>
>> 30.03.2020 20:02, Ivan Rakov пишет:
>>> Vladimir,
>>>
>>> @param forceDeactivation If {@code true}, cluster deactivation will be
>>>> forced.
>>> It's true that it's possible to infer semantics of forced deactivation
>> from
>>> other parts of API. I just wanted to highlight that exactly this
>>> description explains something that can be guessed by the parameter name.
>>> I suppose to shorten the lookup path and shed a light on deactivation
>>> semantics a bit:
>>>
>>>> @param forceDeactivation If {@code true}, cluster will be deactivated
>> even
>>>> if running in-memory caches are present. All data in the corresponding
>>>> caches will be vanished as a result.
>>> Does this make sense?
>>>
>>> On Fri, Mar 27, 2020 at 12:00 PM Vladimir Steshin <vl...@gmail.com>
>>> wrote:
>>>
>>>> Ivan, hi.
>>>>
>>>>
>>>> 1) >>> Is it correct? If we are on the same page, let's proceed this way
>>>>
>>>> It is correct.
>>>>
>>>>
>>>> 2) - In many places in the code I can see the following javadoc
>>>>
>>>>> @param forceDeactivation If {@code true}, cluster deactivation will
>> be
>>>> forced.
>>>>
>>>> In the internal params/flags. You can also find /@see
>>>> ClusterState#INACTIVE/ and full description with several public APIs (
>>>> like /Ignite.active(boolean)/ ):
>>>>
>>>> //
>>>>
>>>> /* <p>/
>>>>
>>>> //
>>>>
>>>> /* <b>NOTE:</b>/
>>>>
>>>> //
>>>>
>>>> /* Deactivation clears in-memory caches (without persistence) including
>>>> the system caches./
>>>>
>>>> Should be enough. Is not?
>>>>
>>>>
>>>> 27.03.2020 10:51, Ivan Rakov пишет:
>>>>> Vladimir, Igniters,
>>>>>
>>>>> Let's emphasize our final plan.
>>>>>
>>>>> We are going to add --force flags that will be necessary to pass for a
>>>>> deactivation if there are in-memory caches to:
>>>>> 1) Rest API (already implemented in [1])
>>>>> 2) Command line utility (already implemented in [1])
>>>>> 3) JMX bean (going to be implemented in [2])
>>>>> We are *not* going to change IgniteCluster or any other thick Java API,
>>>>> thus we are *not* going to merge [3].
>>>>> We plan to *fully rollback* [1] and [2] once cache data survival after
>>>>> activation-deactivation cycle will be implemented.
>>>>>
>>>>> Is it correct? If we are on the same page, let's proceed this way.
>>>>> I propose to:
>>>>> - Create a JIRA issue for in-memory-data-safe deactivation (possibly,
>>>>> without IEP and detailed design so far)
>>>>> - Describe in the issue description what exact parts of API should be
>>>>> removed under the issue scope.
>>>>>
>>>>> Also, a few questions on already merged [1]:
>>>>> - We have removed GridClientClusterState#state(ClusterState) from Java
>>>>> client API. Is it a legitimate thing to do? Don't we have to support
>> API
>>>>> compatibility for thin clients as well?
>>>>> - In many places in the code I can see the following javadoc
>>>>>
>>>>>> @param forceDeactivation If {@code true}, cluster deactivation will
>>>> be forced.
>>>>>> As for me, this javadoc doesn't clarify anything. I'd suggest to
>>>> describe
>>>>> in which cases deactivation won't happen unless it's forced and which
>>>>> impact forced deactivation will bring on the system.
>>>>>
>>>>> [1]: https://issues.apache.org/jira/browse/IGNITE-12701
>>>>> [2]: https://issues.apache.org/jira/browse/IGNITE-12779
>>>>> [3]: https://issues.apache.org/jira/browse/IGNITE-12614
>>>>>
>>>>> --
>>>>> Ivan
>>>>>
>>>>> On Tue, Mar 24, 2020 at 7:18 PM Vladimir Steshin <vl...@gmail.com>
>>>> wrote:
>>>>>> Hi, Igniters.
>>>>>>
>>>>>> I'd like to remind you that cluster can be deactivated by user with 3
>>>>>> utilities: control.sh, *JMX and the REST*. Proposed in [1] solution is
>>>>>> not about control.sh. It suggests same approach regardless of the
>>>>>> utility user executes. The task touches *only* *API of the user
>> calls*,
>>>>>> not the internal APIs.
>>>>>>
>>>>>> The reasons why flag “--yes” and confirmation prompt hasn’t taken into
>>>>>> account for control.sh are:
>>>>>>
>>>>>> -Various commands widely use “--yes” just to start. Even not dangerous
>>>>>> ones require “--yes” to begin. “--force” is dedicated for *harmless
>>>>>> actions*.
>>>>>>
>>>>>> -Checking of probable data erasure works after command start and
>>>>>> “--force” may not be required at all.
>>>>>>
>>>>>> -There are also JMX and REST. They have no “—yes” but should work
>> alike.
>>>>>> To get the deactivation safe I propose to merge last ticket
>> with
>>>>>> the JMX fixes [2]. In future releases, I believe, we should estimate
>>>>>> jobs and fix memory erasure in general. For now, let’s prevent it.
>> WDYT?
>>>>>>
>>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-12614
>>>>>>
>>>>>> [2] https://issues.apache.org/jira/browse/IGNITE-12779
>>>>>>
>>>>>>
>>>>>> 24.03.2020 15:55, Вячеслав Коптилин пишет:
>>>>>>> Hello Nikolay,
>>>>>>>
>>>>>>> I am talking about the interactive mode of the control utility, which
>>>>>>> requires explicit confirmation from the user.
>>>>>>> Please take a look at DeactivateCommand#prepareConfirmation and its
>>>>>> usages.
>>>>>>> It seems to me, this mode has the same aim as the forceDeactivation
>>>> flag.
>>>>>>> We can change the message returned by
>>>>>> DeactivateCommand#confirmationPrompt
>>>>>>> as follows:
>>>>>>> "Warning: the command will deactivate the cluster nnn and
>> clear
>>>>>>> in-memory caches (without persistence) including system caches."
>>>>>>>
>>>>>>> What do you think?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> S.
>>>>>>>
>>>>>>> вт, 24 мар. 2020 г. в 13:07, Nikolay Izhikov <ni...@apache.org>:
>>>>>>>
>>>>>>>> Hello, Slava.
>>>>>>>>
>>>>>>>> Are you talking about this commit [1] (sorry for commit message it’s
>>>> due
>>>>>>>> to the Github issue)?
>>>>>>>>
>>>>>>>> The message for this command for now
>>>>>>>>
>>>>>>>> «Deactivation stopped. Deactivation clears in-memory caches (without
>>>>>>>> persistence) including the system caches.»
>>>>>>>>
>>>>>>>> Is it clear enough?
>>>>>>>>
>>>>>>>> [1]
>>>>>>>>
>> https://github.com/apache/ignite/commit/4921fcf1fecbd8a1ab02099e09cc2adb0b3ff88a
>>>>>>>>> 24 марта 2020 г., в 13:02, Вячеслав Коптилин <
>>>> slava.koptilin@gmail.com
>>>>>>>> написал(а):
>>>>>>>>> Hi Nikolay,
>>>>>>>>>
>>>>>>>>>> 1. We should add —force flag to the command.sh deactivation
>> command.
>>>>>>>>> I just checked and it seems that the deactivation command
>>>>>>>>> (control-utility.sh) already has a confirmation option.
>>>>>>>>> Perhaps, we need to clearly state the consequences of using this
>>>>>> command
>>>>>>>>> with in-memory caches.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> S.
>>>>>>>>>
>>>>>>>>> вт, 24 мар. 2020 г. в 12:51, Nikolay Izhikov <nizhikov@apache.org
>>> :
>>>>>>>>>> Hello, Alexey.
>>>>>>>>>>
>>>>>>>>>> I just repeat our agreement to be on the same page
>>>>>>>>>>
>>>>>>>>>>> The confirmation should only present in the user-facing
>> interfaces.
>>>>>>>>>> 1. We should add —force flag to the command.sh deactivation
>> command.
>>>>>>>>>> 2. We should throw the exception if cluster has in-memory caches
>> and
>>>>>>>>>> —force=false.
>>>>>>>>>> 3. We shouldn’t change Java API for deactivation.
>>>>>>>>>>
>>>>>>>>>> Is it correct?
>>>>>>>>>>
>>>>>>>>>>> The DROP TABLE command does not have a "yes I am sure" clause in
>> it
>>>>>>>>>> I think it because the command itself has a «DROP» word in it’s
>>>> name.
>>>>>>>>>> Which clearly explains what will happen on it’s execution.
>>>>>>>>>>
>>>>>>>>>> On the opposite «deactivation» command doesn’t have any sign of
>>>>>>>>>> destructive operation.
>>>>>>>>>> That’s why we should warn user about it’s consequences.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> 24 марта 2020 г., в 12:38, Alexey Goncharuk <
>>>>>>>> alexey.goncharuk@gmail.com>
>>>>>>>>>> написал(а):
>>>>>>>>>>> Igniters, Ivan, Nikolay,
>>>>>>>>>>>
>>>>>>>>>>> I am strongly against adding confirmation flags to any kind of
>>>> APIs,
>>>>>>>>>>> whether we change the deactivation behavior or not (even though I
>>>>>> agree
>>>>>>>>>>> that it makes sense to fix the deactivation to not clean up the
>>>>>>>> in-memory
>>>>>>>>>>> data). The confirmation should only present in the user-facing
>>>>>>>>>> interfaces.
>>>>>>>>>>> I cannot recall any software interface which has such a flag.
>> None
>>>> of
>>>>>>>> the
>>>>>>>>>>> syscalls which delete files (a very destructive operation) have
>>>> this
>>>>>>>>>> flag.
>>>>>>>>>>> The DROP TABLE command does not have a "yes I am sure" clause in
>>>> it.
>>>>>>>> As I
>>>>>>>>>>> already mentioned, when used programmatically, most users will
>>>> likely
>>>>>>>>>>> simply pass 'true' as the new flag because they already know the
>>>>>>>>>> behavior.
>>>>>>>>>>> This is a clear sign of a bad design choice.
>>>>>>>>>>>
>>>>>>>>>>> On top of that, given that it is our intention to change the
>>>> behavior
>>>>>>>> of
>>>>>>>>>>> deactivation to not loose the in-memory data, it does not make
>>>> sense
>>>>>> to
>>>>>>>>>> me
>>>>>>>>>>> to change the API.