Re: Is ML module @IgniteExperimental?

[Denis Magda]

Alexey, thanks for sharing details and your reasoning behind the taken
actions. It makes sense. I've updated the machine learning pages on the new
website that will be released in several days.

-
Denis


On Tue, Mar 24, 2020 at 11:07 AM Alexey Zinoviev 
wrote:

> Hi, Denis!
>
> Be honest, the significant amount of the ML contirbutors left the community
> previous year in frustration with unfinished parts.
> In this situation, I reduced the unsed and broken parts according our
> previous discussions peer-to-peer (not on devlist, our mistake) to release
> the stable core of ML which could be supported with reduced power.
>
> The reasons for GA removal
> 1. It doesn't related to the ML topic
> 2. It has no intersection with the ML package (as you mentioned)
> 3. It doesn't support Ignite code and in many places Java codestyle
> 4. It was experimental package placed in ML in time of earliest experiments
> in 2017
> 5. Nobody doesn't want to support this for the years
>
> Genetic Algorithms could be moved to Ignite-extension (if somebody
> interested in it)
>
> A lot of things are changed since release 2.7
>
> Lessons are learnt, I will start discussion topics next time for the
> significant changes or removal in API, moreover, the next releases I hope
> to use new @IgniteExperimental (it was added too late) and another
> annotations for the release cycle.
>
> вт, 24 мар. 2020 г. в 20:00, Denis Magda :
>
> > Alexey,
> >
> > I missed this thread and only now realized that TensorFlow, genetic
> > algorithms and some other APIs were expelled from 2.8. I would encourage
> us
> > to start a dedicated discussion for any APIs removal or significant
> changes
> > to let other community members share their opinions or take appropriate
> > actions (like proper documentation redirects setup for pages that are
> gone
> > and updates on the website like [1] and [2]). For instance, I have no
> glue
> > that the topic of TensorFlow removal was briefly mentioned in this
> > discussion thread.
> >
> > I see the reasoning about TensorFlow but why have we removed generic
> > algorithms that had a dependency on the compute APIs only?
> >
> > [1] https://ignite.apache.org/features/tensorflow.html
> > [2] https://ignite.apache.org/features/machinelearning.html#ga-grid
> >
> > -
> > Denis
> >
> >
> > On Mon, Feb 17, 2020 at 5:39 AM Alexey Zinoviev 
> > wrote:
> >
> > > Ok, agree, that I should start discussion before making changes, but I
> > was
> > > limited by release 2.8 and trying don;'t be a delayed person for that.
> > > During release I was focused on fixing bugs and don't tests TF and
> Ignite
> > > together
> > >
> > > I thought that as a maintainer of ML module I could do perform these
> > > actions.
> > >
> > > Below I will share my statement why it should be removed and why it
> > should
> > > be removed immediately
> > >
> > > About TensorFlow module (reason for removal)
> > >
> > >1. This module is only one module that uses IGFS and needs in
> > FileSystem
> > >on Ignte side due to TensorFlow API
> > >2. This module a part of bridge between Ignite ML and Tensorflow and
> > its
> > >broken after changes in TensorFlow on TensorFlow side
> > >3. TensorFlow released new version without Ignite bridge, no chance
> to
> > >run them together for new releases
> > >4. This module wasn't complete and developer who did this, left the
> > >community
> > >5. The development skills for this story require python/C++/java
> > >programming together
> > >6. The module is a source of bugs which could be fixed for release
> 2.8
> > >and possibly for future releases (nobody in community could this)
> > >7. The release size reduced from 6 Gb to 4.5 due to removed
> > dependencies
> > >8. TensorFlow now is not popular among Data Scientists, the PyTorch
> is
> > >the most popular tool for Deep Learning (like NetBeans and IDEA)
> > >9. Nobody uses that in production because it was developed between
> 2.7
> > >and 2.8 (2.7 has only proof-of-concept)
> > >
> > > Nikolay, sorry for that, hope to share more information about the ML
> and
> > > discuss here the main changes before actions.
> > >
> > > пн, 17 февр. 2020 г. в 16:18, Nikolay Izhikov :
> > >
> > > > Hello, Alexey.
> > > >
> > > > > The main reason, the modules are not work proper way, were
> > > experimental,
> > > > > never released as a production-ready, support old, outdated
> version,
> > > the
> > > > > external frameworks, like Tensorflow, move integration with ignite
> to
> > > the
> > > > > special repos, they are not finished, the code there is broken and
> > > > couldn't
> > > > > be fixed, because and I have no power/C++ skills/permission to
> commit
> > > > > something to them and time to support this broken modules.
> > > >
> > > >
> > > > Do we have some tickets or wider explanation for it?
> > > > It very uncommon for me that the decision to remove modules from the
> > > > master and release is not 

Re: Is ML module @IgniteExperimental?

[Alexey Zinoviev]

Hi, Denis!

Be honest, the significant amount of the ML contirbutors left the community
previous year in frustration with unfinished parts.
In this situation, I reduced the unsed and broken parts according our
previous discussions peer-to-peer (not on devlist, our mistake) to release
the stable core of ML which could be supported with reduced power.

The reasons for GA removal
1. It doesn't related to the ML topic
2. It has no intersection with the ML package (as you mentioned)
3. It doesn't support Ignite code and in many places Java codestyle
4. It was experimental package placed in ML in time of earliest experiments
in 2017
5. Nobody doesn't want to support this for the years

Genetic Algorithms could be moved to Ignite-extension (if somebody
interested in it)

A lot of things are changed since release 2.7

Lessons are learnt, I will start discussion topics next time for the
significant changes or removal in API, moreover, the next releases I hope
to use new @IgniteExperimental (it was added too late) and another
annotations for the release cycle.

вт, 24 мар. 2020 г. в 20:00, Denis Magda :

> Alexey,
>
> I missed this thread and only now realized that TensorFlow, genetic
> algorithms and some other APIs were expelled from 2.8. I would encourage us
> to start a dedicated discussion for any APIs removal or significant changes
> to let other community members share their opinions or take appropriate
> actions (like proper documentation redirects setup for pages that are gone
> and updates on the website like [1] and [2]). For instance, I have no glue
> that the topic of TensorFlow removal was briefly mentioned in this
> discussion thread.
>
> I see the reasoning about TensorFlow but why have we removed generic
> algorithms that had a dependency on the compute APIs only?
>
> [1] https://ignite.apache.org/features/tensorflow.html
> [2] https://ignite.apache.org/features/machinelearning.html#ga-grid
>
> -
> Denis
>
>
> On Mon, Feb 17, 2020 at 5:39 AM Alexey Zinoviev 
> wrote:
>
> > Ok, agree, that I should start discussion before making changes, but I
> was
> > limited by release 2.8 and trying don;'t be a delayed person for that.
> > During release I was focused on fixing bugs and don't tests TF and Ignite
> > together
> >
> > I thought that as a maintainer of ML module I could do perform these
> > actions.
> >
> > Below I will share my statement why it should be removed and why it
> should
> > be removed immediately
> >
> > About TensorFlow module (reason for removal)
> >
> >1. This module is only one module that uses IGFS and needs in
> FileSystem
> >on Ignte side due to TensorFlow API
> >2. This module a part of bridge between Ignite ML and Tensorflow and
> its
> >broken after changes in TensorFlow on TensorFlow side
> >3. TensorFlow released new version without Ignite bridge, no chance to
> >run them together for new releases
> >4. This module wasn't complete and developer who did this, left the
> >community
> >5. The development skills for this story require python/C++/java
> >programming together
> >6. The module is a source of bugs which could be fixed for release 2.8
> >and possibly for future releases (nobody in community could this)
> >7. The release size reduced from 6 Gb to 4.5 due to removed
> dependencies
> >8. TensorFlow now is not popular among Data Scientists, the PyTorch is
> >the most popular tool for Deep Learning (like NetBeans and IDEA)
> >9. Nobody uses that in production because it was developed between 2.7
> >and 2.8 (2.7 has only proof-of-concept)
> >
> > Nikolay, sorry for that, hope to share more information about the ML and
> > discuss here the main changes before actions.
> >
> > пн, 17 февр. 2020 г. в 16:18, Nikolay Izhikov :
> >
> > > Hello, Alexey.
> > >
> > > > The main reason, the modules are not work proper way, were
> > experimental,
> > > > never released as a production-ready, support old, outdated version,
> > the
> > > > external frameworks, like Tensorflow, move integration with ignite to
> > the
> > > > special repos, they are not finished, the code there is broken and
> > > couldn't
> > > > be fixed, because and I have no power/C++ skills/permission to commit
> > > > something to them and time to support this broken modules.
> > >
> > >
> > > Do we have some tickets or wider explanation for it?
> > > It very uncommon for me that the decision to remove modules from the
> > > master and release is not discussed widely in the community.
> > >
> > > > 17 февр. 2020 г., в 14:39, Ravil Galeyev 
> > > написал(а):
> > > >
> > > > Hi Team,
> > > >
> > > > First of all, let me introduce myself. I’m Ravil, I contribute to the
> > ML
> > > > module since 2018 and from time to time I make talks about it. (I..e
> > data
> > > > science summit in Warsaw [1]).
> > > >
> > > > So, Alexey made a huge effort to develop the ML module but he is not
> > > alone.
> > > > If you check the repo you will find other 

Re: Is ML module @IgniteExperimental?

[Denis Magda]

Alexey,

I missed this thread and only now realized that TensorFlow, genetic
algorithms and some other APIs were expelled from 2.8. I would encourage us
to start a dedicated discussion for any APIs removal or significant changes
to let other community members share their opinions or take appropriate
actions (like proper documentation redirects setup for pages that are gone
and updates on the website like [1] and [2]). For instance, I have no glue
that the topic of TensorFlow removal was briefly mentioned in this
discussion thread.

I see the reasoning about TensorFlow but why have we removed generic
algorithms that had a dependency on the compute APIs only?

[1] https://ignite.apache.org/features/tensorflow.html
[2] https://ignite.apache.org/features/machinelearning.html#ga-grid

-
Denis


On Mon, Feb 17, 2020 at 5:39 AM Alexey Zinoviev 
wrote:

> Ok, agree, that I should start discussion before making changes, but I was
> limited by release 2.8 and trying don;'t be a delayed person for that.
> During release I was focused on fixing bugs and don't tests TF and Ignite
> together
>
> I thought that as a maintainer of ML module I could do perform these
> actions.
>
> Below I will share my statement why it should be removed and why it should
> be removed immediately
>
> About TensorFlow module (reason for removal)
>
>1. This module is only one module that uses IGFS and needs in FileSystem
>on Ignte side due to TensorFlow API
>2. This module a part of bridge between Ignite ML and Tensorflow and its
>broken after changes in TensorFlow on TensorFlow side
>3. TensorFlow released new version without Ignite bridge, no chance to
>run them together for new releases
>4. This module wasn't complete and developer who did this, left the
>community
>5. The development skills for this story require python/C++/java
>programming together
>6. The module is a source of bugs which could be fixed for release 2.8
>and possibly for future releases (nobody in community could this)
>7. The release size reduced from 6 Gb to 4.5 due to removed dependencies
>8. TensorFlow now is not popular among Data Scientists, the PyTorch is
>the most popular tool for Deep Learning (like NetBeans and IDEA)
>9. Nobody uses that in production because it was developed between 2.7
>and 2.8 (2.7 has only proof-of-concept)
>
> Nikolay, sorry for that, hope to share more information about the ML and
> discuss here the main changes before actions.
>
> пн, 17 февр. 2020 г. в 16:18, Nikolay Izhikov :
>
> > Hello, Alexey.
> >
> > > The main reason, the modules are not work proper way, were
> experimental,
> > > never released as a production-ready, support old, outdated version,
> the
> > > external frameworks, like Tensorflow, move integration with ignite to
> the
> > > special repos, they are not finished, the code there is broken and
> > couldn't
> > > be fixed, because and I have no power/C++ skills/permission to commit
> > > something to them and time to support this broken modules.
> >
> >
> > Do we have some tickets or wider explanation for it?
> > It very uncommon for me that the decision to remove modules from the
> > master and release is not discussed widely in the community.
> >
> > > 17 февр. 2020 г., в 14:39, Ravil Galeyev 
> > написал(а):
> > >
> > > Hi Team,
> > >
> > > First of all, let me introduce myself. I’m Ravil, I contribute to the
> ML
> > > module since 2018 and from time to time I make talks about it. (I..e
> data
> > > science summit in Warsaw [1]).
> > >
> > > So, Alexey made a huge effort to develop the ML module but he is not
> > alone.
> > > If you check the repo you will find other contributors.
> > >
> > > Therefore the ML module is alive and is able to run and has the
> roadmap.
> > > For me, it means that it’s not a raw project.
> > >
> > > Regarding documentation, it’d like to mention the code is the best
> > > documentation :)
> > >
> > > We have examples for most algorithms [2]. But if it needed I’m ready to
> > > help the community with documentation in English German Polish or
> > Russain.
> > >
> > >
> > > [1] https://dssconf.pl/
> > >
> > > [2]
> > >
> >
> https://github.com/apache/ignite/tree/master/examples/src/main/java/org/apache/ignite/examples/ml
> > >
> > > Best regards,
> > >
> > > Ravil
> > >
> > >
> > > On Mon, 17 Feb 2020 at 11:49, Alexey Zinoviev 
> > > wrote:
> > >
> > >> Hello, Igniters, and you, Nikolay.
> > >>
> > >> First of all, if you have real interest to the ML module and its
> state,
> > I
> > >> could make call with you and explain this.
> > >>
> > >>
> > >> *As far as I know, for now, we have only 1 active contributor to this
> > area
> > >> -Alexey Zinoviev.*
> > >> Currently, we have 2 active contributors, me and Ravil Galeeyev, a few
> > >> newbies, another guys who started tensorflow and another modules and
> > >> submodules don't visit the community for many months.
> > >>
> > >> *Is ML module production ready?*
> > >> This 

Re: Data vanished from cluster after INACTIVE/ACTIVE switch

[Vladimir Steshin]

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 :


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, Вячеслав Коптилин 

написал(а):

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 :


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.

Re: Data vanished from cluster after INACTIVE/ACTIVE switch

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



-- 
Best wishes,
Amelchev Nikita

Re: IGNITE-12702 - Discussion

[kartik somani]

Thank you for the clarification

On Mon, Mar 23, 2020, 11:28 PM Denis Magda  wrote:

> Hi Kartik,
>
> Actually, it means quite the opposite. You should fill in the release notes
> field before the ticket is resolved/closed so that a release manager of a
> next Ignite version adds a proper line to the final release notes with all
> the changes.
>
> As for the docs, it's highly likely you don't need to update Ignite
> technical documentation (apacheignite.readme.io), thus, the field should
> be
> disabled. However, see if APIs (Javadocs) require an update. Hope this
> helps. Btw, this page elaborates more on our current docs process that is
> being revisited and improved soon:
> https://cwiki.apache.org/confluence/display/IGNITE/How+to+Document
>
> -
> Denis
>
>
> On Sun, Mar 22, 2020 at 6:33 AM kartik somani <
> kartiksomani.offic...@gmail.com> wrote:
>
> > Hello Igniters,
> >
> > I want to start working on ticket IGNITE-12702
> > .  The flags on this
> > issue are "Docs Required, Release Notes Required". Does this mean I have
> to
> > first write/get docs and release notes before starting work on this?
> >
> > Regards,
> > Kartik
> >
>

Re: Introduce separate SQL configuration

[Вячеслав Коптилин]

Hello Taras,

> My be add javadoc for each changed parameter instead of list?
Yep, this makes sense to me.

> But we cannot reference internal packages in public API.
Hmm, it is weird that out JMX beans reside are placed in internal packages.
Perhaps, it is just enough to mention that the property can be changed
through corresponding JMX bean and not provide ф link to the bean itself.
However, this approach is not user friendly, I think.

Thanks,
S.

пн, 23 мар. 2020 г. в 18:17, Taras Ledkov :

> Hi,
>
>  > It would be nice to provide a list of properties that can be changed
> at runtime
>
> Ok, I catch the your and Pavel's opinion.
> My be add javadoc for each changed parameter instead of list?
>
> e.g. for longQueryWarningTimeout
> See {@link
> org.apache.ignite.internal.mxbean.SqlQueryMXBean#setLongQueryWarningTimeout}
>
>
> to change this parameter in runtime.
>
> But we cannot reference internal packages in public API.
>
> On 23.03.2020 17:51, Вячеслав Коптилин wrote:
> > Hello Taras,
> >
> > I think this is the right way.
> > I agree with Pavel's comment. It would be nice to provide a list of
> > properties that can be changed at runtime and remove the word “Initial”
> > from the class name.
> >
> > Thanks,
> > S.
> >
> > пн, 23 мар. 2020 г. в 16:54, Pavel Tupitsyn :
> >
> >> Hi Taras,
> >>
> >> I support the idea, it brings some order to the IgniteConfiguration.
> >>
> >> However, I have strong objections to `Initial` prefix:
> >>> many parameters may be changed in runtime
> >> This applies to many other config properties, but we don't name them
> >> `Initial`
> >>
> >> Let's just make it SqlConfiguration.
> >>
> >> On Mon, Mar 23, 2020 at 1:04 PM Taras Ledkov 
> wrote:
> >>
> >>> Hi, Igniters.
> >>>
> >>> I propose to introduce separate configuration class for SQL
> >> configuration.
> >>> e.g. SqlInitailConfiguration.
> >>>
> >>> Now we have several SQL parameters:
> >>> - sqlSchemas;
> >>> - defaultQueryTimeout;
> >>> - longQueryWarningTimeout;
> >>> - sqlQueryHistorySize;
> >>>
> >>> are placed at the IgniteConfiguration.
> >>> I propose to deprecate the SQL properties in the IgniteConfiguration
> and
> >>> redirect the calls to child configuration object of
> >>> SqlInitailConfiguration.
> >>>
> >>> We are going to add several additional parameters in the nearest
> future.
> >>> I guess separate configuration is better than many plain SQL parameters
> >>> at the root IgniteConfiguration.
> >>>
> >>> I insist on the configuration be called 'Initial' because many
> >>> parameters may be changed in runtime
> >>> (e.g. longQueryWarningTimeout by JMX, set of schemas by creating caches
> >>> etc).
> >>>
> >>> I've created PR [1] that contains proposal changes. The patch is not
> >>> final clean.
> >>> I think PR may be useful to demonstrate the proposal.
> >>>
> >>> Please let me know your opinion.
> >>>
> >>> [1]. https://github.com/apache/ignite/pull/7559
> >>>
> >>>
> >>> --
> >>> Taras Ledkov
> >>> Mail-To: tled...@gridgain.com
> >>>
> >>>
> --
> Taras Ledkov
> Mail-To: tled...@gridgain.com
>
>

[jira] [Created] (IGNITE-12833) JDBC thin client SELECT hangs under 2.8.0

[Veena Mithare (Jira)]

Veena Mithare created IGNITE-12833:
--

 Summary: JDBC thin client SELECT hangs under 2.8.0
 Key: IGNITE-12833
 URL: https://issues.apache.org/jira/browse/IGNITE-12833
 Project: Ignite
  Issue Type: Bug
  Components: security
Affects Versions: 2.8
Reporter: Veena Mithare


|
|When security is enabled, and an update or select sql is issued from dbeaver, 
the security context in 
class GridIOManager , 
method -createGridIoMessage - 
line - ctx.security().securityContext() returns  the securitycontext of the 
thin client. 

The message generated out of createGridIoMessage  is passed on to the next 
node. 

This is used in 
class - IgniteSecurityProcessor 
method - ( withContext) 
line - ctx.discovery().node(uuid) 
on the next node :     

@Override public OperationSecurityContext withContext(UUID nodeId) 
{        return withContext(            secCtxs.computeIfAbsent(nodeId,         
      
uuid -> nodeSecurityContext(                    marsh, 
U.resolveClassLoader(ctx.config()), ctx.discovery().node(uuid)               
)            )        );    } 


The ctx.discovery().node(uuid) used to 
determine the ClusterNode that is passed into nodeSecurityContext() returns 
null, since the uuid is that of the remote client id not the remote node id. 


Hence 
class: SecurityUtils.java 
method : nodeSecurityContext 
line :         byte[] subjBytes = 
node.attribute(IgniteNodeAttributes.ATTR_SECURITY_SUBJECT_V2); 

Throws null pointer exception since node is null. 


Related ticket : 
IGNITE-12579
 
Related discussion : 
[http://apache-ignite-users.70518.x6.nabble.com/2-8-0-JDBC-Thin-Client-Unable-to-load-the-tables-via-DBeaver-td31681.html#a31847]|
|



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

[jira] [Created] (IGNITE-12834) [TeamCity] Suites fails with the execution timeout

[Nikolay Izhikov (Jira)]

Nikolay Izhikov created IGNITE-12834:


 Summary: [TeamCity] Suites fails with the execution timeout
 Key: IGNITE-12834
 URL: https://issues.apache.org/jira/browse/IGNITE-12834
 Project: Ignite
  Issue Type: Bug
Reporter: Nikolay Izhikov


Right now it's a common case when some random suite ends with the execution 
timeout error.

Examples:

* 
https://ci.ignite.apache.org/viewLog.html?buildId=5152046=buildResultsDiv=IgniteTests24Java8_MvccCache7
* 
https://ci.ignite.apache.org/viewLog.html?buildId=5151963=buildResultsDiv=IgniteTests24Java8_JavaThinClient

We need to investigate those failures and fix the root cause of it.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Re: Data vanished from cluster after INACTIVE/ACTIVE switch

[Вячеслав Коптилин]

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 :

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

Re: Security Subject of thin client on remote nodes

[Denis Garus]

Hi, guys!


I agree that we should rework the security API, but it can take a long
time.

And currently, our users have certain impediments that are blockers for
their job.

I think we have to fix bugs that IEP-41 [1] contains as soon as possible to
support our users.

 From my point of view, IEP-41 is the best place to track bug fixing.



   1.
   
https://cwiki.apache.org/confluence/display/IGNITE/IEP-41%3A+Security+Context+of+thin+client+on+remote+nodes


вт, 24 мар. 2020 г. в 12:26, Ivan Rakov :

> Alexey,
>
> That can be another version of our plan. If everyone agrees that
> SecurityContext and SecuritySubject should be merged, such fix of thin
> clients' issue will bring us closer to the final solution.
> Denis, what do you think?
>
> On Tue, Mar 24, 2020 at 10:38 AM Alexei Scherbakov <
> alexey.scherbak...@gmail.com> wrote:
>
> > Why can't we start gradually changing security API right now ?
> > I see no point in delaying with.
> > All changes will go to next 2.9 release anyway.
> >
> > My proposal:
> > 1. Get rid of security context. Doing this will bring security API to
> more
> > or less consistent state.
> > 2. Remove IEP-41 because it's no longer needed because of change [1]
> > 3. Propose an IEP to make security API avoid using internals.
> >
> >
> >
> > пн, 23 мар. 2020 г. в 19:53, Denis Garus :
> >
> > > Hello, Alexei, Ivan!
> > >
> > > >> Seems like security API is indeed a bit over-engineered
> > >
> > > Nobody has doubt we should do a reworking of GridSecurityProcessor.
> > > But this point is outside of scope thin client's problem that we are
> > > solving.
> > > I think we can create new IEP that will accumulate all ideas of
> Ignite's
> > > security improvements.
> > >
> > > >> Presence of the separate #securityContext(UUID) highlights that user
> > > indeed should care
> > > >> about propagation of thin clients' contexts between the cluster
> nodes.
> > >
> > > I agree with Ivan. I've implemented both variants,
> > > and I like one with #securityContext(UUID) more.
> > >
> > > Could you please take a look at PR [1] for the issue [2]?
> > >
> > > 1. https://github.com/apache/ignite/pull/7523
> > > 2. https://issues.apache.org/jira/browse/IGNITE-12759
> > >
> > > пн, 23 мар. 2020 г. в 11:45, Ivan Rakov :
> > >
> > > > Alex, Denis,
> > > >
> > > > Seems like security API is indeed a bit over-engineered.
> > > >
> > > > Let's get rid of SecurityContext and use SecuritySubject instead.
> > > > > SecurityContext is just a POJO wrapper over
> > > > > SecuritySubject's
> > > > > org.apache.ignite.plugin.security.SecuritySubject#permissions.
> > > > > It's functionality can be easily moved to SecuritySubject.
> > > >
> > > > I totally agree. Both subject and context are implemented by plugin
> > > > provider, and I don't see any reason to keep both abstractions,
> > > especially
> > > > if we are going to get rid of transferring subject in node attributes
> > > > (argument that subject is more lightweight won't work anymore).
> > > >
> > > > Also, there's kind of mess in node authentication logic. There are at
> > > least
> > > > two components responsible for it: DiscoverySpiNodeAuthenticator
> (which
> > > is
> > > > forcibly set by GridDiscoveryManager, but in fact public) and
> > > > GridSecurityProcessor (which performs actual node auth logic, but
> > > private).
> > > > I also don't understand why we need both
> > > > #authenticate(AuthenticationContext) and
> #authenticateNode(ClusterNode,
> > > > SecurityCredentials) methods while it's possible to set explicit
> > > > SecuritySubjectType.REMOTE_NODE in AuthenticationContext (this is
> > > arguable;
> > > > perhaps there are strong reasons).
> > > >
> > > > Finally, areas of responsibility between IgniteSecurity and
> > > > GridSecurityProcessor are kind of mixed. As far as I understand, the
> > > first
> > > > is responsible for Ignite-internal management of security logic
> > (keeping
> > > > thread-local context, caching security contexts, etc; we don't expect
> > > > IgniteSecurity to be replaced by plugin provider) and the latter is
> > > > responsible for user-custom authentication / authorization logic. To
> be
> > > > honest, it took plenty of time to figure this out for me.
> > > >
> > > > From my point of view, we should make GridSecurityProcessor interface
> > > > public, rename it (it requires plenty of time to find the difference
> > from
> > > > IgniteSecurity), make its API as simple and non-duplicating as
> possible
> > > and
> > > > clarify its area of responsibility (e.g. should it be responsible for
> > > > propagation of successfully authenticated subject among all nodes or
> > > not?)
> > > > to make it easy to embed custom security logic in Ignite.
> > > >
> > > > Regarding thin clients fix: implementation made by Denis suits better
> > to
> > > > the very implicit contract that it's better to change API contracts
> of
> > an
> > > > internal IgniteSecurity than of internal GridSecurityProcessor (which
> > > > 

Re: Who can update TC Bot version.

[Petr Ivanov]

We can manage that here.


> On 24 Mar 2020, at 13:29, Dmitriy Pavlov  wrote:
> 
> Hi,
> 
> I could potentially update this version in the source code, but I'm not
> able to deploy new version to the server (since it is now sponsored by
> GrigGain, we need someone who will deploy).
> 
> Sincerely,
> Dmitriy Pavlov
> 
> пн, 23 мар. 2020 г. в 08:59, Zhenya Stanilovsky :
> 
>> Igniters, 2.8 version already released, but looks like TC bot [1] [2]
>> still using 2.7.6 ver. , who can update it ?
>> Thanks !
>> 
>> [1] https://github.com/apache/ignite-teamcity-bot
>> [2] https://mtcga.gridgain.com/
>> 
>> 
>> 
>> 

Re: Data vanished from cluster after INACTIVE/ACTIVE switch

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

Re: Who can update TC Bot version.

[Dmitriy Pavlov]

Hi,

I could potentially update this version in the source code, but I'm not
able to deploy new version to the server (since it is now sponsored by
GrigGain, we need someone who will deploy).

Sincerely,
Dmitriy Pavlov

пн, 23 мар. 2020 г. в 08:59, Zhenya Stanilovsky :

> Igniters, 2.8 version already released, but looks like TC bot [1] [2]
> still using 2.7.6 ver. , who can update it ?
> Thanks !
>
> [1] https://github.com/apache/ignite-teamcity-bot
> [2] https://mtcga.gridgain.com/
>
>
>
>

Re: Data vanished from cluster after INACTIVE/ACTIVE switch

[Nikolay Izhikov]

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, Вячеслав Коптилин  
> написал(а):
> 
> 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 :
> 
>> 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 
>> написал(а):
>>> 
>>> Igniters, Ivan, Nikolay,
>>> 
>>> I am strongly against adding confirmation flags to any kind of APIs,
>>> whether we change the deactivation behavior or not (even though I agree
>>> that it makes sense to fix the deactivation to not clean up the in-memory
>>> data). The confirmation should only present in the user-facing
>> interfaces.
>>> I cannot recall any software interface which has such a flag. None of the
>>> syscalls which delete files (a very destructive operation) have this
>> flag.
>>> The DROP TABLE command does not have a "yes I am sure" clause in it. As I
>>> already mentioned, when used programmatically, most users will likely
>>> simply pass 'true' as the new flag because they already know the
>> behavior.
>>> This is a clear sign of a bad design choice.
>>> 
>>> On top of that, given that it is our intention to change the behavior of
>>> deactivation to not loose the in-memory data, it does not make sense to
>> me
>>> to change the API.
>> 
>> 

Re: Data vanished from cluster after INACTIVE/ACTIVE switch

[Вячеслав Коптилин]

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 :

> 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 
> написал(а):
> >
> > Igniters, Ivan, Nikolay,
> >
> > I am strongly against adding confirmation flags to any kind of APIs,
> > whether we change the deactivation behavior or not (even though I agree
> > that it makes sense to fix the deactivation to not clean up the in-memory
> > data). The confirmation should only present in the user-facing
> interfaces.
> > I cannot recall any software interface which has such a flag. None of the
> > syscalls which delete files (a very destructive operation) have this
> flag.
> > The DROP TABLE command does not have a "yes I am sure" clause in it. As I
> > already mentioned, when used programmatically, most users will likely
> > simply pass 'true' as the new flag because they already know the
> behavior.
> > This is a clear sign of a bad design choice.
> >
> > On top of that, given that it is our intention to change the behavior of
> > deactivation to not loose the in-memory data, it does not make sense to
> me
> > to change the API.
>
>

Re: Data vanished from cluster after INACTIVE/ACTIVE switch

[Nikolay Izhikov]

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  
> написал(а):
> 
> Igniters, Ivan, Nikolay,
> 
> I am strongly against adding confirmation flags to any kind of APIs,
> whether we change the deactivation behavior or not (even though I agree
> that it makes sense to fix the deactivation to not clean up the in-memory
> data). The confirmation should only present in the user-facing interfaces.
> I cannot recall any software interface which has such a flag. None of the
> syscalls which delete files (a very destructive operation) have this flag.
> The DROP TABLE command does not have a "yes I am sure" clause in it. As I
> already mentioned, when used programmatically, most users will likely
> simply pass 'true' as the new flag because they already know the behavior.
> This is a clear sign of a bad design choice.
> 
> On top of that, given that it is our intention to change the behavior of
> deactivation to not loose the in-memory data, it does not make sense to me
> to change the API.

Re: Data vanished from cluster after INACTIVE/ACTIVE switch

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

Re: Data vanished from cluster after INACTIVE/ACTIVE switch

[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 
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 
> написал(а):
> >
> >>
> >> 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 
> > 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 <
> >> alexey.scherbak...@gmail.com> написал(а):
> >>>
> >>> 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 :
> >>>
>  Hello, Ivan.
> 
> > Seems like we don't have a final agreement on whether we should add
> >> force
>  flag to deactivation API.
> 
>  I think the 

Re: Security Subject of thin client on remote nodes

[Ivan Rakov]

Alexey,

That can be another version of our plan. If everyone agrees that
SecurityContext and SecuritySubject should be merged, such fix of thin
clients' issue will bring us closer to the final solution.
Denis, what do you think?

On Tue, Mar 24, 2020 at 10:38 AM Alexei Scherbakov <
alexey.scherbak...@gmail.com> wrote:

> Why can't we start gradually changing security API right now ?
> I see no point in delaying with.
> All changes will go to next 2.9 release anyway.
>
> My proposal:
> 1. Get rid of security context. Doing this will bring security API to more
> or less consistent state.
> 2. Remove IEP-41 because it's no longer needed because of change [1]
> 3. Propose an IEP to make security API avoid using internals.
>
>
>
> пн, 23 мар. 2020 г. в 19:53, Denis Garus :
>
> > Hello, Alexei, Ivan!
> >
> > >> Seems like security API is indeed a bit over-engineered
> >
> > Nobody has doubt we should do a reworking of GridSecurityProcessor.
> > But this point is outside of scope thin client's problem that we are
> > solving.
> > I think we can create new IEP that will accumulate all ideas of Ignite's
> > security improvements.
> >
> > >> Presence of the separate #securityContext(UUID) highlights that user
> > indeed should care
> > >> about propagation of thin clients' contexts between the cluster nodes.
> >
> > I agree with Ivan. I've implemented both variants,
> > and I like one with #securityContext(UUID) more.
> >
> > Could you please take a look at PR [1] for the issue [2]?
> >
> > 1. https://github.com/apache/ignite/pull/7523
> > 2. https://issues.apache.org/jira/browse/IGNITE-12759
> >
> > пн, 23 мар. 2020 г. в 11:45, Ivan Rakov :
> >
> > > Alex, Denis,
> > >
> > > Seems like security API is indeed a bit over-engineered.
> > >
> > > Let's get rid of SecurityContext and use SecuritySubject instead.
> > > > SecurityContext is just a POJO wrapper over
> > > > SecuritySubject's
> > > > org.apache.ignite.plugin.security.SecuritySubject#permissions.
> > > > It's functionality can be easily moved to SecuritySubject.
> > >
> > > I totally agree. Both subject and context are implemented by plugin
> > > provider, and I don't see any reason to keep both abstractions,
> > especially
> > > if we are going to get rid of transferring subject in node attributes
> > > (argument that subject is more lightweight won't work anymore).
> > >
> > > Also, there's kind of mess in node authentication logic. There are at
> > least
> > > two components responsible for it: DiscoverySpiNodeAuthenticator (which
> > is
> > > forcibly set by GridDiscoveryManager, but in fact public) and
> > > GridSecurityProcessor (which performs actual node auth logic, but
> > private).
> > > I also don't understand why we need both
> > > #authenticate(AuthenticationContext) and #authenticateNode(ClusterNode,
> > > SecurityCredentials) methods while it's possible to set explicit
> > > SecuritySubjectType.REMOTE_NODE in AuthenticationContext (this is
> > arguable;
> > > perhaps there are strong reasons).
> > >
> > > Finally, areas of responsibility between IgniteSecurity and
> > > GridSecurityProcessor are kind of mixed. As far as I understand, the
> > first
> > > is responsible for Ignite-internal management of security logic
> (keeping
> > > thread-local context, caching security contexts, etc; we don't expect
> > > IgniteSecurity to be replaced by plugin provider) and the latter is
> > > responsible for user-custom authentication / authorization logic. To be
> > > honest, it took plenty of time to figure this out for me.
> > >
> > > From my point of view, we should make GridSecurityProcessor interface
> > > public, rename it (it requires plenty of time to find the difference
> from
> > > IgniteSecurity), make its API as simple and non-duplicating as possible
> > and
> > > clarify its area of responsibility (e.g. should it be responsible for
> > > propagation of successfully authenticated subject among all nodes or
> > not?)
> > > to make it easy to embed custom security logic in Ignite.
> > >
> > > Regarding thin clients fix: implementation made by Denis suits better
> to
> > > the very implicit contract that it's better to change API contracts of
> an
> > > internal IgniteSecurity than of internal GridSecurityProcessor (which
> > > actually mustn't be internal).
> > >
> > > > My approach doesn't require any IEPs, just minor change in code and
> to
> > > >
> > > >
> > >
> >
> org.apache.ignite.internal.processors.security.IgniteSecurity#authenticate(AuthenticationContext)
> > > > contract.
> > >
> > > Looks like a misuse of #authenticate method to me. It should perform
> > > initial authentication based on credentials (this may include queries
> to
> > > external authentication subsystem, e.g. LDAP). User may want to don't
> > > authenticate thin client on every node (this will increase the number
> of
> > > requests to auth subsystem unless user implicitly implements
> propagation
> > of
> > > thin clients' contexts between nodes and make 

Re: Data vanished from cluster after INACTIVE/ACTIVE switch

[Nikolay Izhikov]

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  написал(а):
> 
>> 
>> 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 
> 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 <
>> alexey.scherbak...@gmail.com> написал(а):
>>> 
>>> 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 :
>>> 
 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 

Re: Apache Ignite 2.8.0 spring services configuration null fields

[myset]

Thank you for help ezhuravl,
Your proposal sample it is OK.
In my example tests.TestServiceImpl class extends another class with all the
fields (geters and seters) and work methods inside.

In 2.7.6 works in 2.8.0 @ runtime all fields are null on the same
conditions/environment (java 11.0.4).

If i put all fields and methods inside TestServiceImpl everithing works
well.

Maybe, it is a matter of inheritance and security access ?!!?

What do you think?



--
Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/

Re: 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 
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 <
> alexey.scherbak...@gmail.com> написал(а):
> >
> > 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 :
> >
> >> 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 
> >> написал(а):
> >>>
> >>> 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 

Re: Reference of local service.

[Vladimir Steshin]

    Hi, folks.

 I'd like to advance the final decision. Is it OK to:

1)Make IgniteService.serviceProxy() return proxy for locally deployed 
service too.


2)Make the proxy collect metrics of service method invocations without 
any additional conditions, interfaces, options.


3)    Deprecate IgniteService.service().

WDYT?


17.03.2020 19:56, Vladimir Steshin пишет:

Andrey,

>>> Is it possible to return actual class instance instead of 
interface from serviceProxy() method?


No, it is not possible. IgniteServiceImpl doesn't allow that:

public  T serviceProxy(final String name, final Class 
svcItf, final boolean sticky, final long timeout) {

    ...
    A.ensure(svcItf.isInterface(), "Service class must be an 
interface: " + svcItf);


    ...

}


17.03.2020 17:34, Andrey Gura пишет:

Vladimir,

Why not using IgniteServices.serviceProxy() for that? Since it 
requires an interface, It could return proxy for local service too and

keep backward compatibility at the same time.

Is it possible to return actual class instance instead of interface
from serviceProxy() method? E.g. could I get ServiceImpl as result of
method call instead of ServiceItf?

On Tue, Mar 17, 2020 at 9:50 AM Vladimir Steshin  
wrote:

Andrey,

IgniteServices.service() method could return actual interface 
implementation instead of interface itself.


IgniteServices.service() always return actual local service 
instance, no proxy, might be without any interface but except Service.



If yes then we can add new method IgniteService.serviceLocalProxy().
It will not break backward compatibility and will always return 
proxy.
Why not using IgniteServices.serviceProxy() for that? Since it 
requires an interface, It could return proxy for local service too and

keep backward compatibility at the same time.

16.03.2020 20:21, Andrey Gura пишет:

Vladimir,


We won’t affect existing services
How exactly will we affect services without special interface? 
Please see

the benchmarks in previous email.

I talk about backward compatibility, not about performance. But it
doesn't matter because... see below.

My fault. From discussion I realized that services doesn't require
interface. But indeed it does require.

If I understand correctly, IgniteServices.service() method could
return actual interface implementation instead of interface itself.
Am I right?

If yes then we can add new method IgniteService.serviceLocalProxy().
It will not break backward compatibility and will always return proxy.

On Thu, Mar 12, 2020 at 2:25 PM Vladimir Steshin 
 wrote:

Andrey, hi.


We won’t affect existing services
How exactly will we affect services without special interface? 
Please see

the benchmarks in previous email.


what if we generate will generate proxy that collects service’s 
metrics

only if service will implement some special interface?


I don’t like idea enabling/disabling metrics involves code change,
compilation. I believe it should be an external option, probably 
available

at runtime through JMX.


we can impose additional requirement for services that want use 
metrics

out of box. … service must have own interface and only invocations of
methods of this interface will be taken into account for metrics 
collection.


Why one more interface? To work via proxy, with remote services user
already has to use an interface additionally to Service. If we 
introduce
proxy for local services too (as suggested earlier), an interface 
will be
required. Current IgniteService#serviceProxy() already requires 
interface
even for local service. I don’t think we need one more special 
interface.




user always can use own metrics framework.
Since we do not significantly affect services user can use both or 
disable

our by an option.


With the discussion before and the benchmark I propose:


- Let IgniteService#serviceProxy() give GridServiceProxy for local 
services
too. It already requires to work via interface. So it’s safe for 
user code.



- Deprecate IgniteService#service()


- Make service metrics enabled by default for all services.


- Bring system param which disables metrics by default for all 
services.



- Bring parameter/method in MetricsMxBean which allows 
disabling/enabling

metrics for all services at run time.

Makes sense?

чт, 5 мар. 2020 г., 16:48 Andrey Gura :


Hi there,

what if we will generate proxy that collects service's metrics 
only if

service will implement some special interface? In such case:

- we won't affect existing services at all.
- we can impose additional requirement for services that want use
metrics out of box (i.e. service that implements our special 
interface
*must* also have own interface and only invocations of methods of 
this

interface will be taken into account for metrics collection).
- user always can use own metrics framework instead of our (just do
not implement this new special interface).

About metrics enabling/disabling. At present IGNITE-11927 doesn't
solve this problem. Just because 

Re: Security Subject of thin client on remote nodes

[Alexei Scherbakov]

Why can't we start gradually changing security API right now ?
I see no point in delaying with.
All changes will go to next 2.9 release anyway.

My proposal:
1. Get rid of security context. Doing this will bring security API to more
or less consistent state.
2. Remove IEP-41 because it's no longer needed because of change [1]
3. Propose an IEP to make security API avoid using internals.



пн, 23 мар. 2020 г. в 19:53, Denis Garus :

> Hello, Alexei, Ivan!
>
> >> Seems like security API is indeed a bit over-engineered
>
> Nobody has doubt we should do a reworking of GridSecurityProcessor.
> But this point is outside of scope thin client's problem that we are
> solving.
> I think we can create new IEP that will accumulate all ideas of Ignite's
> security improvements.
>
> >> Presence of the separate #securityContext(UUID) highlights that user
> indeed should care
> >> about propagation of thin clients' contexts between the cluster nodes.
>
> I agree with Ivan. I've implemented both variants,
> and I like one with #securityContext(UUID) more.
>
> Could you please take a look at PR [1] for the issue [2]?
>
> 1. https://github.com/apache/ignite/pull/7523
> 2. https://issues.apache.org/jira/browse/IGNITE-12759
>
> пн, 23 мар. 2020 г. в 11:45, Ivan Rakov :
>
> > Alex, Denis,
> >
> > Seems like security API is indeed a bit over-engineered.
> >
> > Let's get rid of SecurityContext and use SecuritySubject instead.
> > > SecurityContext is just a POJO wrapper over
> > > SecuritySubject's
> > > org.apache.ignite.plugin.security.SecuritySubject#permissions.
> > > It's functionality can be easily moved to SecuritySubject.
> >
> > I totally agree. Both subject and context are implemented by plugin
> > provider, and I don't see any reason to keep both abstractions,
> especially
> > if we are going to get rid of transferring subject in node attributes
> > (argument that subject is more lightweight won't work anymore).
> >
> > Also, there's kind of mess in node authentication logic. There are at
> least
> > two components responsible for it: DiscoverySpiNodeAuthenticator (which
> is
> > forcibly set by GridDiscoveryManager, but in fact public) and
> > GridSecurityProcessor (which performs actual node auth logic, but
> private).
> > I also don't understand why we need both
> > #authenticate(AuthenticationContext) and #authenticateNode(ClusterNode,
> > SecurityCredentials) methods while it's possible to set explicit
> > SecuritySubjectType.REMOTE_NODE in AuthenticationContext (this is
> arguable;
> > perhaps there are strong reasons).
> >
> > Finally, areas of responsibility between IgniteSecurity and
> > GridSecurityProcessor are kind of mixed. As far as I understand, the
> first
> > is responsible for Ignite-internal management of security logic (keeping
> > thread-local context, caching security contexts, etc; we don't expect
> > IgniteSecurity to be replaced by plugin provider) and the latter is
> > responsible for user-custom authentication / authorization logic. To be
> > honest, it took plenty of time to figure this out for me.
> >
> > From my point of view, we should make GridSecurityProcessor interface
> > public, rename it (it requires plenty of time to find the difference from
> > IgniteSecurity), make its API as simple and non-duplicating as possible
> and
> > clarify its area of responsibility (e.g. should it be responsible for
> > propagation of successfully authenticated subject among all nodes or
> not?)
> > to make it easy to embed custom security logic in Ignite.
> >
> > Regarding thin clients fix: implementation made by Denis suits better to
> > the very implicit contract that it's better to change API contracts of an
> > internal IgniteSecurity than of internal GridSecurityProcessor (which
> > actually mustn't be internal).
> >
> > > My approach doesn't require any IEPs, just minor change in code and to
> > >
> > >
> >
> org.apache.ignite.internal.processors.security.IgniteSecurity#authenticate(AuthenticationContext)
> > > contract.
> >
> > Looks like a misuse of #authenticate method to me. It should perform
> > initial authentication based on credentials (this may include queries to
> > external authentication subsystem, e.g. LDAP). User may want to don't
> > authenticate thin client on every node (this will increase the number of
> > requests to auth subsystem unless user implicitly implements propagation
> of
> > thin clients' contexts between nodes and make #authenticate cluster-wide
> > idempotent: first call should perform actual authentication, next calls
> > should retrieve context of already authenticated client). Presence of the
> > separate #securityContext(UUID) highlights that user indeed should care
> > about propagation of thin clients' contexts between the cluster nodes.
> >
> > --
> > Ivan
> >
> > On Fri, Mar 20, 2020 at 12:22 PM Veena Mithare  >
> > wrote:
> >
> > > Hi Alexei, Denis,
> > >
> > > One of the main usecases of thin client authentication is to be able to
> > > audit the 

Re: Data vanished from cluster after INACTIVE/ACTIVE switch

[Nikolay Izhikov]

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

Re: Data vanished from cluster after INACTIVE/ACTIVE switch

[Alexei Scherbakov]

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 :

> 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 
> написал(а):
> >
> > 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 
> wrote:
> >
> >> Nikolay, I think we should reconsider clearing at least system caches
> >> when deactivating.
> >>
> >> 17.03.2020 14:18, Nikolay Izhikov пишет:
> >>> Hello,