You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Robert Nettleton <rn...@hortonworks.com> on 2016/04/25 17:29:00 UTC

Review Request 46639: Add counter limits to LogSearch integration error logs that occur during failed connection attempts

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

Review request for Ambari, Oliver Szabo and Sumit Mohanty.


Bugs: AMBARI-16093
    https://issues.apache.org/jira/browse/AMBARI-16093


Repository: ambari


Description
-------

This patch resolves AMBARI-16093.

Hundreds of repeated error logs were occuring in instances when the LogSearch server was unexpectedly shutdown, and Ambari Server was not yet aware of the change in service state. 

The LoggingSearchPropertyProvider already waits until the LOGSEARCH_SERVER component is in the started state, but if that component shuts down unexpectedly, there will be a small time window during which the provider will attempt to connect to the LogSearch server, which will then fail and generate error logs.

In order to resolve this logging issue, this patch implements the following:

1. Implements a new utility class to handle checks on a specified counter, which can be used to limit the number of times a given error log is included in ambari-server.log. 
2. Modifies the LoggingSearchPropertyProvider and LoggingRequestHelperImpl classes to use this new utility class in cases where this problem of repeated logging may occur.  
3. Implements a new unit test case for the new utility class, to verify that a given log is only passed to the logger in certain circumstances, based on the value of the specified counter.


Diffs
-----

  ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LoggingRequestHelperImpl.java a5cd369 
  ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LoggingSearchPropertyProvider.java e2da4c8 
  ambari-server/src/main/java/org/apache/ambari/server/controller/logging/Utils.java PRE-CREATION 
  ambari-server/src/test/java/org/apache/ambari/server/controller/logging/UtilsTest.java PRE-CREATION 

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


Testing
-------

1. Deployed a 1-VM cluster without LogSearch enabled, verified that the cluster deployed successfully, and that no extraneous error logging occurring in ambari-server.log or ambari-server.out.
2. Deployed a 1-VM cluster with LogSearch enabled, verified that the cluster deployed successfully, verified that the LogSearch Integration APIs are functioning as expected.  As part of this test, I also killed the LogSearch Server with "kill -9", accessed the various portions of the UI that rely on the HostComponent REST API, and verified that no extraneous logging was present in ambari-server.log or ambari-server.out.
3. Ran the ambari-server "mvn clean test" suite with my patch applied.  Test Results:

"
Results :

Tests run: 4273, Failures: 0, Errors: 0, Skipped: 32
"

"
INFO: AMBARI_SERVER_LIB is not set, using default /usr/lib/ambari-server
INFO: Return code from stack upgrade command, retcode = 0
StackAdvisor implementation for stack HDP1, version 2.0.6 was not found
Returning DefaultStackAdvisor implementation
StackAdvisor implementation for stack XYZ, version 1.0.0 was loaded
StackAdvisor implementation for stack XYZ, version 1.0.1 was loaded
Returning XYZ101StackAdvisor implementation
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 1:19:32.519s
[INFO] Finished at: Mon Apr 25 11:02:50 EDT 2016
[INFO] Final Memory: 38M/720M
[INFO] ————————————————————————————————————
"


Thanks,

Robert Nettleton


Re: Review Request 46639: Add counter limits to LogSearch integration error logs that occur during failed connection attempts

Posted by Robert Nettleton <rn...@hortonworks.com>.

> On April 25, 2016, 4 p.m., Oliver Szabo wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/logging/Utils.java, line 29
> > <https://reviews.apache.org/r/46639/diff/1/?file=1359667#file1359667line29>
> >
> >     Utils is too generic for me, can this be more specified ? (like SomethingUtils.java, i know its in a specified package, but maybe it would be easier to find with a less generic name)

I'm not sure I agree that the name is too generic, since as you mentioned it is scoped in the intended package.

As Sumit mentioned above, this might eventually be a useful utility for more than just this case, but I believe that the code might need some refactoring in order to make this generic enough to be useful across Ambari logging.

Since we'll probably do that in a future patch at some point, I'm less inclined to think that we should rename this now, since this functionality might be moved to a different, more common, package anyway in a future change.

Please let me know if you agree. 

Thanks.


- Robert


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


On April 25, 2016, 3:28 p.m., Robert Nettleton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46639/
> -----------------------------------------------------------
> 
> (Updated April 25, 2016, 3:28 p.m.)
> 
> 
> Review request for Ambari, Oliver Szabo and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-16093
>     https://issues.apache.org/jira/browse/AMBARI-16093
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> This patch resolves AMBARI-16093.
> 
> Hundreds of repeated error logs were occuring in instances when the LogSearch server was unexpectedly shutdown, and Ambari Server was not yet aware of the change in service state. 
> 
> The LoggingSearchPropertyProvider already waits until the LOGSEARCH_SERVER component is in the started state, but if that component shuts down unexpectedly, there will be a small time window during which the provider will attempt to connect to the LogSearch server, which will then fail and generate error logs.
> 
> In order to resolve this logging issue, this patch implements the following:
> 
> 1. Implements a new utility class to handle checks on a specified counter, which can be used to limit the number of times a given error log is included in ambari-server.log. 
> 2. Modifies the LoggingSearchPropertyProvider and LoggingRequestHelperImpl classes to use this new utility class in cases where this problem of repeated logging may occur.  
> 3. Implements a new unit test case for the new utility class, to verify that a given log is only passed to the logger in certain circumstances, based on the value of the specified counter.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LoggingRequestHelperImpl.java a5cd369 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LoggingSearchPropertyProvider.java e2da4c8 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/logging/Utils.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/logging/UtilsTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46639/diff/
> 
> 
> Testing
> -------
> 
> 1. Deployed a 1-VM cluster without LogSearch enabled, verified that the cluster deployed successfully, and that no extraneous error logging occurring in ambari-server.log or ambari-server.out.
> 2. Deployed a 1-VM cluster with LogSearch enabled, verified that the cluster deployed successfully, verified that the LogSearch Integration APIs are functioning as expected.  As part of this test, I also killed the LogSearch Server with "kill -9", accessed the various portions of the UI that rely on the HostComponent REST API, and verified that no extraneous logging was present in ambari-server.log or ambari-server.out.
> 3. Ran the ambari-server "mvn clean test" suite with my patch applied.  Test Results:
> 
> "
> Results :
> 
> Tests run: 4273, Failures: 0, Errors: 0, Skipped: 32
> "
> 
> "
> INFO: AMBARI_SERVER_LIB is not set, using default /usr/lib/ambari-server
> INFO: Return code from stack upgrade command, retcode = 0
> StackAdvisor implementation for stack HDP1, version 2.0.6 was not found
> Returning DefaultStackAdvisor implementation
> StackAdvisor implementation for stack XYZ, version 1.0.0 was loaded
> StackAdvisor implementation for stack XYZ, version 1.0.1 was loaded
> Returning XYZ101StackAdvisor implementation
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 1:19:32.519s
> [INFO] Finished at: Mon Apr 25 11:02:50 EDT 2016
> [INFO] Final Memory: 38M/720M
> [INFO] ————————————————————————————————————
> "
> 
> 
> Thanks,
> 
> Robert Nettleton
> 
>


Re: Review Request 46639: Add counter limits to LogSearch integration error logs that occur during failed connection attempts

Posted by Oliver Szabo <os...@hortonworks.com>.

> On April 25, 2016, 4 p.m., Oliver Szabo wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/logging/Utils.java, line 29
> > <https://reviews.apache.org/r/46639/diff/1/?file=1359667#file1359667line29>
> >
> >     Utils is too generic for me, can this be more specified ? (like SomethingUtils.java, i know its in a specified package, but maybe it would be easier to find with a less generic name)
> 
> Robert Nettleton wrote:
>     I'm not sure I agree that the name is too generic, since as you mentioned it is scoped in the intended package.
>     
>     As Sumit mentioned above, this might eventually be a useful utility for more than just this case, but I believe that the code might need some refactoring in order to make this generic enough to be useful across Ambari logging.
>     
>     Since we'll probably do that in a future patch at some point, I'm less inclined to think that we should rename this now, since this functionality might be moved to a different, more common, package anyway in a future change.
>     
>     Please let me know if you agree. 
>     
>     Thanks.

im okay with that now, so i will drop the issue, but if it will go into a more generic package (like org.apache.ambari.server.utils.* -> we have some Utils class there), then it would be better with a more specified name


- Oliver


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


On April 25, 2016, 3:28 p.m., Robert Nettleton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46639/
> -----------------------------------------------------------
> 
> (Updated April 25, 2016, 3:28 p.m.)
> 
> 
> Review request for Ambari, Oliver Szabo and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-16093
>     https://issues.apache.org/jira/browse/AMBARI-16093
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> This patch resolves AMBARI-16093.
> 
> Hundreds of repeated error logs were occuring in instances when the LogSearch server was unexpectedly shutdown, and Ambari Server was not yet aware of the change in service state. 
> 
> The LoggingSearchPropertyProvider already waits until the LOGSEARCH_SERVER component is in the started state, but if that component shuts down unexpectedly, there will be a small time window during which the provider will attempt to connect to the LogSearch server, which will then fail and generate error logs.
> 
> In order to resolve this logging issue, this patch implements the following:
> 
> 1. Implements a new utility class to handle checks on a specified counter, which can be used to limit the number of times a given error log is included in ambari-server.log. 
> 2. Modifies the LoggingSearchPropertyProvider and LoggingRequestHelperImpl classes to use this new utility class in cases where this problem of repeated logging may occur.  
> 3. Implements a new unit test case for the new utility class, to verify that a given log is only passed to the logger in certain circumstances, based on the value of the specified counter.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LoggingRequestHelperImpl.java a5cd369 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LoggingSearchPropertyProvider.java e2da4c8 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/logging/Utils.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/logging/UtilsTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46639/diff/
> 
> 
> Testing
> -------
> 
> 1. Deployed a 1-VM cluster without LogSearch enabled, verified that the cluster deployed successfully, and that no extraneous error logging occurring in ambari-server.log or ambari-server.out.
> 2. Deployed a 1-VM cluster with LogSearch enabled, verified that the cluster deployed successfully, verified that the LogSearch Integration APIs are functioning as expected.  As part of this test, I also killed the LogSearch Server with "kill -9", accessed the various portions of the UI that rely on the HostComponent REST API, and verified that no extraneous logging was present in ambari-server.log or ambari-server.out.
> 3. Ran the ambari-server "mvn clean test" suite with my patch applied.  Test Results:
> 
> "
> Results :
> 
> Tests run: 4273, Failures: 0, Errors: 0, Skipped: 32
> "
> 
> "
> INFO: AMBARI_SERVER_LIB is not set, using default /usr/lib/ambari-server
> INFO: Return code from stack upgrade command, retcode = 0
> StackAdvisor implementation for stack HDP1, version 2.0.6 was not found
> Returning DefaultStackAdvisor implementation
> StackAdvisor implementation for stack XYZ, version 1.0.0 was loaded
> StackAdvisor implementation for stack XYZ, version 1.0.1 was loaded
> Returning XYZ101StackAdvisor implementation
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 1:19:32.519s
> [INFO] Finished at: Mon Apr 25 11:02:50 EDT 2016
> [INFO] Final Memory: 38M/720M
> [INFO] ————————————————————————————————————
> "
> 
> 
> Thanks,
> 
> Robert Nettleton
> 
>


Re: Review Request 46639: Add counter limits to LogSearch integration error logs that occur during failed connection attempts

Posted by Robert Nettleton <rn...@hortonworks.com>.

> On April 25, 2016, 4 p.m., Oliver Szabo wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/logging/Utils.java, line 29
> > <https://reviews.apache.org/r/46639/diff/1/?file=1359667#file1359667line29>
> >
> >     Utils is too generic for me, can this be more specified ? (like SomethingUtils.java, i know its in a specified package, but maybe it would be easier to find with a less generic name)
> 
> Robert Nettleton wrote:
>     I'm not sure I agree that the name is too generic, since as you mentioned it is scoped in the intended package.
>     
>     As Sumit mentioned above, this might eventually be a useful utility for more than just this case, but I believe that the code might need some refactoring in order to make this generic enough to be useful across Ambari logging.
>     
>     Since we'll probably do that in a future patch at some point, I'm less inclined to think that we should rename this now, since this functionality might be moved to a different, more common, package anyway in a future change.
>     
>     Please let me know if you agree. 
>     
>     Thanks.
> 
> Oliver Szabo wrote:
>     im okay with that now, so i will drop the issue, but if it will go into a more generic package (like org.apache.ambari.server.utils.* -> we have some Utils class there), then it would be better with a more specified name

I agree.  Thanks for the comments.


- Robert


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


On April 25, 2016, 3:28 p.m., Robert Nettleton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46639/
> -----------------------------------------------------------
> 
> (Updated April 25, 2016, 3:28 p.m.)
> 
> 
> Review request for Ambari, Oliver Szabo and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-16093
>     https://issues.apache.org/jira/browse/AMBARI-16093
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> This patch resolves AMBARI-16093.
> 
> Hundreds of repeated error logs were occuring in instances when the LogSearch server was unexpectedly shutdown, and Ambari Server was not yet aware of the change in service state. 
> 
> The LoggingSearchPropertyProvider already waits until the LOGSEARCH_SERVER component is in the started state, but if that component shuts down unexpectedly, there will be a small time window during which the provider will attempt to connect to the LogSearch server, which will then fail and generate error logs.
> 
> In order to resolve this logging issue, this patch implements the following:
> 
> 1. Implements a new utility class to handle checks on a specified counter, which can be used to limit the number of times a given error log is included in ambari-server.log. 
> 2. Modifies the LoggingSearchPropertyProvider and LoggingRequestHelperImpl classes to use this new utility class in cases where this problem of repeated logging may occur.  
> 3. Implements a new unit test case for the new utility class, to verify that a given log is only passed to the logger in certain circumstances, based on the value of the specified counter.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LoggingRequestHelperImpl.java a5cd369 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LoggingSearchPropertyProvider.java e2da4c8 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/logging/Utils.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/logging/UtilsTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46639/diff/
> 
> 
> Testing
> -------
> 
> 1. Deployed a 1-VM cluster without LogSearch enabled, verified that the cluster deployed successfully, and that no extraneous error logging occurring in ambari-server.log or ambari-server.out.
> 2. Deployed a 1-VM cluster with LogSearch enabled, verified that the cluster deployed successfully, verified that the LogSearch Integration APIs are functioning as expected.  As part of this test, I also killed the LogSearch Server with "kill -9", accessed the various portions of the UI that rely on the HostComponent REST API, and verified that no extraneous logging was present in ambari-server.log or ambari-server.out.
> 3. Ran the ambari-server "mvn clean test" suite with my patch applied.  Test Results:
> 
> "
> Results :
> 
> Tests run: 4273, Failures: 0, Errors: 0, Skipped: 32
> "
> 
> "
> INFO: AMBARI_SERVER_LIB is not set, using default /usr/lib/ambari-server
> INFO: Return code from stack upgrade command, retcode = 0
> StackAdvisor implementation for stack HDP1, version 2.0.6 was not found
> Returning DefaultStackAdvisor implementation
> StackAdvisor implementation for stack XYZ, version 1.0.0 was loaded
> StackAdvisor implementation for stack XYZ, version 1.0.1 was loaded
> Returning XYZ101StackAdvisor implementation
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 1:19:32.519s
> [INFO] Finished at: Mon Apr 25 11:02:50 EDT 2016
> [INFO] Final Memory: 38M/720M
> [INFO] ————————————————————————————————————
> "
> 
> 
> Thanks,
> 
> Robert Nettleton
> 
>


Re: Review Request 46639: Add counter limits to LogSearch integration error logs that occur during failed connection attempts

Posted by Oliver Szabo <os...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46639/#review130435
-----------------------------------------------------------




ambari-server/src/main/java/org/apache/ambari/server/controller/logging/Utils.java (line 29)
<https://reviews.apache.org/r/46639/#comment194177>

    Utils is too generic for me, can this be more specified ? (like SomethingUtils.java, i know its in a specified package, but maybe it would be easier to find with a less generic name)


- Oliver Szabo


On April 25, 2016, 3:28 p.m., Robert Nettleton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46639/
> -----------------------------------------------------------
> 
> (Updated April 25, 2016, 3:28 p.m.)
> 
> 
> Review request for Ambari, Oliver Szabo and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-16093
>     https://issues.apache.org/jira/browse/AMBARI-16093
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> This patch resolves AMBARI-16093.
> 
> Hundreds of repeated error logs were occuring in instances when the LogSearch server was unexpectedly shutdown, and Ambari Server was not yet aware of the change in service state. 
> 
> The LoggingSearchPropertyProvider already waits until the LOGSEARCH_SERVER component is in the started state, but if that component shuts down unexpectedly, there will be a small time window during which the provider will attempt to connect to the LogSearch server, which will then fail and generate error logs.
> 
> In order to resolve this logging issue, this patch implements the following:
> 
> 1. Implements a new utility class to handle checks on a specified counter, which can be used to limit the number of times a given error log is included in ambari-server.log. 
> 2. Modifies the LoggingSearchPropertyProvider and LoggingRequestHelperImpl classes to use this new utility class in cases where this problem of repeated logging may occur.  
> 3. Implements a new unit test case for the new utility class, to verify that a given log is only passed to the logger in certain circumstances, based on the value of the specified counter.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LoggingRequestHelperImpl.java a5cd369 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LoggingSearchPropertyProvider.java e2da4c8 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/logging/Utils.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/logging/UtilsTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46639/diff/
> 
> 
> Testing
> -------
> 
> 1. Deployed a 1-VM cluster without LogSearch enabled, verified that the cluster deployed successfully, and that no extraneous error logging occurring in ambari-server.log or ambari-server.out.
> 2. Deployed a 1-VM cluster with LogSearch enabled, verified that the cluster deployed successfully, verified that the LogSearch Integration APIs are functioning as expected.  As part of this test, I also killed the LogSearch Server with "kill -9", accessed the various portions of the UI that rely on the HostComponent REST API, and verified that no extraneous logging was present in ambari-server.log or ambari-server.out.
> 3. Ran the ambari-server "mvn clean test" suite with my patch applied.  Test Results:
> 
> "
> Results :
> 
> Tests run: 4273, Failures: 0, Errors: 0, Skipped: 32
> "
> 
> "
> INFO: AMBARI_SERVER_LIB is not set, using default /usr/lib/ambari-server
> INFO: Return code from stack upgrade command, retcode = 0
> StackAdvisor implementation for stack HDP1, version 2.0.6 was not found
> Returning DefaultStackAdvisor implementation
> StackAdvisor implementation for stack XYZ, version 1.0.0 was loaded
> StackAdvisor implementation for stack XYZ, version 1.0.1 was loaded
> Returning XYZ101StackAdvisor implementation
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 1:19:32.519s
> [INFO] Finished at: Mon Apr 25 11:02:50 EDT 2016
> [INFO] Final Memory: 38M/720M
> [INFO] ————————————————————————————————————
> "
> 
> 
> Thanks,
> 
> Robert Nettleton
> 
>


Re: Review Request 46639: Add counter limits to LogSearch integration error logs that occur during failed connection attempts

Posted by Sumit Mohanty <sm...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46639/#review130432
-----------------------------------------------------------


Ship it!




Ship It!

- Sumit Mohanty


On April 25, 2016, 3:28 p.m., Robert Nettleton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46639/
> -----------------------------------------------------------
> 
> (Updated April 25, 2016, 3:28 p.m.)
> 
> 
> Review request for Ambari, Oliver Szabo and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-16093
>     https://issues.apache.org/jira/browse/AMBARI-16093
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> This patch resolves AMBARI-16093.
> 
> Hundreds of repeated error logs were occuring in instances when the LogSearch server was unexpectedly shutdown, and Ambari Server was not yet aware of the change in service state. 
> 
> The LoggingSearchPropertyProvider already waits until the LOGSEARCH_SERVER component is in the started state, but if that component shuts down unexpectedly, there will be a small time window during which the provider will attempt to connect to the LogSearch server, which will then fail and generate error logs.
> 
> In order to resolve this logging issue, this patch implements the following:
> 
> 1. Implements a new utility class to handle checks on a specified counter, which can be used to limit the number of times a given error log is included in ambari-server.log. 
> 2. Modifies the LoggingSearchPropertyProvider and LoggingRequestHelperImpl classes to use this new utility class in cases where this problem of repeated logging may occur.  
> 3. Implements a new unit test case for the new utility class, to verify that a given log is only passed to the logger in certain circumstances, based on the value of the specified counter.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LoggingRequestHelperImpl.java a5cd369 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LoggingSearchPropertyProvider.java e2da4c8 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/logging/Utils.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/logging/UtilsTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46639/diff/
> 
> 
> Testing
> -------
> 
> 1. Deployed a 1-VM cluster without LogSearch enabled, verified that the cluster deployed successfully, and that no extraneous error logging occurring in ambari-server.log or ambari-server.out.
> 2. Deployed a 1-VM cluster with LogSearch enabled, verified that the cluster deployed successfully, verified that the LogSearch Integration APIs are functioning as expected.  As part of this test, I also killed the LogSearch Server with "kill -9", accessed the various portions of the UI that rely on the HostComponent REST API, and verified that no extraneous logging was present in ambari-server.log or ambari-server.out.
> 3. Ran the ambari-server "mvn clean test" suite with my patch applied.  Test Results:
> 
> "
> Results :
> 
> Tests run: 4273, Failures: 0, Errors: 0, Skipped: 32
> "
> 
> "
> INFO: AMBARI_SERVER_LIB is not set, using default /usr/lib/ambari-server
> INFO: Return code from stack upgrade command, retcode = 0
> StackAdvisor implementation for stack HDP1, version 2.0.6 was not found
> Returning DefaultStackAdvisor implementation
> StackAdvisor implementation for stack XYZ, version 1.0.0 was loaded
> StackAdvisor implementation for stack XYZ, version 1.0.1 was loaded
> Returning XYZ101StackAdvisor implementation
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 1:19:32.519s
> [INFO] Finished at: Mon Apr 25 11:02:50 EDT 2016
> [INFO] Final Memory: 38M/720M
> [INFO] ————————————————————————————————————
> "
> 
> 
> Thanks,
> 
> Robert Nettleton
> 
>


Re: Review Request 46639: Add counter limits to LogSearch integration error logs that occur during failed connection attempts

Posted by Robert Nettleton <rn...@hortonworks.com>.

> On April 25, 2016, 3:34 p.m., Sumit Mohanty wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/logging/Utils.java, line 29
> > <https://reviews.apache.org/r/46639/diff/1/?file=1359667#file1359667line29>
> >
> >     Is it something that AMS provider loggers can use or should use?

Thanks for the review!

Yes, this utility probably could be used by the AMS property provider logic as well.  The AMS Property Provider code already uses this static counter approach, which I thought would be useful for this current code as well.  

We could consider moving this utility into a more common package for use by other components, maybe in a future patch.


- Robert


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


On April 25, 2016, 3:28 p.m., Robert Nettleton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46639/
> -----------------------------------------------------------
> 
> (Updated April 25, 2016, 3:28 p.m.)
> 
> 
> Review request for Ambari, Oliver Szabo and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-16093
>     https://issues.apache.org/jira/browse/AMBARI-16093
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> This patch resolves AMBARI-16093.
> 
> Hundreds of repeated error logs were occuring in instances when the LogSearch server was unexpectedly shutdown, and Ambari Server was not yet aware of the change in service state. 
> 
> The LoggingSearchPropertyProvider already waits until the LOGSEARCH_SERVER component is in the started state, but if that component shuts down unexpectedly, there will be a small time window during which the provider will attempt to connect to the LogSearch server, which will then fail and generate error logs.
> 
> In order to resolve this logging issue, this patch implements the following:
> 
> 1. Implements a new utility class to handle checks on a specified counter, which can be used to limit the number of times a given error log is included in ambari-server.log. 
> 2. Modifies the LoggingSearchPropertyProvider and LoggingRequestHelperImpl classes to use this new utility class in cases where this problem of repeated logging may occur.  
> 3. Implements a new unit test case for the new utility class, to verify that a given log is only passed to the logger in certain circumstances, based on the value of the specified counter.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LoggingRequestHelperImpl.java a5cd369 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LoggingSearchPropertyProvider.java e2da4c8 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/logging/Utils.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/logging/UtilsTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46639/diff/
> 
> 
> Testing
> -------
> 
> 1. Deployed a 1-VM cluster without LogSearch enabled, verified that the cluster deployed successfully, and that no extraneous error logging occurring in ambari-server.log or ambari-server.out.
> 2. Deployed a 1-VM cluster with LogSearch enabled, verified that the cluster deployed successfully, verified that the LogSearch Integration APIs are functioning as expected.  As part of this test, I also killed the LogSearch Server with "kill -9", accessed the various portions of the UI that rely on the HostComponent REST API, and verified that no extraneous logging was present in ambari-server.log or ambari-server.out.
> 3. Ran the ambari-server "mvn clean test" suite with my patch applied.  Test Results:
> 
> "
> Results :
> 
> Tests run: 4273, Failures: 0, Errors: 0, Skipped: 32
> "
> 
> "
> INFO: AMBARI_SERVER_LIB is not set, using default /usr/lib/ambari-server
> INFO: Return code from stack upgrade command, retcode = 0
> StackAdvisor implementation for stack HDP1, version 2.0.6 was not found
> Returning DefaultStackAdvisor implementation
> StackAdvisor implementation for stack XYZ, version 1.0.0 was loaded
> StackAdvisor implementation for stack XYZ, version 1.0.1 was loaded
> Returning XYZ101StackAdvisor implementation
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 1:19:32.519s
> [INFO] Finished at: Mon Apr 25 11:02:50 EDT 2016
> [INFO] Final Memory: 38M/720M
> [INFO] ————————————————————————————————————
> "
> 
> 
> Thanks,
> 
> Robert Nettleton
> 
>


Re: Review Request 46639: Add counter limits to LogSearch integration error logs that occur during failed connection attempts

Posted by Sumit Mohanty <sm...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46639/#review130431
-----------------------------------------------------------




ambari-server/src/main/java/org/apache/ambari/server/controller/logging/Utils.java (line 29)
<https://reviews.apache.org/r/46639/#comment194175>

    Is it something that AMS provider loggers can use or should use?


- Sumit Mohanty


On April 25, 2016, 3:28 p.m., Robert Nettleton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46639/
> -----------------------------------------------------------
> 
> (Updated April 25, 2016, 3:28 p.m.)
> 
> 
> Review request for Ambari, Oliver Szabo and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-16093
>     https://issues.apache.org/jira/browse/AMBARI-16093
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> This patch resolves AMBARI-16093.
> 
> Hundreds of repeated error logs were occuring in instances when the LogSearch server was unexpectedly shutdown, and Ambari Server was not yet aware of the change in service state. 
> 
> The LoggingSearchPropertyProvider already waits until the LOGSEARCH_SERVER component is in the started state, but if that component shuts down unexpectedly, there will be a small time window during which the provider will attempt to connect to the LogSearch server, which will then fail and generate error logs.
> 
> In order to resolve this logging issue, this patch implements the following:
> 
> 1. Implements a new utility class to handle checks on a specified counter, which can be used to limit the number of times a given error log is included in ambari-server.log. 
> 2. Modifies the LoggingSearchPropertyProvider and LoggingRequestHelperImpl classes to use this new utility class in cases where this problem of repeated logging may occur.  
> 3. Implements a new unit test case for the new utility class, to verify that a given log is only passed to the logger in certain circumstances, based on the value of the specified counter.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LoggingRequestHelperImpl.java a5cd369 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LoggingSearchPropertyProvider.java e2da4c8 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/logging/Utils.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/logging/UtilsTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46639/diff/
> 
> 
> Testing
> -------
> 
> 1. Deployed a 1-VM cluster without LogSearch enabled, verified that the cluster deployed successfully, and that no extraneous error logging occurring in ambari-server.log or ambari-server.out.
> 2. Deployed a 1-VM cluster with LogSearch enabled, verified that the cluster deployed successfully, verified that the LogSearch Integration APIs are functioning as expected.  As part of this test, I also killed the LogSearch Server with "kill -9", accessed the various portions of the UI that rely on the HostComponent REST API, and verified that no extraneous logging was present in ambari-server.log or ambari-server.out.
> 3. Ran the ambari-server "mvn clean test" suite with my patch applied.  Test Results:
> 
> "
> Results :
> 
> Tests run: 4273, Failures: 0, Errors: 0, Skipped: 32
> "
> 
> "
> INFO: AMBARI_SERVER_LIB is not set, using default /usr/lib/ambari-server
> INFO: Return code from stack upgrade command, retcode = 0
> StackAdvisor implementation for stack HDP1, version 2.0.6 was not found
> Returning DefaultStackAdvisor implementation
> StackAdvisor implementation for stack XYZ, version 1.0.0 was loaded
> StackAdvisor implementation for stack XYZ, version 1.0.1 was loaded
> Returning XYZ101StackAdvisor implementation
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 1:19:32.519s
> [INFO] Finished at: Mon Apr 25 11:02:50 EDT 2016
> [INFO] Final Memory: 38M/720M
> [INFO] ————————————————————————————————————
> "
> 
> 
> Thanks,
> 
> Robert Nettleton
> 
>


Re: Review Request 46639: Add counter limits to LogSearch integration error logs that occur during failed connection attempts

Posted by Oliver Szabo <os...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46639/#review130441
-----------------------------------------------------------


Ship it!




Ship It!

- Oliver Szabo


On April 25, 2016, 3:28 p.m., Robert Nettleton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46639/
> -----------------------------------------------------------
> 
> (Updated April 25, 2016, 3:28 p.m.)
> 
> 
> Review request for Ambari, Oliver Szabo and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-16093
>     https://issues.apache.org/jira/browse/AMBARI-16093
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> This patch resolves AMBARI-16093.
> 
> Hundreds of repeated error logs were occuring in instances when the LogSearch server was unexpectedly shutdown, and Ambari Server was not yet aware of the change in service state. 
> 
> The LoggingSearchPropertyProvider already waits until the LOGSEARCH_SERVER component is in the started state, but if that component shuts down unexpectedly, there will be a small time window during which the provider will attempt to connect to the LogSearch server, which will then fail and generate error logs.
> 
> In order to resolve this logging issue, this patch implements the following:
> 
> 1. Implements a new utility class to handle checks on a specified counter, which can be used to limit the number of times a given error log is included in ambari-server.log. 
> 2. Modifies the LoggingSearchPropertyProvider and LoggingRequestHelperImpl classes to use this new utility class in cases where this problem of repeated logging may occur.  
> 3. Implements a new unit test case for the new utility class, to verify that a given log is only passed to the logger in certain circumstances, based on the value of the specified counter.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LoggingRequestHelperImpl.java a5cd369 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/logging/LoggingSearchPropertyProvider.java e2da4c8 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/logging/Utils.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/logging/UtilsTest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46639/diff/
> 
> 
> Testing
> -------
> 
> 1. Deployed a 1-VM cluster without LogSearch enabled, verified that the cluster deployed successfully, and that no extraneous error logging occurring in ambari-server.log or ambari-server.out.
> 2. Deployed a 1-VM cluster with LogSearch enabled, verified that the cluster deployed successfully, verified that the LogSearch Integration APIs are functioning as expected.  As part of this test, I also killed the LogSearch Server with "kill -9", accessed the various portions of the UI that rely on the HostComponent REST API, and verified that no extraneous logging was present in ambari-server.log or ambari-server.out.
> 3. Ran the ambari-server "mvn clean test" suite with my patch applied.  Test Results:
> 
> "
> Results :
> 
> Tests run: 4273, Failures: 0, Errors: 0, Skipped: 32
> "
> 
> "
> INFO: AMBARI_SERVER_LIB is not set, using default /usr/lib/ambari-server
> INFO: Return code from stack upgrade command, retcode = 0
> StackAdvisor implementation for stack HDP1, version 2.0.6 was not found
> Returning DefaultStackAdvisor implementation
> StackAdvisor implementation for stack XYZ, version 1.0.0 was loaded
> StackAdvisor implementation for stack XYZ, version 1.0.1 was loaded
> Returning XYZ101StackAdvisor implementation
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 1:19:32.519s
> [INFO] Finished at: Mon Apr 25 11:02:50 EDT 2016
> [INFO] Final Memory: 38M/720M
> [INFO] ————————————————————————————————————
> "
> 
> 
> Thanks,
> 
> Robert Nettleton
> 
>