You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by "Sunny Chan, CLSA" <su...@clsa.com> on 2019/12/20 06:44:02 UTC

RE: Log level changes in GridCacheWriteBehindStore.updateStore

Sorry for taking so long, but it has dropped down my priority list :(

I have now provided a github pull request for the logging changes I would like to make. Please review and let me know whether my patch is acceptable

-----Original Message-----
From: Sunny Chan, CLSA 
Sent: Friday, August 30, 2019 4:25 PM
To: Ilya Kasnacheev <il...@gmail.com>; dev <de...@ignite.apache.org>
Subject: RE: Log level changes in GridCacheWriteBehindStore.updateStore

I have created a ticket (https://issues.apache.org/jira/browse/IGNITE-12120) for this, and I will provide a patch shortly.

-----Original Message-----
From: Ilya Kasnacheev <il...@gmail.com>
Sent: Wednesday, August 28, 2019 12:05 AM
To: dev <de...@ignite.apache.org>
Subject: Re: Log level changes in GridCacheWriteBehindStore.updateStore

Hello!

I think you should go ahead and create a JIRA ticket about it, then someone (or even you) can check it and fix it.

Regards,
--
Ilya Kasnacheev


пн, 26 авг. 2019 г. в 09:17, Sunny Chan, CLSA <su...@clsa.com>:

> Hello,
>
>
>
> In the GridCacheWriteBehindStore
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_apach
> e_ignite_blob_7e73098d4d6e3d5f78326cb11dac7e083a2312dd_modules_core_sr
> c_main_java_org_apache_ignite_internal_processors_cache_store_GridCach
> eWriteBehindStore.java-23L893&d=DwIFaQ&c=lxzXOFU02467FL7HOPRqCw&r=9dPM
> IMeLdcqo3dX29zZZ4USXrC7gp_0p-Fvzzp2WzgU&m=JOamAEwUhUCz7wcYbnKkCfpwnG8q
> tB1U_izUlGW4qDA&s=W9vOm8Nf_aH8d8yXHGBFLuXIzT8uzT3iteuXahcxyEw&e= >, 
> when the updateStore failed to write to underlying store, it logs this 
> as
> error:
>
>
>
> LT.error(log, e, "Unable to update underlying store: " + store);
>
>
>
> After this line logged the error, it would return false so that it 
> would retry the store (by returning false).
>
>
>
> While later on in the updatStore function, when the writeCache 
> overflows, it would log this:
>
>
>
> log.warning("Failed to update store (value will be lost as current 
> buffer size is greater " + …
>
>
>
> then it will remove the failed entry.
>
>
>
> I think the severity of the log messages is not right, as the fail 
> update would still be retried.
>
>
>
> So I propose to change the log severity level so that the first one 
> would be a warn, and second one would be error. Is that acceptable?
>
>
>
> *Sunny Chan*
>
> *Senior Lead Engineer, Executive Services*
>
> D  +852 2600 8907  |  M  +852 6386 1835  |  T  +852 2600 8888
>
> 5/F, One Island East, 18 Westlands Road, Island East, Hong Kong
>
>
>
> [image: :1. Social Media Icons:CLSA_Social Media Icons_linkedin.png] 
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__hk.linkedin.com_
> company_clsa&d=DwIFaQ&c=lxzXOFU02467FL7HOPRqCw&r=9dPMIMeLdcqo3dX29zZZ4
> USXrC7gp_0p-Fvzzp2WzgU&m=JOamAEwUhUCz7wcYbnKkCfpwnG8qtB1U_izUlGW4qDA&s
> =HPrBHWKCEIOY9wK-xT79ymzAzy8nBOh1YzieNbw0l24&e= >[image: :1. Social 
> Media Icons:CLSA_Social Media Icons_twitter.png] <https://urldefense.proofpoint.com/v2/url?u=https-3A__twitter.com_clsainsights-3Flang-3Den&d=DwIFaQ&c=lxzXOFU02467FL7HOPRqCw&r=9dPMIMeLdcqo3dX29zZZ4USXrC7gp_0p-Fvzzp2WzgU&m=JOamAEwUhUCz7wcYbnKkCfpwnG8qtB1U_izUlGW4qDA&s=vjJgUqAlFGm2LOO2lh4niU9ztMI8kiWXazxhkof8XMc&e= >[image: :1. Social Media Icons:CLSA_Social Media Icons_youtube.png] <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.youtube.com_channel_UC0qWp-5FlLnOcRYmBlCNQgZKA&d=DwIFaQ&c=lxzXOFU02467FL7HOPRqCw&r=9dPMIMeLdcqo3dX29zZZ4USXrC7gp_0p-Fvzzp2WzgU&m=JOamAEwUhUCz7wcYbnKkCfpwnG8qtB1U_izUlGW4qDA&s=R62RtJ6zWcytHsNFpFvxr88mDob2r_RXHmMSSYCtS9I&e= >[image: :1.
> Social Media Icons:CLSA_Social Media Icons_facebook.png] 
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.facebook.com
> _clsacommunity_&d=DwIFaQ&c=lxzXOFU02467FL7HOPRqCw&r=9dPMIMeLdcqo3dX29z
> ZZ4USXrC7gp_0p-Fvzzp2WzgU&m=JOamAEwUhUCz7wcYbnKkCfpwnG8qtB1U_izUlGW4qD
> A&s=r1KlYPqSQyBKNAfdIHwO2M5EnTIFsi2dY59TmIsKE6Y&e= >
>
>
>
> *clsa.com* <https://www.clsa.com/>
>
> *Insights. Liquidity. Capital. *
>
>
>
> [image: CLSA_RGB] <https://www.clsa.com/member>
>
>
>
> *A CITIC Securities Company*
>
>
>
> The content of this communication is intended for the recipient and is 
> subject to CLSA Legal and Regulatory Notices.
> These can be viewed at https://www.clsa.com/disclaimer.html or sent to 
> you upon request.
> Please consider before printing. CLSA is ISO14001 certified and 
> committed to reducing its impact on the environment.
>

The content of this communication is intended for the recipient and is subject to CLSA Legal and Regulatory Notices.
These can be viewed at https://www.clsa.com/disclaimer.html or sent to you upon request.
Please consider before printing. CLSA is ISO14001 certified and committed to reducing its impact on the environment.

Re: Log level changes in GridCacheWriteBehindStore.updateStore

Posted by Ilya Kasnacheev <il...@gmail.com>.
Hello!

I have merged your PR to master after some tweaks.

Regards,
-- 
Ilya Kasnacheev


пт, 20 дек. 2019 г. в 09:44, Sunny Chan, CLSA <su...@clsa.com>:

> Sorry for taking so long, but it has dropped down my priority list :(
>
> I have now provided a github pull request for the logging changes I would
> like to make. Please review and let me know whether my patch is acceptable
>
> -----Original Message-----
> From: Sunny Chan, CLSA
> Sent: Friday, August 30, 2019 4:25 PM
> To: Ilya Kasnacheev <il...@gmail.com>; dev <
> dev@ignite.apache.org>
> Subject: RE: Log level changes in GridCacheWriteBehindStore.updateStore
>
> I have created a ticket (
> https://issues.apache.org/jira/browse/IGNITE-12120) for this, and I will
> provide a patch shortly.
>
> -----Original Message-----
> From: Ilya Kasnacheev <il...@gmail.com>
> Sent: Wednesday, August 28, 2019 12:05 AM
> To: dev <de...@ignite.apache.org>
> Subject: Re: Log level changes in GridCacheWriteBehindStore.updateStore
>
> Hello!
>
> I think you should go ahead and create a JIRA ticket about it, then
> someone (or even you) can check it and fix it.
>
> Regards,
> --
> Ilya Kasnacheev
>
>
> пн, 26 авг. 2019 г. в 09:17, Sunny Chan, CLSA <su...@clsa.com>:
>
> > Hello,
> >
> >
> >
> > In the GridCacheWriteBehindStore
> > <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_apach
> > e_ignite_blob_7e73098d4d6e3d5f78326cb11dac7e083a2312dd_modules_core_sr
> > c_main_java_org_apache_ignite_internal_processors_cache_store_GridCach
> > eWriteBehindStore.java-23L893&d=DwIFaQ&c=lxzXOFU02467FL7HOPRqCw&r=9dPM
> > IMeLdcqo3dX29zZZ4USXrC7gp_0p-Fvzzp2WzgU&m=JOamAEwUhUCz7wcYbnKkCfpwnG8q
> > tB1U_izUlGW4qDA&s=W9vOm8Nf_aH8d8yXHGBFLuXIzT8uzT3iteuXahcxyEw&e= >,
> > when the updateStore failed to write to underlying store, it logs this
> > as
> > error:
> >
> >
> >
> > LT.error(log, e, "Unable to update underlying store: " + store);
> >
> >
> >
> > After this line logged the error, it would return false so that it
> > would retry the store (by returning false).
> >
> >
> >
> > While later on in the updatStore function, when the writeCache
> > overflows, it would log this:
> >
> >
> >
> > log.warning("Failed to update store (value will be lost as current
> > buffer size is greater " + …
> >
> >
> >
> > then it will remove the failed entry.
> >
> >
> >
> > I think the severity of the log messages is not right, as the fail
> > update would still be retried.
> >
> >
> >
> > So I propose to change the log severity level so that the first one
> > would be a warn, and second one would be error. Is that acceptable?
> >
> >
> >
> > *Sunny Chan*
> >
> > *Senior Lead Engineer, Executive Services*
> >
> > D  +852 2600 8907  |  M  +852 6386 1835  |  T  +852 2600 8888
> >
> > 5/F, One Island East, 18 Westlands Road, Island East, Hong Kong
> >
> >
> >
> > [image: :1. Social Media Icons:CLSA_Social Media Icons_linkedin.png]
> > <https://urldefense.proofpoint.com/v2/url?u=https-3A__hk.linkedin.com_
> > company_clsa&d=DwIFaQ&c=lxzXOFU02467FL7HOPRqCw&r=9dPMIMeLdcqo3dX29zZZ4
> > USXrC7gp_0p-Fvzzp2WzgU&m=JOamAEwUhUCz7wcYbnKkCfpwnG8qtB1U_izUlGW4qDA&s
> > =HPrBHWKCEIOY9wK-xT79ymzAzy8nBOh1YzieNbw0l24&e= >[image: :1. Social
> > Media Icons:CLSA_Social Media Icons_twitter.png] <
> https://urldefense.proofpoint.com/v2/url?u=https-3A__twitter.com_clsainsights-3Flang-3Den&d=DwIFaQ&c=lxzXOFU02467FL7HOPRqCw&r=9dPMIMeLdcqo3dX29zZZ4USXrC7gp_0p-Fvzzp2WzgU&m=JOamAEwUhUCz7wcYbnKkCfpwnG8qtB1U_izUlGW4qDA&s=vjJgUqAlFGm2LOO2lh4niU9ztMI8kiWXazxhkof8XMc&e=
> >[image: :1. Social Media Icons:CLSA_Social Media Icons_youtube.png] <
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.youtube.com_channel_UC0qWp-5FlLnOcRYmBlCNQgZKA&d=DwIFaQ&c=lxzXOFU02467FL7HOPRqCw&r=9dPMIMeLdcqo3dX29zZZ4USXrC7gp_0p-Fvzzp2WzgU&m=JOamAEwUhUCz7wcYbnKkCfpwnG8qtB1U_izUlGW4qDA&s=R62RtJ6zWcytHsNFpFvxr88mDob2r_RXHmMSSYCtS9I&e=
> >[image: :1.
> > Social Media Icons:CLSA_Social Media Icons_facebook.png]
> > <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.facebook.com
> > _clsacommunity_&d=DwIFaQ&c=lxzXOFU02467FL7HOPRqCw&r=9dPMIMeLdcqo3dX29z
> > ZZ4USXrC7gp_0p-Fvzzp2WzgU&m=JOamAEwUhUCz7wcYbnKkCfpwnG8qtB1U_izUlGW4qD
> > A&s=r1KlYPqSQyBKNAfdIHwO2M5EnTIFsi2dY59TmIsKE6Y&e= >
> >
> >
> >
> > *clsa.com* <https://www.clsa.com/>
> >
> > *Insights. Liquidity. Capital. *
> >
> >
> >
> > [image: CLSA_RGB] <https://www.clsa.com/member>
> >
> >
> >
> > *A CITIC Securities Company*
> >
> >
> >
> > The content of this communication is intended for the recipient and is
> > subject to CLSA Legal and Regulatory Notices.
> > These can be viewed at https://www.clsa.com/disclaimer.html or sent to
> > you upon request.
> > Please consider before printing. CLSA is ISO14001 certified and
> > committed to reducing its impact on the environment.
> >
>
> The content of this communication is intended for the recipient and is
> subject to CLSA Legal and Regulatory Notices.
> These can be viewed at https://www.clsa.com/disclaimer.html or sent to
> you upon request.
> Please consider before printing. CLSA is ISO14001 certified and committed
> to reducing its impact on the environment.
>