You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zookeeper.apache.org by dd...@apache.org on 2021/06/30 07:10:31 UTC

[zookeeper] branch branch-3.7 updated (7048d3a -> fb86553)

This is an automated email from the ASF dual-hosted git repository.

ddiederen pushed a change to branch branch-3.7
in repository https://gitbox.apache.org/repos/asf/zookeeper.git.


    from 7048d3a  ZOOKEEPER-4309: QuorumCnxManager's ListenerHandler thread leak
     new 203de65  ZOOKEEPER-4284: Add metrics for observer sync time
     new fb86553  ZOOKEEPER-4318: Only report the follower sync time metrics if sync is completed

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../java/org/apache/zookeeper/server/ServerMetrics.java |  2 ++
 .../org/apache/zookeeper/server/quorum/Follower.java    | 17 +++++++----------
 .../org/apache/zookeeper/server/quorum/Observer.java    |  4 ++++
 .../org/apache/zookeeper/test/ObserverMasterTest.java   | 14 ++++++++++++++
 4 files changed, 27 insertions(+), 10 deletions(-)

[zookeeper] 02/02: ZOOKEEPER-4318: Only report the follower sync time metrics if sync is completed

Posted by dd...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

ddiederen pushed a commit to branch branch-3.7
in repository https://gitbox.apache.org/repos/asf/zookeeper.git

commit fb86553cd45ec43b58cdb2dd5d56e4f59854ab29
Author: liwang <li...@apple.com>
AuthorDate: Wed Jun 30 06:59:22 2021 +0000

    ZOOKEEPER-4318: Only report the follower sync time metrics if sync is completed
    
    Motivation
    
    - Currently we report the follower sync time regardless whether sync is completed.  This will give us some noisy data such as 0 sync time in cases where sync immediately failed.
    
    Change
    
    - Only report the sync time if the sync is completed.
    
    Author: liwang <li...@apple.com>
    
    Reviewers: Enrico Olivelli <eo...@apache.org>, Michael Han <ha...@apache.org>, Damien Diederen <dd...@apache.org>
    
    Closes #1712 from li4wang/ZOOKEEPER-4318
    
    (cherry picked from commit 70f70d821c2c5225edeb54a8af0bd1911a51fc89)
    Signed-off-by: Damien Diederen <dd...@apache.org>
---
 .../org/apache/zookeeper/server/quorum/Follower.java    | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Follower.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Follower.java
index 971710c..0eff9d2 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Follower.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Follower.java
@@ -103,16 +103,13 @@ public class Follower extends Learner {
                     throw new IOException("Error: Epoch of leader is lower");
                 }
                 long startTime = Time.currentElapsedTime();
-                try {
-                    self.setLeaderAddressAndId(leaderServer.addr, leaderServer.getId());
-                    self.setZabState(QuorumPeer.ZabState.SYNCHRONIZATION);
-                    syncWithLeader(newEpochZxid);
-                    self.setZabState(QuorumPeer.ZabState.BROADCAST);
-                    completedSync = true;
-                } finally {
-                    long syncTime = Time.currentElapsedTime() - startTime;
-                    ServerMetrics.getMetrics().FOLLOWER_SYNC_TIME.add(syncTime);
-                }
+                self.setLeaderAddressAndId(leaderServer.addr, leaderServer.getId());
+                self.setZabState(QuorumPeer.ZabState.SYNCHRONIZATION);
+                syncWithLeader(newEpochZxid);
+                self.setZabState(QuorumPeer.ZabState.BROADCAST);
+                completedSync = true;
+                long syncTime = Time.currentElapsedTime() - startTime;
+                ServerMetrics.getMetrics().FOLLOWER_SYNC_TIME.add(syncTime);
                 if (self.getObserverMasterPort() > 0) {
                     LOG.info("Starting ObserverMaster");
 

[zookeeper] 01/02: ZOOKEEPER-4284: Add metrics for observer sync time

Posted by dd...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

ddiederen pushed a commit to branch branch-3.7
in repository https://gitbox.apache.org/repos/asf/zookeeper.git

commit 203de65823bb12420fd0a030705f632e7ae5d814
Author: liwang <li...@apple.com>
AuthorDate: Wed Jun 30 06:53:49 2021 +0000

    ZOOKEEPER-4284: Add metrics for observer sync time
    
    Motivation
    
    - With enabling the feature of followers hosting observers, we would need a metric to measure the observer sync time just like what we have for the follower sync time.
    
    Changes
    
    - Added the "observer_sync_time" metrics
    - Added unit test on the metrics
    
    Author: liwang <li...@apple.com>
    
    Reviewers: Enrico Olivelli <eo...@apache.org>, Michael Han <ha...@apache.org>, Damien Diederen <dd...@apache.org>
    
    Closes #1691 from li4wang/ZOOKEEPER-4284
    
    (cherry picked from commit 2d3065606a444c0b711c1809ce296db2ba56cb0c)
    Signed-off-by: Damien Diederen <dd...@apache.org>
---
 .../java/org/apache/zookeeper/server/ServerMetrics.java    |  2 ++
 .../java/org/apache/zookeeper/server/quorum/Observer.java  |  4 ++++
 .../java/org/apache/zookeeper/test/ObserverMasterTest.java | 14 ++++++++++++++
 3 files changed, 20 insertions(+)

diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerMetrics.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerMetrics.java
index 99e9206..8ef5235 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerMetrics.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ServerMetrics.java
@@ -75,6 +75,7 @@ public final class ServerMetrics {
         UPDATE_LATENCY = metricsContext.getSummary("updatelatency", DetailLevel.ADVANCED);
         PROPAGATION_LATENCY = metricsContext.getSummary("propagation_latency", DetailLevel.ADVANCED);
         FOLLOWER_SYNC_TIME = metricsContext.getSummary("follower_sync_time", DetailLevel.BASIC);
+        OBSERVER_SYNC_TIME = metricsContext.getSummary("observer_sync_time", DetailLevel.BASIC);
         ELECTION_TIME = metricsContext.getSummary("election_time", DetailLevel.BASIC);
         LOOKING_COUNT = metricsContext.getCounter("looking_count");
         DIFF_COUNT = metricsContext.getCounter("diff_count");
@@ -296,6 +297,7 @@ public final class ServerMetrics {
     public final Summary PROPAGATION_LATENCY;
 
     public final Summary FOLLOWER_SYNC_TIME;
+    public final Summary OBSERVER_SYNC_TIME;
 
     public final Summary ELECTION_TIME;
 
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Observer.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Observer.java
index 55e273b..d3aa41b 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Observer.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Observer.java
@@ -23,6 +23,7 @@ import java.nio.ByteBuffer;
 import java.util.concurrent.ThreadLocalRandom;
 import java.util.concurrent.atomic.AtomicReference;
 import org.apache.jute.Record;
+import org.apache.zookeeper.common.Time;
 import org.apache.zookeeper.server.ObserverBean;
 import org.apache.zookeeper.server.Request;
 import org.apache.zookeeper.server.ServerMetrics;
@@ -114,11 +115,14 @@ public class Observer extends Learner {
                     throw new Exception("learned about role change");
                 }
 
+                final long startTime = Time.currentElapsedTime();
                 self.setLeaderAddressAndId(master.addr, master.getId());
                 self.setZabState(QuorumPeer.ZabState.SYNCHRONIZATION);
                 syncWithLeader(newLeaderZxid);
                 self.setZabState(QuorumPeer.ZabState.BROADCAST);
                 completedSync = true;
+                final long syncTime = Time.currentElapsedTime() - startTime;
+                ServerMetrics.getMetrics().OBSERVER_SYNC_TIME.add(syncTime);
                 QuorumPacket qp = new QuorumPacket();
                 while (this.isRunning() && nextLearnerMaster.get() == null) {
                     readPacket(qp);
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ObserverMasterTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ObserverMasterTest.java
index 2d2bea7..4ba33c7 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ObserverMasterTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ObserverMasterTest.java
@@ -55,6 +55,7 @@ import org.apache.zookeeper.ZooKeeper.States;
 import org.apache.zookeeper.admin.ZooKeeperAdmin;
 import org.apache.zookeeper.jmx.MBeanRegistry;
 import org.apache.zookeeper.jmx.ZKMBeanInfo;
+import org.apache.zookeeper.metrics.MetricsUtils;
 import org.apache.zookeeper.server.admin.Commands;
 import org.apache.zookeeper.server.quorum.QuorumPeerConfig;
 import org.apache.zookeeper.server.util.PortForwarder;
@@ -85,6 +86,8 @@ public class ObserverMasterTest extends ObserverMasterTestBase {
         assertTrue(ClientBase.waitForServerUp("127.0.0.1:" + CLIENT_PORT_OBS, CONNECTION_TIMEOUT),
                 "waiting for server 3 being up");
 
+        validateObserverSyncTimeMetrics();
+
         if (testObserverMaster) {
             int masterPort = q3.getQuorumPeer().observer.getSocket().getPort();
             LOG.info("port {} {}", masterPort, OM_PORT);
@@ -522,4 +525,15 @@ public class ObserverMasterTest extends ObserverMasterTestBase {
 
     }
 
+    private void validateObserverSyncTimeMetrics() {
+        final String name = "observer_sync_time";
+        final Map<String, Object> metrics = MetricsUtils.currentServerMetrics();
+
+        assertEquals(5, metrics.keySet().stream().filter(key -> key.contains(name)).count());
+        assertNotNull(metrics.get(String.format("avg_%s", name)));
+        assertNotNull(metrics.get(String.format("min_%s", name)));
+        assertNotNull(metrics.get(String.format("max_%s", name)));
+        assertNotNull(metrics.get(String.format("cnt_%s", name)));
+        assertNotNull(metrics.get(String.format("sum_%s", name)));
+    }
 }