[ 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:59 AM: --------------------------------------------------------------- 1. 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 in HiveServer2, the leakage problem should not occur again. The modification is as follows: {code:java} // org.apache.zookeeper.ClientCnxn.SendThread#startConnect 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); } // org.apache.zookeeper.ClientCnxn.SendThread#run if (state.isConnected()) { // determine whether we need to send an AuthFailed event. if (zooKeeperSaslClient != null) { boolean sendAuthEvent = false; if (zooKeeperSaslClient.getSaslState() == ZooKeeperSaslClient.SaslState.INITIAL) { try { zooKeeperSaslClient.initialize(ClientCnxn.this); } catch (SaslException e) { LOG.error("SASL authentication with Zookeeper Quorum member failed: " + e); state = States.AUTH_FAILED; sendAuthEvent = true; } } KeeperState authState = zooKeeperSaslClient.getKeeperState(); if (authState != null) { if (authState == KeeperState.AuthFailed) { // An authentication error occurred during authentication with the Zookeeper Server. state = States.AUTH_FAILED; sendAuthEvent = true; } else { if (authState == KeeperState.SaslAuthenticated) { sendAuthEvent = true; } } } if (sendAuthEvent == true) { eventThread.queueEvent(new WatchedEvent( Watcher.Event.EventType.None, authState,null)); if (state == States.AUTH_FAILED) { eventThread.queueEventOfDeath(); // =====================modification========================== if (ZooKeeperSaslClient.isKerberosBind()) { LOG.warn("Kerberos is bind but auth failed, Zookeeper client will go to AuthFailed state " + "and will not continue connection to Zookeeper server."); throw new SaslException("zooKeeperSaslClient.State is AuthFailed."); } } } } to = readTimeout - clientCnxnSocket.getIdleRecv(); } // org.apache.zookeeper.client.ZooKeeperSaslClient // =====================modification========================== public static final String ENABLE_CLIENT_KERBEROS_BIND_KEY = "zookeeper.sasl.kerberos.client"; public static final String ENABLE_CLIENT_KERBEROS_BIND_DEFAULT = "false"; public static boolean isKerberosBind() { return Boolean.valueOf(System.getProperty(ENABLE_CLIENT_KERBEROS_BIND_KEY, ENABLE_CLIENT_KERBEROS_BIND_DEFAULT)); }{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(). 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 in HiveServer2, the leakage problem should not occur again. The modification is as follows: {code:java} // org.apache.zookeeper.ClientCnxn.SendThread#startConnect 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); } // org.apache.zookeeper.ClientCnxn.SendThread#run if (state.isConnected()) { // determine whether we need to send an AuthFailed event. if (zooKeeperSaslClient != null) { boolean sendAuthEvent = false; if (zooKeeperSaslClient.getSaslState() == ZooKeeperSaslClient.SaslState.INITIAL) { try { zooKeeperSaslClient.initialize(ClientCnxn.this); } catch (SaslException e) { LOG.error("SASL authentication with Zookeeper Quorum member failed: " + e); state = States.AUTH_FAILED; sendAuthEvent = true; } } KeeperState authState = zooKeeperSaslClient.getKeeperState(); if (authState != null) { if (authState == KeeperState.AuthFailed) { // An authentication error occurred during authentication with the Zookeeper Server. state = States.AUTH_FAILED; sendAuthEvent = true; } else { if (authState == KeeperState.SaslAuthenticated) { sendAuthEvent = true; } } } if (sendAuthEvent == true) { eventThread.queueEvent(new WatchedEvent( Watcher.Event.EventType.None, authState,null)); if (state == States.AUTH_FAILED) { eventThread.queueEventOfDeath(); // =====================modification========================== if (ZooKeeperSaslClient.isKerberosBind()) { LOG.warn("Kerberos is bind but auth failed, Zookeeper client will go to AuthFailed state " + "and will not continue connection to Zookeeper server."); throw new SaslException("zooKeeperSaslClient.State is AuthFailed."); } } } } to = readTimeout - clientCnxnSocket.getIdleRecv(); } // org.apache.zookeeper.client.ZooKeeperSaslClient // =====================modification========================== public static final String ENABLE_CLIENT_KERBEROS_BIND_KEY = "zookeeper.sasl.kerberos.client"; public static final String ENABLE_CLIENT_KERBEROS_BIND_DEFAULT = "false"; public static boolean isKerberosBind() { return Boolean.valueOf(System.getProperty(ENABLE_CLIENT_KERBEROS_BIND_KEY, ENABLE_CLIENT_KERBEROS_BIND_DEFAULT)); }{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)