You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by nakomis <gi...@git.apache.org> on 2015/06/11 13:40:05 UTC
[GitHub] incubator-brooklyn pull request: Reduces excessively chatty loggin...
GitHub user nakomis opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/686
Reduces excessively chatty logging on persistence checkpoints
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/nakomis/incubator-brooklyn checkpoint-log-level
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/686.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #686
----
commit 6281c09ea8c9504882518aaf5e599ad23d12d6f9
Author: Martin Harris <gi...@nakomis.com>
Date: 2015-06-11T11:39:28Z
Reduces excessively chatty logging on persistence checkpoints
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Reduces excessively chatty loggin...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/686#discussion_r35915683
--- Diff: core/src/main/java/brooklyn/entity/rebind/PeriodicDeltaChangeListener.java ---
@@ -494,5 +496,8 @@ public synchronized void onChanged(BrooklynObject instance) {
public PersistenceExceptionHandler getExceptionHandler() {
return exceptionHandler;
}
-
+
+ protected boolean shouldLogCheckpoint() {
+ return (checkpointLogCount.get() < 5) || (checkpointLogCount.get() % 1000 == 0);
--- End diff --
Why the magic number 5?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Reduces excessively chatty loggin...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/686#discussion_r35914657
--- Diff: core/src/main/java/brooklyn/entity/rebind/PeriodicDeltaChangeListener.java ---
@@ -494,5 +496,8 @@ public synchronized void onChanged(BrooklynObject instance) {
public PersistenceExceptionHandler getExceptionHandler() {
return exceptionHandler;
}
-
+
+ protected boolean shouldLogCheckpoint() {
+ return (checkpointLogCount.get() < 5) || (checkpointLogCount.get() % 1000 == 0);
--- End diff --
Did you intend to use incrementAndGet? Nothing increments the counters at the moment so they'll always be 0.
Also suggest getting the value of the integer once so we don't pay synchronisation penalty twice.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Reduces excessively chatty loggin...
Posted by nakomis <gi...@git.apache.org>.
Github user nakomis commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/686#issuecomment-125153305
PR comments addressed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Reduces excessively chatty loggin...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/686#issuecomment-111102830
The test failure is almost certainly unrelated but something we should fix -- for posterity it is:
```
java.net.BindException: Address already in use
at java.net.PlainSocketImpl.socketBind(Native Method)
at java.net.AbstractPlainSocketImpl.bind(AbstractPlainSocketImpl.java:376)
at java.net.ServerSocket.bind(ServerSocket.java:376)
at java.net.ServerSocket.<init>(ServerSocket.java:237)
at java.net.ServerSocket.<init>(ServerSocket.java:128)
at brooklyn.policy.ha.ConnectionFailureDetectorTest.startServerSocket(ConnectionFailureDetectorTest.java:103)
at brooklyn.policy.ha.ConnectionFailureDetectorTest.setUp(ConnectionFailureDetectorTest.java:92)
```
/cc @aledsage @neykov
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Reduces excessively chatty loggin...
Posted by nakomis <gi...@git.apache.org>.
Github user nakomis commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/686#issuecomment-147390814
Failed test passes locally, and appears to be unrelated to this PR
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Reduces excessively chatty loggin...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/686#issuecomment-124338505
I agree we need to massively reduce the logging. It fills up one of our customer's rolling log files way too much.
@nakomis can you please take another look at @ahgittin 's suggestion of how to reduce logging (and let us know if you agree with that)?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Reduces excessively chatty loggin...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/686#discussion_r35915929
--- Diff: core/src/main/java/brooklyn/entity/rebind/PeriodicDeltaChangeListener.java ---
@@ -78,6 +78,8 @@
private static final Logger LOG = LoggerFactory.getLogger(PeriodicDeltaChangeListener.class);
+ protected final AtomicInteger checkpointLogCount = new AtomicInteger();
--- End diff --
I think you should use an `AtomicLong` instead. If `shouldLogCheckpoint` were called once per millisecond the integer would overflow in about three and a half weeks: `(2^31 - 1) / (1000 * 60 * 60 * 24 * 7) = 3.55`. So `shouldLogCheckpoint` will always return true until `checkpointLogCount.get() < 5` is false. With a Long it would take 292,277 millenia to overflow.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Reduces excessively chatty loggin...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/686#issuecomment-111102436
This is extremely useful information if there is latency in checkpointing which has been a problem so I would be loathe to remove it from the default set.
In terms of impact it is a couple of lines every second which is hugely reduced from where it was. But if that is still overly chatty the fix I would recommend would be to log debug for the first few writes after any HA change and every 100th checkpoint and if latency is ever more than some threshhold (1s).
We currently do the first of these already for hot standby mode so you should be able to tap into that counter.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Reduces excessively chatty loggin...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/686#issuecomment-111110629
See `RebindIteration.shouldLogRebinding` -- the count here comes from `RebindManagerImpl` but it is only maintained for RO rebind so you probably want an independent count within the `Periodic` class.
It is reset within the `{start,stop}{Persistence,ReadOnly}` methods so you could add a call in each of these to e.g. `periodic.resetCount()`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Reduces excessively chatty loggin...
Posted by nakomis <gi...@git.apache.org>.
Github user nakomis commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/686#issuecomment-111107326
Hi @ahgittin, where can I find the implementation of the 'first few writes' in hot standby mode? Also, how can I tell if HA has changed?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Reduces excessively chatty loggin...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/686
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Reduces excessively chatty loggin...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/686#discussion_r35916018
--- Diff: core/src/main/java/brooklyn/entity/rebind/PeriodicDeltaChangeListener.java ---
@@ -383,7 +385,7 @@ protected void persistNowInternal(boolean alreadyHasMutex) {
deltaCollector = new DeltaCollector();
}
- if (LOG.isDebugEnabled()) LOG.debug("Checkpointing delta of memento: "
+ if (shouldLogCheckpoint() && LOG.isDebugEnabled()) LOG.debug("Checkpointing delta of memento: "
--- End diff --
Swap the order of the if conditions for cheaper short-circuiting when `isDebugEnabled` is false.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Reduces excessively chatty loggin...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/686#issuecomment-114446722
@nakomis has a point about excessively noisy logs - ran a simple test with brooklyn-ambari and there are so many "Checkpointing delta of memento" messages that it took one minute for my debug log to roll over. It was more like once per millisecond than once per second.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Reduces excessively chatty loggin...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/686#issuecomment-148058323
@nakomis Thanks. Will merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Reduces excessively chatty loggin...
Posted by nakomis <gi...@git.apache.org>.
Github user nakomis commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/686#issuecomment-127547524
Comments addressed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---