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