You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by nabarun nag <nn...@pivotal.io> on 2016/06/13 19:33:42 UTC

Review Request 48661: GEODE-1543: Changed the log level from error to warn

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48661/
-----------------------------------------------------------

Review request for geode, anilkumar gingade, Jason Huynh, Dan Smith, and xiaojian zhou.


Repository: geode


Description
-------

* When multiple peers join the cluster conncurrently, only the first one that joins will distribute the index creation and other will try to create the index locally because of the distributed message and then log an error afterwards.
* This is an expected behaviour and hence should not be logged as error but rather as a warning.


Diffs
-----

  geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 8b9664f 

Diff: https://reviews.apache.org/r/48661/diff/


Testing
-------


Thanks,

nabarun nag


Re: Review Request 48661: GEODE-1543: Changed the log level from error to warn

Posted by Jason Huynh <hu...@gmail.com>.

> On June 13, 2016, 9:33 p.m., Dan Smith wrote:
> > If this is expected behavior, I think this message should be info level or lower. warning still indicates that something is going wrong.

I agree, probably should be info for this.


- Jason


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48661/#review137390
-----------------------------------------------------------


On June 13, 2016, 7:33 p.m., nabarun nag wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48661/
> -----------------------------------------------------------
> 
> (Updated June 13, 2016, 7:33 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Jason Huynh, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> * When multiple peers join the cluster conncurrently, only the first one that joins will distribute the index creation and other will try to create the index locally because of the distributed message and then log an error afterwards.
> * This is an expected behaviour and hence should not be logged as error but rather as a warning.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 8b9664f 
> 
> Diff: https://reviews.apache.org/r/48661/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> nabarun nag
> 
>


Re: Review Request 48661: GEODE-1543: Changed the log level from error to warn

Posted by nabarun nag <nn...@pivotal.io>.

> On June 13, 2016, 9:33 p.m., Dan Smith wrote:
> > If this is expected behavior, I think this message should be info level or lower. warning still indicates that something is going wrong.
> 
> Jason Huynh wrote:
>     I agree, probably should be info for this.
> 
> nabarun nag wrote:
>     I was wondering that the catch block is for general exception ( Exception ex ) which might occur for multiple reaons which may not have been an expected scenario. (The try block had multiple lines of code and i thought that an exception may occur at any point).
>     
>     I think that we should have a separate catch block for the expected scenario with the info level message and leave the general exception catch block with the error level log message.
>     
>     I might be wrong about this :(
> 
> Dan Smith wrote:
>     That seems like a good idea.

After further analysis, I found that there are two checked exceptions which were caught by the previous generic catch block. RegionNotFound Exception and IndexExistsException. RegionNotFoundException will now be treated as an InternalGemFireError and rethrown. However, the IndexExistsException will require more detailed analysis. TBD


- nabarun


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48661/#review137390
-----------------------------------------------------------


On June 13, 2016, 7:33 p.m., nabarun nag wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48661/
> -----------------------------------------------------------
> 
> (Updated June 13, 2016, 7:33 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Jason Huynh, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> * When multiple peers join the cluster conncurrently, only the first one that joins will distribute the index creation and other will try to create the index locally because of the distributed message and then log an error afterwards.
> * This is an expected behaviour and hence should not be logged as error but rather as a warning.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 8b9664f 
> 
> Diff: https://reviews.apache.org/r/48661/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> nabarun nag
> 
>


Re: Review Request 48661: GEODE-1543: Changed the log level from error to warn

Posted by Dan Smith <ds...@pivotal.io>.

> On June 13, 2016, 9:33 p.m., Dan Smith wrote:
> > If this is expected behavior, I think this message should be info level or lower. warning still indicates that something is going wrong.
> 
> Jason Huynh wrote:
>     I agree, probably should be info for this.
> 
> nabarun nag wrote:
>     I was wondering that the catch block is for general exception ( Exception ex ) which might occur for multiple reaons which may not have been an expected scenario. (The try block had multiple lines of code and i thought that an exception may occur at any point).
>     
>     I think that we should have a separate catch block for the expected scenario with the info level message and leave the general exception catch block with the error level log message.
>     
>     I might be wrong about this :(

That seems like a good idea.


- Dan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48661/#review137390
-----------------------------------------------------------


On June 13, 2016, 7:33 p.m., nabarun nag wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48661/
> -----------------------------------------------------------
> 
> (Updated June 13, 2016, 7:33 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Jason Huynh, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> * When multiple peers join the cluster conncurrently, only the first one that joins will distribute the index creation and other will try to create the index locally because of the distributed message and then log an error afterwards.
> * This is an expected behaviour and hence should not be logged as error but rather as a warning.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 8b9664f 
> 
> Diff: https://reviews.apache.org/r/48661/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> nabarun nag
> 
>


Re: Review Request 48661: GEODE-1543: Changed the log level from error to warn

Posted by nabarun nag <nn...@pivotal.io>.

> On June 13, 2016, 9:33 p.m., Dan Smith wrote:
> > If this is expected behavior, I think this message should be info level or lower. warning still indicates that something is going wrong.
> 
> Jason Huynh wrote:
>     I agree, probably should be info for this.

I was wondering that the catch block is for general exception ( Exception ex ) which might occur for multiple reaons which may not have been an expected scenario. (The try block had multiple lines of code and i thought that an exception may occur at any point).

I think that we should have a separate catch block for the expected scenario with the info level message and leave the general exception catch block with the error level log message.

I might be wrong about this :(


- nabarun


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48661/#review137390
-----------------------------------------------------------


On June 13, 2016, 7:33 p.m., nabarun nag wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48661/
> -----------------------------------------------------------
> 
> (Updated June 13, 2016, 7:33 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Jason Huynh, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> * When multiple peers join the cluster conncurrently, only the first one that joins will distribute the index creation and other will try to create the index locally because of the distributed message and then log an error afterwards.
> * This is an expected behaviour and hence should not be logged as error but rather as a warning.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 8b9664f 
> 
> Diff: https://reviews.apache.org/r/48661/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> nabarun nag
> 
>


Re: Review Request 48661: GEODE-1543: Changed the log level from error to warn

Posted by Dan Smith <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48661/#review137390
-----------------------------------------------------------



If this is expected behavior, I think this message should be info level or lower. warning still indicates that something is going wrong.

- Dan Smith


On June 13, 2016, 7:33 p.m., nabarun nag wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48661/
> -----------------------------------------------------------
> 
> (Updated June 13, 2016, 7:33 p.m.)
> 
> 
> Review request for geode, anilkumar gingade, Jason Huynh, Dan Smith, and xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> * When multiple peers join the cluster conncurrently, only the first one that joins will distribute the index creation and other will try to create the index locally because of the distributed message and then log an error afterwards.
> * This is an expected behaviour and hence should not be logged as error but rather as a warning.
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/com/gemstone/gemfire/internal/cache/LocalRegion.java 8b9664f 
> 
> Diff: https://reviews.apache.org/r/48661/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> nabarun nag
> 
>