You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by nkalmar <gi...@git.apache.org> on 2018/04/30 08:17:00 UTC

[GitHub] zookeeper pull request #510: ZOOKEEPER-3019 add metric for slow fsyncs count

GitHub user nkalmar opened a pull request:

    https://github.com/apache/zookeeper/pull/510

    ZOOKEEPER-3019 add metric for slow fsyncs count

    Backported fsync metric count from master.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/nkalmar/zookeeper branch-3.4

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zookeeper/pull/510.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 #510
    
----
commit 1485a05b2a4188b3a9491bbeb79926e2ee7124e0
Author: Norbert Kalmar <nk...@...>
Date:   2018-04-30T08:15:30Z

    ZOOKEEPER-3019 add metric for slow fsyncs count

----


---

[GitHub] zookeeper pull request #510: ZOOKEEPER-3019 add metric for slow fsyncs count

Posted by nkalmar <gi...@git.apache.org>.
Github user nkalmar commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/510#discussion_r192668700
  
    --- Diff: src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java ---
    @@ -320,6 +332,11 @@ public synchronized void commit() throws IOException {
                     long syncElapsedMS =
                         TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startSyncNS);
                     if (syncElapsedMS > fsyncWarningThresholdMS) {
    +                    if(serverStats != null) {
    +                        serverStats.incrementFsyncThresholdExceedCount();
    +                    } else {
    +                        LOG.warn("fsyncWarningThresholdMS exceeded, but serverStats not added in FileTxnLog!");
    --- End diff --
    
    Okay, sounds fair. I will remove it. 
    Thanks for pointing this out!


---

[GitHub] zookeeper issue #510: ZOOKEEPER-3019 add metric for slow fsyncs count

Posted by anmolnar <gi...@git.apache.org>.
Github user anmolnar commented on the issue:

    https://github.com/apache/zookeeper/pull/510
  
    Merged to branch-3.4. Please close the PR @nkalmar 
    Thanks!


---

[GitHub] zookeeper pull request #510: ZOOKEEPER-3019 add metric for slow fsyncs count

Posted by lvfangmin <gi...@git.apache.org>.
Github user lvfangmin commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/510#discussion_r192631195
  
    --- Diff: src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java ---
    @@ -320,6 +332,11 @@ public synchronized void commit() throws IOException {
                     long syncElapsedMS =
                         TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startSyncNS);
                     if (syncElapsedMS > fsyncWarningThresholdMS) {
    +                    if(serverStats != null) {
    +                        serverStats.incrementFsyncThresholdExceedCount();
    +                    } else {
    +                        LOG.warn("fsyncWarningThresholdMS exceeded, but serverStats not added in FileTxnLog!");
    --- End diff --
    
    This log seems redundant to me, we can tell the fsync threshold exceeded from the log at line 340, and serverStats is set, we know there will be stats in serverStats, otherwise we know it's not set.


---

[GitHub] zookeeper pull request #510: ZOOKEEPER-3019 add metric for slow fsyncs count

Posted by nkalmar <gi...@git.apache.org>.
Github user nkalmar closed the pull request at:

    https://github.com/apache/zookeeper/pull/510


---