From b40e18f10578533104033327162b4708edc04b4f Mon Sep 17 00:00:00 2001 From: Andre Kramer Date: Wed, 6 Oct 2021 15:17:07 +0100 Subject: [PATCH 1/3] early ruok for kubernetes ready probe to succeed --- .../zookeeper/server/NettyServerCnxn.java | 19 ++++++++++++++----- .../server/NettyServerCnxnFactory.java | 15 +++++++++++++-- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxn.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxn.java index 269fcd942cc..2560abceeaf 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxn.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxn.java @@ -473,9 +473,13 @@ private void receiveMessage(ByteBuf message) { packetReceived(4 + bb.remaining()); ZooKeeperServer zks = this.zkServer; - if (zks == null || !zks.isRunning()) { + + if (zks == null) { throw new IOException("ZK down"); + } else if (!zks.isRunning()){ + LOG.warn("Zks not running but keep processing"); } + if (initialized) { // TODO: if zks.processPacket() is changed to take a ByteBuffer[], // we could implement zero-copy queueing. @@ -521,11 +525,16 @@ private void receiveMessage(ByteBuf message) { throw new IOException("Len error " + len); } ZooKeeperServer zks = this.zkServer; - if (zks == null || !zks.isRunning()) { - throw new IOException("ZK down"); + + if (zks != null && !zks.isRunning()) { + // checkRequestSize will throw IOException if request is rejected + zks.checkRequestSizeWhenReceivingMessage(len); + } else { + if (len > 64 * 1024) { + throw new IOException("Received message size too large for uninitialized server"); + } + LOG.warn("Skip configurable check for receive length"); } - // checkRequestSize will throw IOException if request is rejected - zks.checkRequestSizeWhenReceivingMessage(len); bb = ByteBuffer.allocate(len); } } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java index 0891021f35d..6db96b49e88 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java @@ -227,12 +227,14 @@ public void channelActive(ChannelHandlerContext ctx) throws Exception { // Check the zkServer assigned to the cnxn is still running, // close it before starting the heavy TLS handshake + /* if (!cnxn.isZKServerRunning()) { LOG.warn("Zookeeper server is not running, close the connection before starting the TLS handshake"); ServerMetrics.getMetrics().CNXN_CLOSED_WITHOUT_ZK_SERVER_RUNNING.add(1); channel.close(); return; } + */ if (handshakeThrottlingEnabled) { // Favor to check and throttling even in dual mode which @@ -264,9 +266,18 @@ public void channelActive(ChannelHandlerContext ctx) throws Exception { if (remoteAddress != null && !((InetSocketAddress) remoteAddress).getAddress().isLoopbackAddress()) { LOG.trace("NettyChannelHandler channelActive: remote={} local={}", remoteAddress, cnxn.getChannel().localAddress()); - zkServer.serverStats().incrementNonMTLSRemoteConnCount(); + + if (zkServer == null || zkServer.serverStats() == null) { + LOG.warn("zkServer is not initialized " + (zkServer == null ? "" : "for stats ") + " no remote connection stats update done"); + } else { + zkServer.serverStats().incrementNonMTLSRemoteConnCount(); + } } else { - zkServer.serverStats().incrementNonMTLSLocalConnCount(); + if (zkServer == null || zkServer.serverStats() == null) { + LOG.warn("zkServer is not initialized " + (zkServer == null ? "" : "for stats ") + " no local connection count stats update done" ); + } else { + zkServer.serverStats().incrementNonMTLSLocalConnCount(); + } } } } From c926b2f4d58ad08c33ff88ff6256347dab77ef60 Mon Sep 17 00:00:00 2001 From: Andre Kramer Date: Fri, 8 Oct 2021 10:49:51 +0100 Subject: [PATCH 2/3] debug log level & remove commented out code --- .../java/org/apache/zookeeper/server/NettyServerCnxn.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxn.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxn.java index 2560abceeaf..dfaf3865c52 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxn.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxn.java @@ -477,7 +477,7 @@ private void receiveMessage(ByteBuf message) { if (zks == null) { throw new IOException("ZK down"); } else if (!zks.isRunning()){ - LOG.warn("Zks not running but keep processing"); + LOG.debug("Zks not running but keep processing"); } if (initialized) { @@ -530,10 +530,10 @@ private void receiveMessage(ByteBuf message) { // checkRequestSize will throw IOException if request is rejected zks.checkRequestSizeWhenReceivingMessage(len); } else { + LOG.debug("Skip configurable check for max receive length"); if (len > 64 * 1024) { throw new IOException("Received message size too large for uninitialized server"); } - LOG.warn("Skip configurable check for receive length"); } bb = ByteBuffer.allocate(len); } From 06e65e5fac757afec1e477e9f8b5c103243e445b Mon Sep 17 00:00:00 2001 From: Andre Kramer Date: Wed, 5 Jan 2022 14:31:05 +0000 Subject: [PATCH 3/3] review comments --- .../org/apache/zookeeper/server/NettyServerCnxn.java | 7 +------ .../zookeeper/server/NettyServerCnxnFactory.java | 11 ----------- 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxn.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxn.java index dfaf3865c52..673feca6bb8 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxn.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxn.java @@ -526,14 +526,9 @@ private void receiveMessage(ByteBuf message) { } ZooKeeperServer zks = this.zkServer; - if (zks != null && !zks.isRunning()) { + if (zks != null && zks.isRunning()) { // checkRequestSize will throw IOException if request is rejected zks.checkRequestSizeWhenReceivingMessage(len); - } else { - LOG.debug("Skip configurable check for max receive length"); - if (len > 64 * 1024) { - throw new IOException("Received message size too large for uninitialized server"); - } } bb = ByteBuffer.allocate(len); } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java index 6db96b49e88..36fc10cc178 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java @@ -225,17 +225,6 @@ public void channelActive(ChannelHandlerContext ctx) throws Exception { NettyServerCnxn cnxn = new NettyServerCnxn(channel, zkServer, NettyServerCnxnFactory.this); ctx.channel().attr(CONNECTION_ATTRIBUTE).set(cnxn); - // Check the zkServer assigned to the cnxn is still running, - // close it before starting the heavy TLS handshake - /* - if (!cnxn.isZKServerRunning()) { - LOG.warn("Zookeeper server is not running, close the connection before starting the TLS handshake"); - ServerMetrics.getMetrics().CNXN_CLOSED_WITHOUT_ZK_SERVER_RUNNING.add(1); - channel.close(); - return; - } - */ - if (handshakeThrottlingEnabled) { // Favor to check and throttling even in dual mode which // accepts both secure and insecure connections, since