Re: Re[2]: [DISCUSSION] MaxLineLength checkstyle rule

2021-05-09 Thread Ivan Pavlukhin
+1 for 140 lines compromise.
+1 if someone is ready to fix everything to fit 120.

BTW, was there any progress with this?

2021-04-15 21:41 GMT+03:00, Maxim Muzafarov :
> Folks,
>
> I've briefly checked the total amount of the max length violations:
>
> 120 line length - 5540 violations
> 130 line length - 1891 violations
> 140 line length - 895 violations
> 150 line length - 478 violations
>
>
> I think the 140 max line length might be the best option for us.
>
> On Thu, 15 Apr 2021 at 14:51, Ivan Daschinsky  wrote:
>>
>> But super long lines are a real problem while merging. It is super
>> inconvenient.
>> 120 chars is a good compromise.
>>
>> чт, 15 апр. 2021 г. в 14:39, Zhenya Stanilovsky
>> > >:
>>
>> >
>> > Python is not so verbose as java )
>> > +1 for 140
>> >
>> > >Hi!
>> > >Personally, I suppose that 120 chars per line is OK. Moreover, many
>> > >codestyles suggests less chars per line.
>> > >For example PEP8 recommends 80 (but we use 120 in pyignite and flake8
>> > >codestyle checks it). Google java codestyle insists on 100.
>> > >
>> > >More than 120 chars is too long as for me and is not convenient for
>> > > 3-way
>> > >merges.
>> > >
>> > >чт, 15 апр. 2021 г. в 12:28, Nikolay Izhikov < nizhi...@apache.org >:
>> > >
>> > >> Hello, Ilya.
>> > >>
>> > >> Thanks for the feedback.
>> > >>
>> > >> 140 characters is fine for me.
>> > >>
>> > >> > 15 апр. 2021 г., в 12:25, Ilya Kasnacheev <
>> > >> > ilya.kasnach...@gmail.com
>> > >
>> > >> написал(а):
>> > >> >
>> > >> > Hello!
>> > >> >
>> > >> > Please find attached the distribution of line lengths in the
>> > >> > project,
>> > in
>> > >> the form of (count, line length).
>> > >> >
>> > >> > I think that we can enforce a hard limit of 140 chars per line. I
>> > think
>> > >> that having longer lines is excessive and does not benefit
>> > >> readability.
>> > >> >
>> > >> > Having a limit of 150 or 180 does not give us much since there's
>> > >> > still
>> > >> a long tail which has to be fixed.
>> > >> >
>> > >> > Regards,
>> > >> > --
>> > >> > Ilya Kasnacheev
>> > >> >
>> > >> >
>> > >> > чт, 15 апр. 2021 г. в 11:30, Nikolay Izhikov < nizhi...@apache.org
>> > >> > >:
>> > >> > Hello, Igniters.
>> > >> >
>> > >> > Right now, we have a code style rule [1] - the line should fit in
>> > >> > 120
>> > >> characters.
>> > >> > But, this rule violated in many and many places through code.
>> > >> > I have a plan to add a check style rule to force maximum line
>> > >> > length.
>> > >> >
>> > >> > For me, personally, 120 characters a bit old-fashioned
>> > >> > restriction.
>> > >> > Should we increase the maximum line length to 150 or even 180
>> > characters?
>> > >> >
>> > >> > [1]
>> > https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines
>> > >> > 
>> > >>
>> > >>
>> > >--
>> > >Sincerely yours, Ivan Daschinskiy
>> >
>> >
>> >
>> >
>>
>>
>>
>> --
>> Sincerely yours, Ivan Daschinskiy
>


-- 

Best regards,
Ivan Pavlukhin


[MTCGA]: new failures in builds [5999963] needs to be handled

2021-05-09 Thread dpavlov . tasks
Hi Igniters,

 I've detected some new issue on TeamCity to be handled. You are more than 
welcomed to help.

 *New Critical Failure in master RDD 
https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_Rdd?branch=%3Cdefault%3E
 No changes in the build

 - Here's a reminder of what contributors were agreed to do 
https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute 
 - Should you have any questions please contact dev@ignite.apache.org 

Best Regards,
Apache Ignite TeamCity Bot 
https://github.com/apache/ignite-teamcity-bot
Notification generated at 22:25:19 09-05-2021 


Re: Exceeding the DataStorageConfiguration#getMaxWalArchiveSize due to historical rebalance

2021-05-09 Thread Stanislav Lukyanov
Discuss this with Kirill verbally.

Kirill showed me that having the min threshold doesn't quite work.
It doesn't work because we no longer know how much WAL we should remove if we 
reach getMaxWalArchiveSize.

For example, say we have minWalArchiveTimespan=2 hours and 
maxWalArchiveSize=2GB.
Say, under normal load on stable topology 2 hours of WAL use 1 GB of space.
Now, say we're doing historical rebalance and reserve the WAL archive.
The WAL archive starts growing and soon it occupies 2 GB.
Now what?
We're supposed to give up WAL reservations and start agressively removing WAL 
archive.
But it is not clear when can we stop removing WAL archive - since last 2 hours 
of WAL are larger than our maxWalArchiveSize
there is no meaningful point the system can use as a "minimum" WAL size.

I understand the description above is a bit messy but I believe that whoever is 
interested in this will understand it
after drawing this on paper.


I'm giving up on my latest suggestion about time-based minimum. Let's keep it 
simple.

I suggest the minWalArchiveSize and maxWalArchvieSize properties as the 
solution,
with the behavior as initially described by Kirill.

Stan


> On 7 May 2021, at 15:09, ткаленко кирилл  wrote:
> 
> Stas hello!
> 
> I didn't quite get your last idea. 
> What will we do if we reach getMaxWalArchiveSize? Shall we not delete the 
> segment until minWalArchiveTimespan?
> 
> 06.05.2021, 20:00, "Stanislav Lukyanov" :
>> An interesting suggestion I heard today.
>> 
>> The minWalArchiveSize property might actually be minWalArchiveTimespan - 
>> i.e. be a number of seconds instead of a number of bytes!
>> 
>> I think this makes perfect sense from the user point of view.
>> "I want to have WAL archive for at least N hours but I have a limit of M 
>> gigabytes to store it".
>> 
>> Do we have checkpoint timestamp stored anywhere? (cp start markers?)
>> Perhaps we can actually implement this?
>> 
>> Thanks,
>> Stan
>> 
>>>  On 6 May 2021, at 14:13, Stanislav Lukyanov  wrote:
>>> 
>>>  +1 to cancel WAL reservation on reaching getMaxWalArchiveSize
>>>  +1 to add a public property to replace 
>>> IGNITE_THRESHOLD_WAL_ARCHIVE_SIZE_PERCENTAGE
>>> 
>>>  I don't like the name getWalArchiveSize - I think it's a bit confusing (is 
>>> it the current size? the minimal size? the target size?)
>>>  I suggest to name the property geMintWalArchiveSize. I think that this is 
>>> exactly what it is - the minimal size of the archive that we want to have.
>>>  The archive size at all times should be between min and max.
>>>  If archive size is less than min or more than max then the system 
>>> functionality can degrade (e.g. historical rebalance may not work as 
>>> expected).
>>>  I think these rules are intuitively understood from the "min" and "max" 
>>> names.
>>> 
>>>  Ilya's suggestion about throttling is great although I'd do this in a 
>>> different ticket.
>>> 
>>>  Thanks,
>>>  Stan
>>> 
  On 5 May 2021, at 19:25, Maxim Muzafarov  wrote:
 
  Hello, Kirill
 
  +1 for this change, however, there are too many configuration settings
  that exist for the user to configure Ignite cluster. It is better to
  keep the options that we already have and fix the behaviour of the
  rebalance process as you suggested.
 
  On Tue, 4 May 2021 at 19:01, ткаленко кирилл  wrote:
>  Hi Ilya!
> 
>  Then we can greatly reduce the user load on the cluster until the 
> rebalance is over. Which can be critical for the user.
> 
>  04.05.2021, 18:43, "Ilya Kasnacheev" :
>>  Hello!
>> 
>>  Maybe we can have a mechanic here similar (or equal) to checkpoint based
>>  write throttling?
>> 
>>  So we will be throttling for both checkpoint page buffer and WAL limit.
>> 
>>  Regards,
>>  --
>>  Ilya Kasnacheev
>> 
>>  вт, 4 мая 2021 г. в 11:29, ткаленко кирилл :
>> 
>>>  Hello everybody!
>>> 
>>>  At the moment, if there are partitions for the rebalance for which the
>>>  historical rebalance will be used, then we reserve segments in the WAL
>>>  archive (we do not allow cleaning the WAL archive) until the rebalance 
>>> for
>>>  all cache groups is over.
>>> 
>>>  If a cluster is under load during the rebalance, WAL archive size may
>>>  significantly exceed limits set in
>>>  DataStorageConfiguration#getMaxWalArchiveSize until the process is
>>>  complete. This may lead to user issues and nodes may crash with the "No
>>>  space left on device" error.
>>> 
>>>  We have a system property IGNITE_THRESHOLD_WAL_ARCHIVE_SIZE_PERCENTAGE 
>>> by
>>>  default 0.5, which sets the threshold (multiplied by 
>>> getMaxWalArchiveSize)
>>>  from which and up to which the WAL archive will be cleared, i.e. sets 
>>> the
>>>  size of the WAL archive that will always be on the node. I propose to
>>>  replace this system property with the
>>>  DataStorageConfi

Re: Change IGNITE_PDS_WAL_REBALANCE_THRESHOLD from System property to Configuraton

2021-05-09 Thread Atri Sharma
+1

On Sun, 9 May 2021, 21:33 Stanislav Lukyanov, 
wrote:

> Hi Eduard,
>
> I strongly believe that if a configuration option is cluster wide then it
> belongs to distributed metastore and not to IgniteConfiguration.
> This allows to get cluster-wide consistency guarantees and API for dynamic
> change out of the box (need to teach the internals to re-read the property
> from DMS every time of course).
>
> WDYT?
>
> Stan
>
> > On 6 May 2021, at 16:35, Eduard Rakhmankulov 
> wrote:
> >
> > Some addition.
> >
> > I want to add configuration to
> >
> org.apache.ignite.configuration.DataStorageConfiguration#getDefaultWarmUpConfiguration#getP
> > artitionWalRebalanceThreshold
> > which will have same semantics as system property (number of entries in
> WAL
> > to trigger rebalance).
> >
> > On Thu, 6 May 2021 at 15:50, Eduard Rakhmankulov 
> > wrote:
> >
> >> Hello, Igniters!
> >>
> >> I suggest changing IGNITE_PDS_WAL_REBALANCE_THRESHOLD from system
> >> properties to IgniteConfiguration.
> >> This configuration is effectively cluster-wide (because only the
> >> coordinator's configuration matters when the heuristic with this
> property
> >> applies).
> >>
> >> It is easier to validate that we have the same configuration on all
> nodes
> >> than system property (in the case when another coordinator was elected).
> >>
> >> --
> >> Best regards, Eduard.
> >>
> >
> >
> > --
> > С уважением, Рахманкулов Э.Р.
>
>


Re: Change IGNITE_PDS_WAL_REBALANCE_THRESHOLD from System property to Configuraton

2021-05-09 Thread Stanislav Lukyanov
Hi Eduard,

I strongly believe that if a configuration option is cluster wide then it 
belongs to distributed metastore and not to IgniteConfiguration.
This allows to get cluster-wide consistency guarantees and API for dynamic 
change out of the box (need to teach the internals to re-read the property from 
DMS every time of course).

WDYT?

Stan

> On 6 May 2021, at 16:35, Eduard Rakhmankulov  wrote:
> 
> Some addition.
> 
> I want to add configuration to
> org.apache.ignite.configuration.DataStorageConfiguration#getDefaultWarmUpConfiguration#getP
> artitionWalRebalanceThreshold
> which will have same semantics as system property (number of entries in WAL
> to trigger rebalance).
> 
> On Thu, 6 May 2021 at 15:50, Eduard Rakhmankulov 
> wrote:
> 
>> Hello, Igniters!
>> 
>> I suggest changing IGNITE_PDS_WAL_REBALANCE_THRESHOLD from system
>> properties to IgniteConfiguration.
>> This configuration is effectively cluster-wide (because only the
>> coordinator's configuration matters when the heuristic with this property
>> applies).
>> 
>> It is easier to validate that we have the same configuration on all nodes
>> than system property (in the case when another coordinator was elected).
>> 
>> --
>> Best regards, Eduard.
>> 
> 
> 
> -- 
> С уважением, Рахманкулов Э.Р.



[MTCGA]: new failures in builds [5999911] needs to be handled

2021-05-09 Thread dpavlov . tasks
Hi Igniters,

 I've detected some new issue on TeamCity to be handled. You are more than 
welcomed to help.

 *New Critical Failure in master Control Utility 
https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_ControlUtility?branch=%3Cdefault%3E
 No changes in the build

 - Here's a reminder of what contributors were agreed to do 
https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute 
 - Should you have any questions please contact dev@ignite.apache.org 

Best Regards,
Apache Ignite TeamCity Bot 
https://github.com/apache/ignite-teamcity-bot
Notification generated at 18:25:19 09-05-2021 


[MTCGA]: new failures in builds [5999486] needs to be handled

2021-05-09 Thread dpavlov . tasks
Hi Igniters,

 I've detected some new issue on TeamCity to be handled. You are more than 
welcomed to help.

 *New Critical Failure in master Basic 1 
https://ci.ignite.apache.org/buildConfiguration/IgniteTests24Java8_Basic1?branch=%3Cdefault%3E
 No changes in the build

 - Here's a reminder of what contributors were agreed to do 
https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute 
 - Should you have any questions please contact dev@ignite.apache.org 

Best Regards,
Apache Ignite TeamCity Bot 
https://github.com/apache/ignite-teamcity-bot
Notification generated at 10:40:20 09-05-2021