[
https://issues.apache.org/jira/browse/ZOOKEEPER-4885?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17898105#comment-17898105
]
Xin Chen edited comment on ZOOKEEPER-4885 at 11/14/24 7:41 AM:
---------------------------------------------------------------
So {*}there is a real scenario in the production environment{*}:
When a *Curator Client* used to create a EphemeralNode，like code in HiveServer2：
{code:java}
zooKeeperClient =
CuratorFrameworkFactory.builder().connectString(zooKeeperEnsemble).sessionTimeoutMs(sessionTimeout).aclProvider(zooKeeperAclProvider).retryPolicy(new
ExponentialBackoffRetry(baseSleepTime, maxRetries)).build();
zooKeeperClient.start();
PersistentEphemeralNode znode = new PersistentEphemeralNode(zooKeeperClient,
PersistentEphemeralNode.Mode.EPHEMERAL_SEQUENTIAL, pathPrefix, znodeDataUTF8);
znode.start();{code}
There is a callback function in PersistentEphemeralNode：
{code:java}
backgroundCallback = new BackgroundCallback()
{
@Override
public void processResult(CuratorFramework client, CuratorEvent event)
throws Exception
{
String path = null;
if ( event.getResultCode() ==
KeeperException.Code.NODEEXISTS.intValue() )
{
path = event.getPath();
}
else if ( event.getResultCode() == KeeperException.Code.OK.intValue() )
{
path = event.getName();
}
if ( path != null )
{
nodePath.set(path);
watchNode();
CountDownLatch localLatch = initialCreateLatch.getAndSet(null);
if ( localLatch != null )
{
localLatch.countDown();
}
}
else
{
createNode();
}
}
};
// createNode() registers this backgroundCallback again：
createMethod.withMode(mode.getCreateMode(existingPath !=
null)).inBackground(backgroundCallback).forPath(createPath, data.get());{code}
When HiveServer2 is disconnected from the Zookeeper and then network
reconnected, Curator automatically rebuilds the client. Coincidentally, the
login to kerberos fails at this time, and a non SASL authenticated client is
created. The Curator uses this client to rebuild PersistentEphemeralNode under
“/hive”. There will be an error message in creating:
{code:java}
KeeperErrorCode = InvalidACL for /hive/hiveserver2-0... {code}
and only the judgment for NODEEXISTS and OK will be made in
{*}backgroundCallback#processResult{*}. In this case, it will received
InvaldACL as 'event.getResultCode()' and repeatedly call creatNode() and
register backgroundcallback. After the error callback, it will call creatNode()
and register backgroundcallback again. So it enters a dead loop, due to the
callback processing within the Curator, the zk client of hiveserver2 is
completely unaware, while the server will frantically brush InvalidACL logs due
to the fast call of creatNode(). The CPU pressure on the zk server will also
increase dramatically.
*From here, it can be seen that neither the Curator nor Zookeeper wants to
automatically rebuild the client for scenarios where Kerberos ACL
authentication fails.*
If we don't modify the Curator code, a solution is proposed for this scenario,
which involves monitoring the event on the Hiveserver2 side. When the zk client
sends an Authfailed event, the current Curator client is closed and rebuilt,
and the ephemeral node is recreated until it is successfully created.
However, there is another issue with this modification. When the authfailed
event occurs, rebuilding the Curator client. If the zk server is disconnected
for a period of time and the reconnection may generates an expire event, the
expire event will occur after the authfailed event and trigger automatic
reconstruction within the Curator. At this point, there will be an additional
connection between hiveserver2 and the zk server, as well as an additional zk
client, which is considered a {*}client leak{*}.
{code:java}
// Expired event trigger Curator to Reset()
org.apache.curator.ConnectionState#checkState case Expired:
{
isConnected = false;
checkNewConnectionString = false;
handleExpiredSession();
break;
}
org.apache.curator.ConnectionState#handleExpiredSession
private void handleExpiredSession()
{
log.warn("Session expired event received");
tracer.get().addCount("session-expired", 1);
try
{
reset();
}
catch ( Exception e )
{
queueBackgroundException(e);
}
}
//========== reset() will rebuild Zookeeper Client =================
org.apache.curator.ConnectionState#reset
private synchronized void reset() throws Exception
{
log.debug("reset");
instanceIndex.incrementAndGet();
isConnected.set(false);
connectionStartMs = System.currentTimeMillis();
zooKeeper.closeAndReset();
zooKeeper.getZooKeeper(); // initiate connection
}{code}
{*}In order to solve the problem of client leak in this scheme{*}, it is
proposed to synchronously modify the Zookeeper client code(ClientCnxn.java).
The purpose of the modification is to avoid generating expired events after
authfailed events, so that the Curator side will not trigger
handleExpirdSession() and reset(). This is customized for some clients that
need to set SASL permission znode. The client process needs to carry an
environment variable (newly added)：
{code:java}
zookeeper.sasl.kerberos.client=true{code}
When creating such a client, after the authfailed event generated during login,
Zookeeper. state can be set to AuthFailed, so exit the retry connection with
the zk server, and close the event thread in the same time. In this way,
because the zk client has disconnected from the server internally, the expired
event will not be triggered again. We can actively initiate the reconstruction
of the client and znode, the leakage problem should not occur again. The
modification is as follows:
{code:java}
private void startConnect(InetSocketAddress addr) throws IOException {
// initializing it for new connection
saslLoginFailed = false;
state = States.CONNECTING;
setName(getName().replaceAll("\\(.*\\)",
"(" + addr.getHostName() + ":" + addr.getPort() + ")"));
if (ZooKeeperSaslClient.isEnabled()) {
try {
if (zooKeeperSaslClient != null) {
zooKeeperSaslClient.shutdown();
}
zooKeeperSaslClient = new
ZooKeeperSaslClient(SaslServerPrincipal.getServerPrincipal(addr));
} catch (LoginException e) {
// An authentication error occurred when the SASL client tried to
initialize:
// for Kerberos this means that the client failed to authenticate
with the KDC.
// This is different from an authentication error that occurs
during communication
// with the Zookeeper server, which is handled below.
eventThread.queueEvent(new WatchedEvent(
Watcher.Event.EventType.None,
Watcher.Event.KeeperState.AuthFailed, null));
saslLoginFailed = true;
// =====================modification==========================
if (ZooKeeperSaslClient.isKerberosBind()) {
state = States.AUTH_FAILED;
LOG.warn("Kerberos is bind but LoginException occurs, Zookeeper
client will go to AuthFailed state "
+ "and will not continue connection to Zookeeper
server.");
throw new SaslException(e.getMessage());
} else {
LOG.warn("SASL configuration failed: " + e + " Will continue
connection to Zookeeper server without "
+ "SASL authentication, if Zookeeper server allows
it.");
}
}
}
logStartConnect(addr);
clientCnxnSocket.connect(addr);
}{code}
was (Author: JIRAUSER298666):
So {*}there is a real scenario in the production environment{*}:
When a *Curator Client* used to create a EphemeralNode，like code in HiveServer2：
{code:java}
zooKeeperClient =
CuratorFrameworkFactory.builder().connectString(zooKeeperEnsemble).sessionTimeoutMs(sessionTimeout).aclProvider(zooKeeperAclProvider).retryPolicy(new
ExponentialBackoffRetry(baseSleepTime, maxRetries)).build();
zooKeeperClient.start();
PersistentEphemeralNode znode = new PersistentEphemeralNode(zooKeeperClient,
PersistentEphemeralNode.Mode.EPHEMERAL_SEQUENTIAL, pathPrefix, znodeDataUTF8);
znode.start();{code}
There is a callback function in PersistentEphemeralNode：
{code:java}
backgroundCallback = new BackgroundCallback()
{
@Override
public void processResult(CuratorFramework client, CuratorEvent event)
throws Exception
{
String path = null;
if ( event.getResultCode() ==
KeeperException.Code.NODEEXISTS.intValue() )
{
path = event.getPath();
}
else if ( event.getResultCode() == KeeperException.Code.OK.intValue() )
{
path = event.getName();
}
if ( path != null )
{
nodePath.set(path);
watchNode();
CountDownLatch localLatch = initialCreateLatch.getAndSet(null);
if ( localLatch != null )
{
localLatch.countDown();
}
}
else
{
createNode();
}
}
};
// createNode() registers this backgroundCallback again：
createMethod.withMode(mode.getCreateMode(existingPath !=
null)).inBackground(backgroundCallback).forPath(createPath, data.get());{code}
When HiveServer2 is disconnected from the Zookeeper and then network
reconnected, Curator automatically rebuilds the client. Coincidentally, the
login to kerberos fails at this time, and a non SASL authenticated client is
created. The Curator uses this client to rebuild PersistentEphemeralNode under
“/hive”. There will be an error message in creating:
{code:java}
KeeperErrorCode = InvalidACL for /hive/hiveserver2-0... {code}
and only the judgment for NODEEXISTS and OK will be made in
{*}backgroundCallback#processResult{*}. In this case, it will received
InvaldACL as 'event.getResultCode()' and repeatedly call creatNode() and
register backgroundcallback. After the error callback, it will call creatNode()
and register backgroundcallback again. So it enters a dead loop, due to the
callback processing within the Curator, the zk client of hiveserver2 is
completely unaware, while the server will frantically brush InvalidACL logs due
to the fast call of creatNode(). The CPU pressure on the zk server will also
increase dramatically.
*From here, it can be seen that neither the Curator nor Zookeeper wants to
automatically rebuild the client for scenarios where Kerberos ACL
authentication fails.*
If we don't modify the Curator code, a solution is proposed for this scenario,
which involves monitoring the event on the Hiveserver2 side. When the zk client
sends an Authfailed event, the current Curator client is closed and rebuilt,
and the ephemeral node is recreated until it is successfully created.
However, there is another issue with this modification. When the authfailed
event occurs, rebuilding the Curator client. If the zk server is disconnected
for a period of time and the reconnection may generates an expire event, the
expire event will occur after the authfailed event and trigger automatic
reconstruction within the Curator. At this point, there will be an additional
connection between hiveserver2 and the zk server, as well as an additional zk
client, which is considered a {*}client leak{*}.
{code:java}
// Expired event trigger Curator to Reset()
org.apache.curator.ConnectionState#checkState case Expired:
{
isConnected = false;
checkNewConnectionString = false;
handleExpiredSession();
break;
}
org.apache.curator.ConnectionState#handleExpiredSession
private void handleExpiredSession()
{
log.warn("Session expired event received");
tracer.get().addCount("session-expired", 1);
try
{
reset();
}
catch ( Exception e )
{
queueBackgroundException(e);
}
}
//========== reset() will rebuild Zookeeper Client =================
org.apache.curator.ConnectionState#reset
private synchronized void reset() throws Exception
{
log.debug("reset");
instanceIndex.incrementAndGet();
isConnected.set(false);
connectionStartMs = System.currentTimeMillis();
zooKeeper.closeAndReset();
zooKeeper.getZooKeeper(); // initiate connection
}{code}
{*}In order to solve the problem of client leak in this scheme{*}, it is
proposed to synchronously modify the Zookeeper client code(ClientCnxn.java).
The purpose of the modification is to avoid generating expired events after
authfailed events, so that the Curator side will not trigger
handleExpirdSession() and reset(). Set Zookeeper.state to AuthFailed after the
authfailed event generated during login, exit the retry connection with the zk
server, and close the event thread. The modifications are as follows:
{code:java}
private void startConnect(InetSocketAddress addr) throws IOException {
// initializing it for new connection
saslLoginFailed = false;
state = States.CONNECTING;
setName(getName().replaceAll("\\(.*\\)",
"(" + addr.getHostName() + ":" + addr.getPort() + ")"));
if (ZooKeeperSaslClient.isEnabled()) {
try {
if (zooKeeperSaslClient != null) {
zooKeeperSaslClient.shutdown();
}
zooKeeperSaslClient = new
ZooKeeperSaslClient(SaslServerPrincipal.getServerPrincipal(addr));
} catch (LoginException e) {
// An authentication error occurred when the SASL client tried to
initialize:
// for Kerberos this means that the client failed to authenticate
with the KDC.
// This is different from an authentication error that occurs
during communication
// with the Zookeeper server, which is handled below.
eventThread.queueEvent(new WatchedEvent(
Watcher.Event.EventType.None,
Watcher.Event.KeeperState.AuthFailed, null));
saslLoginFailed = true;
//
if (ZooKeeperSaslClient.isKerberosBind()) {
state = States.AUTH_FAILED;
LOG.warn("Kerberos is bind but LoginException occurs, Zookeeper
client will go to AuthFailed state "
+ "and will not continue connection to Zookeeper
server.");
throw new SaslException(e.getMessage());
} else {
LOG.warn("SASL configuration failed: " + e + " Will continue
connection to Zookeeper server without "
+ "SASL authentication, if Zookeeper server allows
it.");
}
}
}
logStartConnect(addr);
clientCnxnSocket.connect(addr);
} {code}
> Can Non-SASL-Clients automatically recover with the recovery of kerberos
> communication？
> ---------------------------------------------------------------------------------------
>
> Key: ZOOKEEPER-4885
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-4885
> Project: ZooKeeper
> Issue Type: Improvement
> Affects Versions: 3.4.14, 3.6.4, 3.9.3
> Reporter: Xin Chen
> Priority: Major
>
> About ZOOKEEPER-2139 & ZOOKEEPER-2323, it just avoids ZooKeeper clients into
> infinite AuthFailedException. Noauth Exception still exists!
> LoginException was thrown through each login, but at this point, a zkclient
> without Kerberos SASL authentication was created. Non SASL Znodes can be
> operated on in the future. However, when Kerberos recovers from network
> disconnections and other anomalies, the previously created zkclient without
> SASL authentication is still being used without rebuilding the login or
> recreating a saslclient. If it is used to operate on ACL Znodes at this time,
> an error will always be reported:
> {code:java}
> KeeperErrorCode = NoAuth for /zookeeper
> or
> KeeperErrorCode = AuthFailed for /zookeeper
> or
> KeeperErrorCode = InvalidACL for /zookeeper{code}
> Isn't this a question that should be considered? And I also met this issue
> in ZK-3.6.4，It seems that this issue has not been considered in the updated
> version.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
