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 <vlads...@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 <vlads...@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 <vlads...@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 <
> vlads...@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 <
> vlads...@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 <
> nizhi...@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.kopti...@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 <
> >> nizhi...@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.goncha...@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.
>
