Introduce separate SQL configuration

[Taras Ledkov]

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

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

[slava.koptilin]

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

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

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

--
Taras Ledkov
Mail-To: [hidden email]

[Ilya Kasnacheev]

Hello!

I think that we should defer changes to IgniteConfiguration to  AI 3.0.

Maybe it will motivate us to start work on it.

Just my opinion, without any reservations.

Regards.
--
Ilya Kasnacheev


пн, 23 мар. 2020 г. в 13:04, Taras Ledkov <[hidden email]>:

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

[slava.koptilin]

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

> 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 <[hidden email]>:
> >
> >> 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 <[hidden email]>
> 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: [hidden email]
> >>>
> >>>
> --
> Taras Ledkov
> Mail-To: [hidden email]
>
>