Data vanished from cluster after INACTIVE/ACTIVE switch

[Ivan Rakov]

>
> I think the only question is - Do we need —force flag in Java API or not.

 From my perspective, there's also no agreement that it should be present
in the thin clients' API. For instance, I think it shouldn't.

As far as I know, IGNITE_REUSE_MEMORY_ON_DEACTIVATE is for *other* purpose.
> Can you provide a simple reproducer when in-memory data not cleared on
> deactivation?

 Preserving in-memory data isn't implemented so far, so I can't provide a
reproducer. My point is that we are halfway through it: we can build a
solution based on IGNITE_REUSE_MEMORY_ON_DEACTIVATE and additional logic
with reusing memory pages.

For me, the ultimate value of Ignite into real production environment is
> user data.
> If we have some cases when data is lost - we should avoid it as hard as we
> can.
>
> So, for now, this flag required.

Totally agree that sudden vanishing of user data is unacceptable. But I
don't see how it implies that we have to solve this issue by tangling
public API. If we see that system behaves incorrectly, I believe we should
fix the issue instead of adapting API to temporary flaws. I think that
clear description of active(false) impact in the documentation is more than
enough: on the one hand, if user didn't read documentation for the method
he calls, he can't complain about the consequences; on the other hand, if
user decided to deactivate the cluster for no matter what, -force flag will
barely stop him.
We anyway have enough time before 2.9 to implement a proper solution.

To sum it up, the question is whether we should reflect temporary system
design flaws in the API. I think, we surely shouldn't: API certainly lives
longer and is not intended to collect workarounds for all bugs that are
already fixed or planned to be fixed.
We can collect more opinions on this.

On Tue, Mar 24, 2020 at 10:22 AM Nikolay Izhikov <[hidden email]>
wrote:

> Alexey.
>
> Having the way to silently vanish user data is even worse.
> So I’m strictly against removing —force flag.
>
> > 24 марта 2020 г., в 10:16, Alexei Scherbakov <
> [hidden email]> написал(а):
> >
> > Nikolay,
> >
> > I'm on the same page with Ivan.
> >
> > Having "force" flag in public API as preposterous as having it in
> > System.exit.
> > For me it looks like badly designed API.
> > If a call to some method is dangerous it should be clearly specified in
> the
> > javadoc.
> > I'm also against some "temporary" API.
> >
> > We should:
> >
> > 1. Partially remove IGNITE-12701 except javadoc part. Note control.sh
> for a
> > long time has support for a confirmation on deactivation (interactive
> mode).
> > 2. IGNITE_REUSE_MEMORY_ON_DEACTIVATE=true already preserves memory
> content
> > after deactivation. We should start working on restoring page memory
> state
> > after subsequent reactivation.
> > 3. Describe current behavior for in-memory cache on deactivation in
> Ignite
> > documentation.
> >
> >
> > пн, 23 мар. 2020 г. в 21:22, Nikolay Izhikov <[hidden email]>:
> >
> >> Hello, Ivan.
> >>
> >>> Seems like we don't have a final agreement on whether we should add
> force
> >> flag to deactivation API.
> >>
> >> I think the only question is - Do we need —force flag in Java API or
> not.
> >>
> >>
> >>> As a final solution, I'd want to see behavior when all in-memory data
> is
> >> available after deactivation and further activation.
> >>
> >> Agree.
> >>
> >>> I believe it’s possible to don't deallocate memory
> >>> (like mentioned before, we already can achieve that with
> >> IGNITE_REUSE_MEMORY_ON_DEACTIVATE=true) and carefully reuse all loaded
> data
> >> pages on next activation and caches start.
> >>
> >> As far as I know, IGNITE_REUSE_MEMORY_ON_DEACTIVATE is for *other*
> purpose.
> >> Can you provide a simple reproducer when in-memory data not cleared on
> >> deactivation?
> >>
> >>> Considering this, do we really need to introduce force flag as a
> >> temporary precaution?
> >>
> >> My answer is yes we need it.
> >> Right now, we can’t prevent data loss on deactivation for in-memory
> caches.
> >>
> >> For me, the ultimate value of Ignite into real production environment is
> >> user data.
> >> If we have some cases when data is lost - we should avoid it as hard as
> we
> >> can.
> >>
> >> So, for now, this flag required.
> >>
> >>> I suggest to rollback [2] from AI master, stop working on [1] and focus
> >> on how to implement keeping in-memory data after the deactivation.
> >>
> >> I think we can remove —force flag only after implementation of keeping
> >> in-memory data on deactivation would be finished.
> >> If that happens before 2.9 then we can have clearer API.
> >>
> >>> 23 марта 2020 г., в 18:47, Ivan Rakov <[hidden email]>
> >> написал(а):
> >>>
> >>> Folks,
> >>>
> >>> Let's revive this discussion until it's too late and all API changes
> are
> >>> merged to master [1].
> >>> Seems like we don't have a final agreement on whether we should add
> force
> >>> flag to deactivation API.
> >>>
> >>> First of all, I think we are all on the same page that in-memory caches
> >>> data vanishing on deactivation is counter-intuitive and dangerous. As a
> >>> final solution, I'd want to see behavior when all in-memory data is
> >>> available after deactivation and further activation. I believe it's
> >>> possible to don't deallocate memory (like mentioned before, we already
> >> can
> >>> achieve that with IGNITE_REUSE_MEMORY_ON_DEACTIVATE=true) and carefully
> >>> reuse all loaded data pages on next activation and caches start.
> >>>
> >>> Also, this is a wider question, but: do we understand what cluster
> >>> deactivation is actually intended for? I can only think of two cases:
> >>> - graceful cluster shutdown: an ability to cut checkpoints and to end
> >>> transactional load consistently prior to further stop of all nodes
> >>> - blocking all API (both reads and writes) due to some maintenance
> >>> Neither of them requires forcefully clearing all in-memory data on
> >>> deactivation. If everyone agrees, from now on we should assume data
> >>> clearing as system design flaw that should be fixed, not as possible
> >>> scenario which we should support on the API level.
> >>>
> >>> Considering this, do we really need to introduce force flag as a
> >> temporary
> >>> precaution? I have at least two reasons against it:
> >>> 1) Once API was changed and released, we have to support it until the
> >> next
> >>> major release. If we all understand that data vanishing issue is
> >> fixable, I
> >>> believe we shouldn't engrave in the API flags that will become
> pointless.
> >>> 2) More personal one, but I'm against any force flags in the API. This
> >>> makes API harder to understand; more than that, the presence of such
> >> flags
> >>> just highlights that API is poorly designed.
> >>>
> >>> I suggest to rollback [2] from AI master, stop working on [1] and focus
> >> on
> >>> how to implement keeping in-memory data after the deactivation.
> >>> I think we can still require user consent for deactivation via
> control.sh
> >>> (it already requires --yes) and JMX.
> >>>
> >>> Thoughts?
> >>>
> >>> [1]: https://issues.apache.org/jira/browse/IGNITE-12614
> >>> [2]: https://issues.apache.org/jira/browse/IGNITE-12701
> >>>
> >>> --
> >>> Ivan
> >>>
> >>>
> >>> On Tue, Mar 17, 2020 at 2:26 PM Vladimir Steshin <[hidden email]>
> >> wrote:
> >>>
> >>>> Nikolay, I think we should reconsider clearing at least system caches
> >>>> when deactivating.
> >>>>
> >>>> 17.03.2020 14:18, Nikolay Izhikov пишет:
> >>>>> Hello, Vladimir.
> >>>>>
> >>>>> I don’t get it.
> >>>>>
> >>>>> What is your proposal?
> >>>>> What we should do?
> >>>>>
> >>>>>> 17 марта 2020 г., в 14:11, Vladimir Steshin <[hidden email]>
> >>>> написал(а):
> >>>>>>
> >>>>>> Nikolay, hi.
> >>>>>>
> >>>>>>>>> And should be covered with the  —force parameter we added.
> >>>>>> As fix for user cases - yes. My idea is to emphasize overall ability
> >> to
> >>>> lose various objects, not only data. Probably might be reconsidered in
> >>>> future.
> >>>>>>
> >>>>>>
> >>>>>> 17.03.2020 13:49, Nikolay Izhikov пишет:
> >>>>>>> Hello, Vladimir.
> >>>>>>>
> >>>>>>> If there is at lease one persistent data region then system data
> >>>> region also becomes persistent.
> >>>>>>> Your example applies only to pure in-memory clusters.
> >>>>>>>
> >>>>>>> And should be covered with the —force parameter we added.
> >>>>>>>
> >>>>>>> What do you think?
> >>>>>>>
> >>>>>>>> 17 марта 2020 г., в 13:45, Vladimir Steshin <[hidden email]>
> >>>> написал(а):
> >>>>>>>>
> >>>>>>>>    Hi, all.
> >>>>>>>>
> >>>>>>>> Fixes for control.sh and the REST have been merged. Could anyone
> >> take
> >>>> a look to the previous email with an issue? Isn't this conductvery
> >> wierd?
> >>>>>>>>
> >>>>
> >>
> >>
> >
> > --
> >
> > Best regards,
> > Alexei Scherbakov
>
>

[Nikolay Izhikov-2]

Hello, Ivan.

> I believe we should fix the issue instead of adapting API to temporary flaws.

Agree. Let’s fix it.

>  I think that clear description of active(false) impact in the documentation is more than enough

I can’t agree with this point.

We shouldn’t imply the assumption that every user reads the whole documentation and completely understand the consequences of the
deactivation command.

This whole thread shows that even active core developers don't understand it.

So my proposal is to remove --force flag only after we fix deactivation.

> To sum it up, the question is whether we should reflect temporary system design flaws in the API

I can’t agree with the «temporary» design.
We have neither design nor IEP or contributor who can fix current behavior.
And, if I understand Alexey Goncharyuk correctly, current behavior was implemented intentionally.

So, my understanding that current implementation would be here for a while.
And after we fix it I totally support removing —force flag.

> 24 марта 2020 г., в 12:06, Ivan Rakov <[hidden email]> написал(а):
>
>>
>> I think the only question is - Do we need —force flag in Java API or not.
>
> From my perspective, there's also no agreement that it should be present
> in the thin clients' API. For instance, I think it shouldn't.
>
> As far as I know, IGNITE_REUSE_MEMORY_ON_DEACTIVATE is for *other* purpose.
>> Can you provide a simple reproducer when in-memory data not cleared on
>> deactivation?
>
> Preserving in-memory data isn't implemented so far, so I can't provide a
> reproducer. My point is that we are halfway through it: we can build a
> solution based on IGNITE_REUSE_MEMORY_ON_DEACTIVATE and additional logic
> with reusing memory pages.
>
> For me, the ultimate value of Ignite into real production environment is
>> user data.
>> If we have some cases when data is lost - we should avoid it as hard as we
>> can.
>>
>> So, for now, this flag required.
>
> Totally agree that sudden vanishing of user data is unacceptable. But I
> don't see how it implies that we have to solve this issue by tangling
> public API. If we see that system behaves incorrectly, I believe we should
> fix the issue instead of adapting API to temporary flaws. I think that
> clear description of active(false) impact in the documentation is more than
> enough: on the one hand, if user didn't read documentation for the method
> he calls, he can't complain about the consequences; on the other hand, if
> user decided to deactivate the cluster for no matter what, -force flag will
> barely stop him.
> We anyway have enough time before 2.9 to implement a proper solution.
>
> To sum it up, the question is whether we should reflect temporary system
> design flaws in the API. I think, we surely shouldn't: API certainly lives
> longer and is not intended to collect workarounds for all bugs that are
> already fixed or planned to be fixed.
> We can collect more opinions on this.
>
> On Tue, Mar 24, 2020 at 10:22 AM Nikolay Izhikov <[hidden email]>
> wrote:
>
>> Alexey.
>>
>> Having the way to silently vanish user data is even worse.
>> So I’m strictly against removing —force flag.
>>
>>> 24 марта 2020 г., в 10:16, Alexei Scherbakov <
>> [hidden email]> написал(а):
>>>
>>> Nikolay,
>>>
>>> I'm on the same page with Ivan.
>>>
>>> Having "force" flag in public API as preposterous as having it in
>>> System.exit.
>>> For me it looks like badly designed API.
>>> If a call to some method is dangerous it should be clearly specified in
>> the
>>> javadoc.
>>> I'm also against some "temporary" API.
>>>
>>> We should:
>>>
>>> 1. Partially remove IGNITE-12701 except javadoc part. Note control.sh
>> for a
>>> long time has support for a confirmation on deactivation (interactive
>> mode).
>>> 2. IGNITE_REUSE_MEMORY_ON_DEACTIVATE=true already preserves memory
>> content
>>> after deactivation. We should start working on restoring page memory
>> state
>>> after subsequent reactivation.
>>> 3. Describe current behavior for in-memory cache on deactivation in
>> Ignite
>>> documentation.
>>>
>>>
>>> пн, 23 мар. 2020 г. в 21:22, Nikolay Izhikov <[hidden email]>:
>>>
>>>> Hello, Ivan.
>>>>
>>>>> Seems like we don't have a final agreement on whether we should add
>> force
>>>> flag to deactivation API.
>>>>
>>>> I think the only question is - Do we need —force flag in Java API or
>> not.
>>>>
>>>>
>>>>> As a final solution, I'd want to see behavior when all in-memory data
>> is
>>>> available after deactivation and further activation.
>>>>
>>>> Agree.
>>>>
>>>>> I believe it’s possible to don't deallocate memory
>>>>> (like mentioned before, we already can achieve that with
>>>> IGNITE_REUSE_MEMORY_ON_DEACTIVATE=true) and carefully reuse all loaded
>> data
>>>> pages on next activation and caches start.
>>>>
>>>> As far as I know, IGNITE_REUSE_MEMORY_ON_DEACTIVATE is for *other*
>> purpose.
>>>> Can you provide a simple reproducer when in-memory data not cleared on
>>>> deactivation?
>>>>
>>>>> Considering this, do we really need to introduce force flag as a
>>>> temporary precaution?
>>>>
>>>> My answer is yes we need it.
>>>> Right now, we can’t prevent data loss on deactivation for in-memory
>> caches.
>>>>
>>>> For me, the ultimate value of Ignite into real production environment is
>>>> user data.
>>>> If we have some cases when data is lost - we should avoid it as hard as
>> we
>>>> can.
>>>>
>>>> So, for now, this flag required.
>>>>
>>>>> I suggest to rollback [2] from AI master, stop working on [1] and focus
>>>> on how to implement keeping in-memory data after the deactivation.
>>>>
>>>> I think we can remove —force flag only after implementation of keeping
>>>> in-memory data on deactivation would be finished.
>>>> If that happens before 2.9 then we can have clearer API.
>>>>
>>>>> 23 марта 2020 г., в 18:47, Ivan Rakov <[hidden email]>
>>>> написал(а):
>>>>>
>>>>> Folks,
>>>>>
>>>>> Let's revive this discussion until it's too late and all API changes
>> are
>>>>> merged to master [1].
>>>>> Seems like we don't have a final agreement on whether we should add
>> force
>>>>> flag to deactivation API.
>>>>>
>>>>> First of all, I think we are all on the same page that in-memory caches
>>>>> data vanishing on deactivation is counter-intuitive and dangerous. As a
>>>>> final solution, I'd want to see behavior when all in-memory data is
>>>>> available after deactivation and further activation. I believe it's
>>>>> possible to don't deallocate memory (like mentioned before, we already
>>>> can
>>>>> achieve that with IGNITE_REUSE_MEMORY_ON_DEACTIVATE=true) and carefully
>>>>> reuse all loaded data pages on next activation and caches start.
>>>>>
>>>>> Also, this is a wider question, but: do we understand what cluster
>>>>> deactivation is actually intended for? I can only think of two cases:
>>>>> - graceful cluster shutdown: an ability to cut checkpoints and to end
>>>>> transactional load consistently prior to further stop of all nodes
>>>>> - blocking all API (both reads and writes) due to some maintenance
>>>>> Neither of them requires forcefully clearing all in-memory data on
>>>>> deactivation. If everyone agrees, from now on we should assume data
>>>>> clearing as system design flaw that should be fixed, not as possible
>>>>> scenario which we should support on the API level.
>>>>>
>>>>> Considering this, do we really need to introduce force flag as a
>>>> temporary
>>>>> precaution? I have at least two reasons against it:
>>>>> 1) Once API was changed and released, we have to support it until the
>>>> next
>>>>> major release. If we all understand that data vanishing issue is
>>>> fixable, I
>>>>> believe we shouldn't engrave in the API flags that will become
>> pointless.
>>>>> 2) More personal one, but I'm against any force flags in the API. This
>>>>> makes API harder to understand; more than that, the presence of such
>>>> flags
>>>>> just highlights that API is poorly designed.
>>>>>
>>>>> I suggest to rollback [2] from AI master, stop working on [1] and focus
>>>> on
>>>>> how to implement keeping in-memory data after the deactivation.
>>>>> I think we can still require user consent for deactivation via
>> control.sh
>>>>> (it already requires --yes) and JMX.
>>>>>
>>>>> Thoughts?
>>>>>
>>>>> [1]: https://issues.apache.org/jira/browse/IGNITE-12614
>>>>> [2]: https://issues.apache.org/jira/browse/IGNITE-12701
>>>>>
>>>>> --
>>>>> Ivan
>>>>>
>>>>>
>>>>> On Tue, Mar 17, 2020 at 2:26 PM Vladimir Steshin <[hidden email]>
>>>> wrote:
>>>>>
>>>>>> Nikolay, I think we should reconsider clearing at least system caches
>>>>>> when deactivating.
>>>>>>
>>>>>> 17.03.2020 14:18, Nikolay Izhikov пишет:
>>>>>>> Hello, Vladimir.
>>>>>>>
>>>>>>> I don’t get it.
>>>>>>>
>>>>>>> What is your proposal?
>>>>>>> What we should do?
>>>>>>>
>>>>>>>> 17 марта 2020 г., в 14:11, Vladimir Steshin <[hidden email]>
>>>>>> написал(а):
>>>>>>>>
>>>>>>>> Nikolay, hi.
>>>>>>>>
>>>>>>>>>>> And should be covered with the  —force parameter we added.
>>>>>>>> As fix for user cases - yes. My idea is to emphasize overall ability
>>>> to
>>>>>> lose various objects, not only data. Probably might be reconsidered in
>>>>>> future.
>>>>>>>>
>>>>>>>>
>>>>>>>> 17.03.2020 13:49, Nikolay Izhikov пишет:
>>>>>>>>> Hello, Vladimir.
>>>>>>>>>
>>>>>>>>> If there is at lease one persistent data region then system data
>>>>>> region also becomes persistent.
>>>>>>>>> Your example applies only to pure in-memory clusters.
>>>>>>>>>
>>>>>>>>> And should be covered with the —force parameter we added.
>>>>>>>>>
>>>>>>>>> What do you think?
>>>>>>>>>
>>>>>>>>>> 17 марта 2020 г., в 13:45, Vladimir Steshin <[hidden email]>
>>>>>> написал(а):
>>>>>>>>>>
>>>>>>>>>>   Hi, all.
>>>>>>>>>>
>>>>>>>>>> Fixes for control.sh and the REST have been merged. Could anyone
>>>> take
>>>>>> a look to the previous email with an issue? Isn't this conductvery
>>>> wierd?
>>>>>>>>>>
>>>>>>
>>>>
>>>>
>>>
>>> --
>>>
>>> Best regards,
>>> Alexei Scherbakov
>>
>>

[Ivan Rakov]

>
> I can’t agree with the «temporary» design.
> We have neither design nor IEP or contributor who can fix current behavior.
> And, if I understand Alexey Goncharyuk correctly, current behavior was
> implemented intentionally.

 Alex, what do you think? Are we on the same page that desired behavior for
the deactivation is to keep data of all in-memory caches, even though it
was intentionally implemented in 2.0 another way?

On Tue, Mar 24, 2020 at 12:21 PM Nikolay Izhikov <[hidden email]>
wrote:

> Hello, Ivan.
>
> > I believe we should fix the issue instead of adapting API to temporary
> flaws.
>
> Agree. Let’s fix it.
>
> >  I think that clear description of active(false) impact in the
> documentation is more than enough
>
> I can’t agree with this point.
>
> We shouldn’t imply the assumption that every user reads the whole
> documentation and completely understand the consequences of the
> deactivation command.
>
> This whole thread shows that even active core developers don't understand
> it.
>
> So my proposal is to remove --force flag only after we fix deactivation.
>
> > To sum it up, the question is whether we should reflect temporary system
> design flaws in the API
>
> I can’t agree with the «temporary» design.
> We have neither design nor IEP or contributor who can fix current behavior.
> And, if I understand Alexey Goncharyuk correctly, current behavior was
> implemented intentionally.
>
> So, my understanding that current implementation would be here for a while.
> And after we fix it I totally support removing —force flag.
>
> > 24 марта 2020 г., в 12:06, Ivan Rakov <[hidden email]>
> написал(а):
> >
> >>
> >> I think the only question is - Do we need —force flag in Java API or
> not.
> >
> > From my perspective, there's also no agreement that it should be present
> > in the thin clients' API. For instance, I think it shouldn't.
> >
> > As far as I know, IGNITE_REUSE_MEMORY_ON_DEACTIVATE is for *other*
> purpose.
> >> Can you provide a simple reproducer when in-memory data not cleared on
> >> deactivation?
> >
> > Preserving in-memory data isn't implemented so far, so I can't provide a
> > reproducer. My point is that we are halfway through it: we can build a
> > solution based on IGNITE_REUSE_MEMORY_ON_DEACTIVATE and additional logic
> > with reusing memory pages.
> >
> > For me, the ultimate value of Ignite into real production environment is
> >> user data.
> >> If we have some cases when data is lost - we should avoid it as hard as
> we
> >> can.
> >>
> >> So, for now, this flag required.
> >
> > Totally agree that sudden vanishing of user data is unacceptable. But I
> > don't see how it implies that we have to solve this issue by tangling
> > public API. If we see that system behaves incorrectly, I believe we
> should
> > fix the issue instead of adapting API to temporary flaws. I think that
> > clear description of active(false) impact in the documentation is more
> than
> > enough: on the one hand, if user didn't read documentation for the method
> > he calls, he can't complain about the consequences; on the other hand, if
> > user decided to deactivate the cluster for no matter what, -force flag
> will
> > barely stop him.
> > We anyway have enough time before 2.9 to implement a proper solution.
> >
> > To sum it up, the question is whether we should reflect temporary system
> > design flaws in the API. I think, we surely shouldn't: API certainly
> lives
> > longer and is not intended to collect workarounds for all bugs that are
> > already fixed or planned to be fixed.
> > We can collect more opinions on this.
> >
> > On Tue, Mar 24, 2020 at 10:22 AM Nikolay Izhikov <[hidden email]>
> > wrote:
> >
> >> Alexey.
> >>
> >> Having the way to silently vanish user data is even worse.
> >> So I’m strictly against removing —force flag.
> >>
> >>> 24 марта 2020 г., в 10:16, Alexei Scherbakov <
> >> [hidden email]> написал(а):
> >>>
> >>> Nikolay,
> >>>
> >>> I'm on the same page with Ivan.
> >>>
> >>> Having "force" flag in public API as preposterous as having it in
> >>> System.exit.
> >>> For me it looks like badly designed API.
> >>> If a call to some method is dangerous it should be clearly specified in
> >> the
> >>> javadoc.
> >>> I'm also against some "temporary" API.
> >>>
> >>> We should:
> >>>
> >>> 1. Partially remove IGNITE-12701 except javadoc part. Note control.sh
> >> for a
> >>> long time has support for a confirmation on deactivation (interactive
> >> mode).
> >>> 2. IGNITE_REUSE_MEMORY_ON_DEACTIVATE=true already preserves memory
> >> content
> >>> after deactivation. We should start working on restoring page memory
> >> state
> >>> after subsequent reactivation.
> >>> 3. Describe current behavior for in-memory cache on deactivation in
> >> Ignite
> >>> documentation.
> >>>
> >>>
> >>> пн, 23 мар. 2020 г. в 21:22, Nikolay Izhikov <[hidden email]>:
> >>>
> >>>> Hello, Ivan.
> >>>>
> >>>>> Seems like we don't have a final agreement on whether we should add
> >> force
> >>>> flag to deactivation API.
> >>>>
> >>>> I think the only question is - Do we need —force flag in Java API or
> >> not.
> >>>>
> >>>>
> >>>>> As a final solution, I'd want to see behavior when all in-memory data
> >> is
> >>>> available after deactivation and further activation.
> >>>>
> >>>> Agree.
> >>>>
> >>>>> I believe it’s possible to don't deallocate memory
> >>>>> (like mentioned before, we already can achieve that with
> >>>> IGNITE_REUSE_MEMORY_ON_DEACTIVATE=true) and carefully reuse all loaded
> >> data
> >>>> pages on next activation and caches start.
> >>>>
> >>>> As far as I know, IGNITE_REUSE_MEMORY_ON_DEACTIVATE is for *other*
> >> purpose.
> >>>> Can you provide a simple reproducer when in-memory data not cleared on
> >>>> deactivation?
> >>>>
> >>>>> Considering this, do we really need to introduce force flag as a
> >>>> temporary precaution?
> >>>>
> >>>> My answer is yes we need it.
> >>>> Right now, we can’t prevent data loss on deactivation for in-memory
> >> caches.
> >>>>
> >>>> For me, the ultimate value of Ignite into real production environment
> is
> >>>> user data.
> >>>> If we have some cases when data is lost - we should avoid it as hard
> as
> >> we
> >>>> can.
> >>>>
> >>>> So, for now, this flag required.
> >>>>
> >>>>> I suggest to rollback [2] from AI master, stop working on [1] and
> focus
> >>>> on how to implement keeping in-memory data after the deactivation.
> >>>>
> >>>> I think we can remove —force flag only after implementation of keeping
> >>>> in-memory data on deactivation would be finished.
> >>>> If that happens before 2.9 then we can have clearer API.
> >>>>
> >>>>> 23 марта 2020 г., в 18:47, Ivan Rakov <[hidden email]>
> >>>> написал(а):
> >>>>>
> >>>>> Folks,
> >>>>>
> >>>>> Let's revive this discussion until it's too late and all API changes
> >> are
> >>>>> merged to master [1].
> >>>>> Seems like we don't have a final agreement on whether we should add
> >> force
> >>>>> flag to deactivation API.
> >>>>>
> >>>>> First of all, I think we are all on the same page that in-memory
> caches
> >>>>> data vanishing on deactivation is counter-intuitive and dangerous.
> As a
> >>>>> final solution, I'd want to see behavior when all in-memory data is
> >>>>> available after deactivation and further activation. I believe it's
> >>>>> possible to don't deallocate memory (like mentioned before, we
> already
> >>>> can
> >>>>> achieve that with IGNITE_REUSE_MEMORY_ON_DEACTIVATE=true) and
> carefully
> >>>>> reuse all loaded data pages on next activation and caches start.
> >>>>>
> >>>>> Also, this is a wider question, but: do we understand what cluster
> >>>>> deactivation is actually intended for? I can only think of two cases:
> >>>>> - graceful cluster shutdown: an ability to cut checkpoints and to end
> >>>>> transactional load consistently prior to further stop of all nodes
> >>>>> - blocking all API (both reads and writes) due to some maintenance
> >>>>> Neither of them requires forcefully clearing all in-memory data on
> >>>>> deactivation. If everyone agrees, from now on we should assume data
> >>>>> clearing as system design flaw that should be fixed, not as possible
> >>>>> scenario which we should support on the API level.
> >>>>>
> >>>>> Considering this, do we really need to introduce force flag as a
> >>>> temporary
> >>>>> precaution? I have at least two reasons against it:
> >>>>> 1) Once API was changed and released, we have to support it until the
> >>>> next
> >>>>> major release. If we all understand that data vanishing issue is
> >>>> fixable, I
> >>>>> believe we shouldn't engrave in the API flags that will become
> >> pointless.
> >>>>> 2) More personal one, but I'm against any force flags in the API.
> This
> >>>>> makes API harder to understand; more than that, the presence of such
> >>>> flags
> >>>>> just highlights that API is poorly designed.
> >>>>>
> >>>>> I suggest to rollback [2] from AI master, stop working on [1] and
> focus
> >>>> on
> >>>>> how to implement keeping in-memory data after the deactivation.
> >>>>> I think we can still require user consent for deactivation via
> >> control.sh
> >>>>> (it already requires --yes) and JMX.
> >>>>>
> >>>>> Thoughts?
> >>>>>
> >>>>> [1]: https://issues.apache.org/jira/browse/IGNITE-12614
> >>>>> [2]: https://issues.apache.org/jira/browse/IGNITE-12701
> >>>>>
> >>>>> --
> >>>>> Ivan
> >>>>>
> >>>>>
> >>>>> On Tue, Mar 17, 2020 at 2:26 PM Vladimir Steshin <[hidden email]
> >
> >>>> wrote:
> >>>>>
> >>>>>> Nikolay, I think we should reconsider clearing at least system
> caches
> >>>>>> when deactivating.
> >>>>>>
> >>>>>> 17.03.2020 14:18, Nikolay Izhikov пишет:
> >>>>>>> Hello, Vladimir.
> >>>>>>>
> >>>>>>> I don’t get it.
> >>>>>>>
> >>>>>>> What is your proposal?
> >>>>>>> What we should do?
> >>>>>>>
> >>>>>>>> 17 марта 2020 г., в 14:11, Vladimir Steshin <[hidden email]>
> >>>>>> написал(а):
> >>>>>>>>
> >>>>>>>> Nikolay, hi.
> >>>>>>>>
> >>>>>>>>>>> And should be covered with the  —force parameter we added.
> >>>>>>>> As fix for user cases - yes. My idea is to emphasize overall
> ability
> >>>> to
> >>>>>> lose various objects, not only data. Probably might be reconsidered
> in
> >>>>>> future.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> 17.03.2020 13:49, Nikolay Izhikov пишет:
> >>>>>>>>> Hello, Vladimir.
> >>>>>>>>>
> >>>>>>>>> If there is at lease one persistent data region then system data
> >>>>>> region also becomes persistent.
> >>>>>>>>> Your example applies only to pure in-memory clusters.
> >>>>>>>>>
> >>>>>>>>> And should be covered with the —force parameter we added.
> >>>>>>>>>
> >>>>>>>>> What do you think?
> >>>>>>>>>
> >>>>>>>>>> 17 марта 2020 г., в 13:45, Vladimir Steshin <[hidden email]
> >
> >>>>>> написал(а):
> >>>>>>>>>>
> >>>>>>>>>>   Hi, all.
> >>>>>>>>>>
> >>>>>>>>>> Fixes for control.sh and the REST have been merged. Could anyone
> >>>> take
> >>>>>> a look to the previous email with an issue? Isn't this conductvery
> >>>> wierd?
> >>>>>>>>>>
> >>>>>>
> >>>>
> >>>>
> >>>
> >>> --
> >>>
> >>> Best regards,
> >>> Alexei Scherbakov
> >>
> >>
>
>

[Alexey Goncharuk]

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.

[Nikolay Izhikov-2]

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 <[hidden email]> написал(а):
>
> 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.

[slava.koptilin]

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 <[hidden email]>:

> 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 <[hidden email]>
> написал(а):
> >
> > 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.
>
>

[Nikolay Izhikov-2]

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, Вячеслав Коптилин <[hidden email]> написал(а):
>
> 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 <[hidden email]>:
>
>> 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 <[hidden email]>
>> написал(а):
>>>
>>> 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.
>>
>>

[Alexey Goncharuk]

>
> 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?
>

Yes, this is what I had in mind.

[slava.koptilin]

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 <[hidden email]>:

> 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, Вячеслав Коптилин <[hidden email]>
> написал(а):
> >
> > 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 <[hidden email]>:
> >
> >> 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 <
> [hidden email]>
> >> написал(а):
> >>>
> >>> 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.
> >>
> >>
>
>

[Nikita Amelchev]

Hi, Igniters.

Note that DeactivateCommand#confirmationPrompt will be ignored in case
of auto confirmation.

I agree that the force flag should be removed when the user data and
datastructures will not be clearing on deactivation. For now, cmd, jmx
and rest interfaces should be covered.

вт, 24 мар. 2020 г. в 15:56, Вячеслав Коптилин <[hidden email]>:

>
> 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 <[hidden email]>:
>
> > 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, Вячеслав Коптилин <[hidden email]>
> > написал(а):
> > >
> > > 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 <[hidden email]>:
> > >
> > >> 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 <
> > [hidden email]>
> > >> написал(а):
> > >>>
> > >>> 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.
> > >>
> > >>
> >
> >

--
Best wishes,
Amelchev Nikita

[Steshin Vladimir]

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 <[hidden email]>:
>
>> 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, Вячеслав Коптилин <[hidden email]>
>> написал(а):
>>> 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 <[hidden email]>:
>>>
>>>> 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 <
>> [hidden email]>
>>>> написал(а):
>>>>> 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.
>>>>
>>

[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 <[hidden email]> 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 <[hidden email]>:
> >
> >> 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, Вячеслав Коптилин <[hidden email]
> >
> >> написал(а):
> >>> 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 <[hidden email]>:
> >>>
> >>>> 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 <
> >> [hidden email]>
> >>>> написал(а):
> >>>>> 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.
> >>>>
> >>
>

[Nikolay Izhikov-2]

Hello, Ivan.

> Is it correct? If we are on the same page, let's proceed this way.

+1 from me for your plan.

> We have removed GridClientClusterState#state(ClusterState) from Java client API. Is it a legitimate thing to do? Don't we have to support API

There is no changes in Public API except discussed in this thread.
GridClientClusterState is in internal package so it’s an internal class.

org/apache/ignite/internal/client/GridClientClusterState.java

> 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.

OK for me.
Feel free to suggest your statement.

> 27 марта 2020 г., в 10:51, Ivan Rakov <[hidden email]> написал(а):
>
> 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 <[hidden email]> 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 <[hidden email]>:
>>>
>>>> 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, Вячеслав Коптилин <[hidden email]
>>>
>>>> написал(а):
>>>>> 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 <[hidden email]>:
>>>>>
>>>>>> 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 <
>>>> [hidden email]>
>>>>>> написал(а):
>>>>>>> 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.
>>>>>>
>>>>
>>

[Steshin Vladimir]

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 <[hidden email]> 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 <[hidden email]>:
>>>
>>>> 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, Вячеслав Коптилин <[hidden email]
>>>> написал(а):
>>>>> 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 <[hidden email]>:
>>>>>
>>>>>> 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 <
>>>> [hidden email]>
>>>>>> написал(а):
>>>>>>> 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.

[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 <[hidden email]>
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 <[hidden email]>
> 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 <[hidden email]>:
> >>>
> >>>> 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, Вячеслав Коптилин <
> [hidden email]
> >>>> написал(а):
> >>>>> 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 <[hidden email]>:
> >>>>>
> >>>>>> 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 <
> >>>> [hidden email]>
> >>>>>> написал(а):
> >>>>>>> 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.
>

[Steshin Vladimir]

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 <[hidden email]>
> 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 <[hidden email]>
>> 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 <[hidden email]>:
>>>>>
>>>>>> 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, Вячеслав Коптилин <
>> [hidden email]
>>>>>> написал(а):
>>>>>>> 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 <[hidden email]>:
>>>>>>>
>>>>>>>> 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 <
>>>>>> [hidden email]>
>>>>>>>> написал(а):
>>>>>>>>> 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.

[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 <[hidden email]> 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 <[hidden email]>
> > 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 <[hidden email]>
> >> 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 <[hidden email]>:
> >>>>>
> >>>>>> 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, Вячеслав Коптилин <
> >> [hidden email]
> >>>>>> написал(а):
> >>>>>>> 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 <[hidden email]
> >:
> >>>>>>>
> >>>>>>>> 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 <
> >>>>>> [hidden email]>
> >>>>>>>> написал(а):
> >>>>>>>>> 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.
>

[Steshin Vladimir]

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 <[hidden email]> 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 <[hidden email]>
>>> 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 <[hidden email]>
>>>> 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 <[hidden email]>:
>>>>>>>
>>>>>>>> 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, Вячеслав Коптилин <
>>>> [hidden email]
>>>>>>>> написал(а):
>>>>>>>>> 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 <[hidden email]
>>> :
>>>>>>>>>> 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 <
>>>>>>>> [hidden email]>
>>>>>>>>>> написал(а):
>>>>>>>>>>> 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.

[slava.koptilin]

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 <[hidden email]>:

> 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 <[hidden email]>
> 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 <[hidden email]>
> >>> 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 <[hidden email]
> >
> >>>> 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 <[hidden email]
> >:
> >>>>>>>
> >>>>>>>> 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, Вячеслав Коптилин <
> >>>> [hidden email]
> >>>>>>>> написал(а):
> >>>>>>>>> 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 <
> [hidden email]
> >>> :
> >>>>>>>>>> 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 <
> >>>>>>>> [hidden email]>
> >>>>>>>>>> написал(а):
> >>>>>>>>>>> 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.
>

[Steshin Vladimir]

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 <[hidden email]>:
>
>> 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 <[hidden email]>
>> 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 <[hidden email]>
>>>>> 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 <[hidden email]
>>>>>> 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 <[hidden email]
>>> :
>>>>>>>>>> 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, Вячеслав Коптилин <
>>>>>> [hidden email]
>>>>>>>>>> написал(а):
>>>>>>>>>>> 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 <
>> [hidden email]
>>>>> :
>>>>>>>>>>>> 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 <
>>>>>>>>>> [hidden email]>
>>>>>>>>>>>> написал(а):
>>>>>>>>>>>>> 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.