Skip to content

Commit

Permalink
zookeeper: silence warning and log lifecycle state events.
Browse files Browse the repository at this point in the history
Motivation:

Starting with ZooKeeper v3.4.10, the stand-alone server will log the
following error message for each state change:

    ZKShutdownHandler is not registered, so ZooKeeper server won't take
    any action on ERROR or SHUTDOWN server state changes

Although the message is logged at ERROR severity, this lack of action
has no bad effect for our embedded zookeeper cell.  It certainly sounds
scary, though.

Modification:

Unfortunately there is no easy way to silence this error message, as ZK
currently provides no (easy) mechanism to register a ZKShutdownHandler.
This problem is reported as:

	https://issues.apache.org/jira/browse/ZOOKEEPER-2991

Therefore, this patch introduces a simple public class that extends the
package-private class.  This class also logs the ZK life-cycle state
transitions as they few and potentially of interest.

This patch also moves the PatchedZooKeeperServer into the
org.apache.zookeper.server Java package, to allow it to upgrade the
registerServerShutdownHandler method to public.

Finally, the intended countdown latch employed as intended.  Given
dCache closes the ZooKeeper instance "manually", the countdown latch
should have already fired by the time the shutdown(true) call returns.

Result:

No "ZKShutdownHandler is not registered" error is logged.

Potentially interesting ZK life-cycle state transitions are logged.

Target: master
Require-notes: no
Require-book: no
Patch: https://rb.dcache.org/r/10793/
Acked-by: Tigran Mkrtchyan
  • Loading branch information
paulmillar committed Mar 5, 2018
1 parent f913bcf commit 7679b60
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 21 deletions.
@@ -0,0 +1,47 @@
/* dCache - http://www.dcache.org/
*
* Copyright (C) 2018 Deutsches Elektronen-Synchrotron
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
package org.apache.zookeeper.server;

import org.apache.zookeeper.server.ZooKeeperServer.State;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.concurrent.CountDownLatch;

/**
* This class wraps ZooKeeperServerShutdownHandler primarily to provide a
* public class as a work-around for issue ZOOKEEPER-2991. See:
*
* https://issues.apache.org/jira/browse/ZOOKEEPER-2991
*/
public class LoggingShutdownHandler extends ZooKeeperServerShutdownHandler
{
private static final Logger LOGGER = LoggerFactory.getLogger(LoggingShutdownHandler.class);

public LoggingShutdownHandler(CountDownLatch shutdownLatch)
{
super(shutdownLatch);
}

@Override
void handle(State state)
{
LOGGER.warn("Zookeeper server now {}", state);
super.handle(state);
}
}
@@ -0,0 +1,48 @@
/* dCache - http://www.dcache.org/
*
* Copyright (C) 2018 Deutsches Elektronen-Synchrotron
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
package org.apache.zookeeper.server;

/**
* This class contains various work-arounds for bugs in ZooKeeper.
*/
public class PatchedZooKeeperServer extends ZooKeeperServer
{
// Work around https://issues.apache.org/jira/browse/ZOOKEEPER-2515
// and https://issues.apache.org/jira/browse/ZOOKEEPER-2812
@Override
public void createSessionTracker() {
sessionTracker = new SessionTrackerImpl(this, getZKDatabase().getSessionWithTimeOuts(),
tickTime, 1, getZooKeeperServerListener())
{
@Override
public void shutdown() {
super.shutdown();
synchronized (this) {
notifyAll();
}
}
};
}

// Work-around for https://issues.apache.org/jira/browse/ZOOKEEPER-2991
@Override
public void registerServerShutdownHandler(ZooKeeperServerShutdownHandler handler)
{
super.registerServerShutdownHandler(handler);
}
}
Expand Up @@ -19,7 +19,9 @@

import com.google.common.base.Strings;
import org.apache.zookeeper.server.DatadirCleanupManager;
import org.apache.zookeeper.server.LoggingShutdownHandler;
import org.apache.zookeeper.server.NIOServerCnxnFactory;
import org.apache.zookeeper.server.PatchedZooKeeperServer;
import org.apache.zookeeper.server.ServerCnxnFactory;
import org.apache.zookeeper.server.SessionTrackerImpl;
import org.apache.zookeeper.server.ZKDatabase;
Expand All @@ -32,6 +34,7 @@
import java.io.IOException;
import java.io.PrintWriter;
import java.net.InetSocketAddress;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;

import org.dcache.cells.AbstractCell;
Expand All @@ -47,27 +50,6 @@ public class ZooKeeperCell extends AbstractCell
{
private static final Logger LOG = LoggerFactory.getLogger(ZooKeeperCell.class);

private class PatchedZooKeeperServer extends ZooKeeperServer
{
// Work around https://issues.apache.org/jira/browse/ZOOKEEPER-2515
// and https://issues.apache.org/jira/browse/ZOOKEEPER-2812
@Override
public void createSessionTracker() {
sessionTracker = new SessionTrackerImpl(this, getZKDatabase().getSessionWithTimeOuts(),
tickTime, 1, getZooKeeperServerListener())
{
@Override
public void shutdown() {
super.shutdown();
synchronized (this) {
notifyAll();
}
}
};
}
}


@Option(name = "data-log-dir", required = true)
protected File dataLogDir;

Expand Down Expand Up @@ -110,6 +92,7 @@ public void shutdown() {
@Option(name = "autoPurgeIntervalUnit", required = true)
protected TimeUnit autoPurgeIntervalUnit;

private final CountDownLatch shutdownLatch = new CountDownLatch(1);
private ServerCnxnFactory cnxnFactory;
private FileTxnSnapLog txnLog;
private PatchedZooKeeperServer zkServer;
Expand All @@ -130,6 +113,9 @@ protected void starting() throws Exception
checkArgument(autoPurgeInterval > 0, "zookeeper.auto-purge.purge-interval must be non-negative.");
zkServer = new PatchedZooKeeperServer();

// Work-around https://issues.apache.org/jira/browse/ZOOKEEPER-2991
zkServer.registerServerShutdownHandler(new LoggingShutdownHandler(shutdownLatch));

txnLog = new FileTxnSnapLog(dataLogDir, dataDir);
zkServer.setTxnLogFactory(txnLog);
zkServer.setTickTime((int) tickTimeUnit.toMillis(tickTime));
Expand Down Expand Up @@ -175,6 +161,14 @@ public void stopped()
// Must shutdown zkServer before shutting down cnxnFactory since
// zkServer.shutdown flushes any pending messages while
// cnxnFactory.shutdown closes the network sockets.

try {
// This should be a no-op, as ZK should already have shutdown.
shutdownLatch.await(1, TimeUnit.SECONDS);
} catch (InterruptedException e) {
LOG.error("ZooKeeper server failed to shutdown.");
}

if (cnxnFactory != null) {
cnxnFactory.shutdown();
}
Expand Down

0 comments on commit 7679b60

Please sign in to comment.