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

seawinde pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git


The following commit(s) were added to refs/heads/master by this push:
     new c2f9c2fa0b4  [fix](ldap) Improve LDAP authentication resiliency and 
diagnostics (#61673)
c2f9c2fa0b4 is described below

commit c2f9c2fa0b44b2b1ebd3caaae87152cd78ad008e
Author: seawinde <[email protected]>
AuthorDate: Fri Apr 10 11:23:25 2026 +0800

     [fix](ldap) Improve LDAP authentication resiliency and diagnostics (#61673)
    
    ### What problem does this PR solve?
    
    This PR addresses several issues in the FE LDAP authentication path that
    could lead to login hangs, indefinite blocking, unstable search latency,
    and poor observability when the LDAP server is slow or unavailable.
    
      The main changes are:
    
    - Add configurable LDAP timeouts, `ldap_connect_timeout_ms` and
    `ldap_read_timeout_ms` (both default to 5000 ms), so LDAP bind and
    search operations do not block indefinitely.
    - Fix LDAP search connection management by removing the conflicting JNDI
    built-in pooling configuration and adding `ldap_search_use_pool` to
    support both pooled and non-pooled search mode.
    - Improve diagnosability by adding structured performance logs across
    the LDAP authentication chain, including password resolution, bind, user
    lookup, group lookup, cache hit/miss, and authentication result.
    
    Together, these changes improve FE LDAP authentication stability, make
    timeout behavior explicit and configurable, reduce the risk of login
    stalls, and provide better diagnostics for production issues.
---
 .../java/org/apache/doris/common/LdapConfig.java   | 31 ++++++++-
 .../mysql/authenticate/AuthenticatorManager.java   | 17 +++++
 .../mysql/authenticate/ldap/LdapAuthenticator.java | 38 ++++++++---
 .../doris/mysql/authenticate/ldap/LdapClient.java  | 69 +++++++++++++++++--
 .../doris/mysql/authenticate/ldap/LdapManager.java | 52 +++++++++++++--
 .../doris/mysql/privilege/UserPropertyMgr.java     |  5 ++
 .../doris/mysql/authenticate/TestLogAppender.java  | 77 ++++++++++++++++++++++
 .../authenticate/ldap/LdapAuthenticatorTest.java   | 28 ++++++++
 .../mysql/authenticate/ldap/LdapClientTest.java    | 65 ++++++++++++++++++
 .../mysql/authenticate/ldap/LdapManagerTest.java   | 31 +++++++++
 10 files changed, 393 insertions(+), 20 deletions(-)

diff --git a/fe/fe-common/src/main/java/org/apache/doris/common/LdapConfig.java 
b/fe/fe-common/src/main/java/org/apache/doris/common/LdapConfig.java
index 881840696dc..be25b5af0a4 100644
--- a/fe/fe-common/src/main/java/org/apache/doris/common/LdapConfig.java
+++ b/fe/fe-common/src/main/java/org/apache/doris/common/LdapConfig.java
@@ -87,7 +87,36 @@ public class LdapConfig extends ConfigBase {
     public static long ldap_cache_timeout_day = 30;
 
     /**
-     * LDAP pool configuration:
+     * LDAP read timeout in milliseconds.
+     * Controls the maximum time to wait for an LDAP response after a request 
is sent.
+     * Uses JNDI property "com.sun.jndi.ldap.read.timeout".
+     * Set to 0 for no timeout (not recommended). Default 5000ms.
+     */
+    @ConfigBase.ConfField
+    public static int ldap_read_timeout_ms = 5000;
+
+    /**
+     * LDAP connect timeout in milliseconds.
+     * Controls the maximum time to wait for establishing a TCP connection to 
the LDAP server.
+     * Uses JNDI property "com.sun.jndi.ldap.connect.timeout".
+     * Set to 0 for no timeout (not recommended). Default 5000ms.
+     */
+    @ConfigBase.ConfField
+    public static int ldap_connect_timeout_ms = 5000;
+
+    /**
+     * Whether to use connection pooling for LDAP search operations.
+     * When true (default), uses Spring PoolingContextSource with ldap_pool_* 
settings.
+     * When false, each LDAP search creates a fresh connection, avoiding dead 
connection
+     * detection cost (testOnBorrow can burn read_timeout discovering stale 
connections
+     * killed by firewalls/NAT idle timeout). Recommended to set false if 
experiencing
+     * intermittent ~5s LDAP search latency spikes.
+     */
+    @ConfigBase.ConfField
+    public static boolean ldap_search_use_pool = true;
+
+    /**
+     * LDAP pool configuration (only effective when ldap_search_use_pool = 
true):
      * 
https://docs.spring.io/spring-ldap/docs/2.3.3.RELEASE/reference/#pool-configuration
      */
     /**
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/mysql/authenticate/AuthenticatorManager.java
 
b/fe/fe-core/src/main/java/org/apache/doris/mysql/authenticate/AuthenticatorManager.java
index 061d405a25a..ea6173ec527 100644
--- 
a/fe/fe-core/src/main/java/org/apache/doris/mysql/authenticate/AuthenticatorManager.java
+++ 
b/fe/fe-core/src/main/java/org/apache/doris/mysql/authenticate/AuthenticatorManager.java
@@ -161,8 +161,16 @@ public class AuthenticatorManager {
                                 MysqlSerializer serializer,
                                 MysqlAuthPacket authPacket,
                                 MysqlHandshakePacket handshakePacket) throws 
IOException {
+
         String remoteIp = context.getMysqlChannel().getRemoteIp();
         Authenticator primaryAuthenticator = chooseAuthenticator(userName, 
remoteIp);
+        boolean debugEnabled = LOG.isDebugEnabled();
+        long resolveStart = 0L;
+        if (debugEnabled) {
+            LOG.debug("AuthenticatorManager: user={}, authenticator={}",
+                    userName, primaryAuthenticator.getClass().getSimpleName());
+            resolveStart = System.currentTimeMillis();
+        }
         Optional<AuthenticateRequest> primaryRequest = 
resolveAuthenticateRequest(primaryAuthenticator, userName,
                 context, channel, serializer, authPacket, handshakePacket);
         if (!primaryRequest.isPresent()) {
@@ -170,6 +178,11 @@ public class AuthenticatorManager {
         }
 
         AuthenticateRequest request = primaryRequest.get();
+        if (debugEnabled) {
+            long resolveElapsed = System.currentTimeMillis() - resolveStart;
+            LOG.debug("resolvePassword: user={}, elapsed={}ms", userName, 
resolveElapsed);
+            resolveStart = System.currentTimeMillis();
+        }
         remoteIp = request.getRemoteIp();
         if (isOidcAuthenticationWithoutSsl(authPacket, request)) {
             setInsecureOidcTransportError(context);
@@ -177,6 +190,10 @@ public class AuthenticatorManager {
                     new ArrayList<>());
         }
         AuthenticateResponse primaryResponse = 
authenticateWith(primaryAuthenticator, request);
+        if (debugEnabled) {
+            long authenticateElapsed = System.currentTimeMillis() - 
resolveStart;
+            LOG.debug("authenticate: user={}, elapsed={}ms", userName, 
authenticateElapsed);
+        }
         if (primaryResponse.isSuccess()) {
             return finishSuccessfulAuthentication(context, remoteIp, 
primaryResponse, false);
         }
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/mysql/authenticate/ldap/LdapAuthenticator.java
 
b/fe/fe-core/src/main/java/org/apache/doris/mysql/authenticate/ldap/LdapAuthenticator.java
index d9e27e4a9fb..b5502f7bcaf 100644
--- 
a/fe/fe-core/src/main/java/org/apache/doris/mysql/authenticate/ldap/LdapAuthenticator.java
+++ 
b/fe/fe-core/src/main/java/org/apache/doris/mysql/authenticate/ldap/LdapAuthenticator.java
@@ -58,6 +58,7 @@ public class LdapAuthenticator implements Authenticator {
      */
     @Override
     public AuthenticateResponse authenticate(AuthenticateRequest request) 
throws IOException {
+        long start = System.currentTimeMillis();
         if (LOG.isDebugEnabled()) {
             LOG.debug("user:{} start to ldap authenticate.", 
request.getUserName());
         }
@@ -66,7 +67,14 @@ public class LdapAuthenticator implements Authenticator {
             return AuthenticateResponse.failedResponse;
         }
         ClearPassword clearPassword = (ClearPassword) password;
-        return internalAuthenticate(clearPassword.getPassword(), 
request.getUserName(), request.getRemoteIp());
+        AuthenticateResponse response = 
internalAuthenticate(clearPassword.getPassword(),
+                request.getUserName(), request.getRemoteIp());
+        long elapsed = System.currentTimeMillis() - start;
+        if (LOG.isDebugEnabled()) {
+            LOG.debug("LdapAuthenticator.authenticate: user={}, success={}, 
elapsed={}ms",
+                    request.getUserName(), response.isSuccess(), elapsed);
+        }
+        return response;
     }
 
     @Override
@@ -74,11 +82,14 @@ public class LdapAuthenticator implements Authenticator {
         if (qualifiedUser.equals(Auth.ROOT_USER) || 
qualifiedUser.equals(Auth.ADMIN_USER)) {
             return false;
         }
-        // Fixme Note: LdapManager should be managed internally within the 
Ldap plugin
-        // and not be placed inside the Env class. This ensures that 
Ldap-related
-        // logic and dependencies are encapsulated within the plugin, promoting
-        // better modularity and maintainability.
-        return 
Env.getCurrentEnv().getAuth().getLdapManager().doesUserExist(qualifiedUser);
+        long start = System.currentTimeMillis();
+        boolean result = 
Env.getCurrentEnv().getAuth().getLdapManager().doesUserExist(qualifiedUser);
+        long elapsed = System.currentTimeMillis() - start;
+        if (LOG.isDebugEnabled()) {
+            LOG.debug("LdapAuthenticator.canDeal: user={}, result={}, 
elapsed={}ms",
+                    qualifiedUser, result, elapsed);
+        }
+        return result;
     }
 
     /**
@@ -98,12 +109,14 @@ public class LdapAuthenticator implements Authenticator {
         // check user password by ldap server.
         try {
             if 
(!Env.getCurrentEnv().getAuth().getLdapManager().checkUserPasswd(qualifiedUser, 
password)) {
-                LOG.info("user:{} use check LDAP password failed.", userName);
+                if (LOG.isDebugEnabled()) {
+                    LOG.debug("internalAuthenticate: user={}, success=false", 
userName);
+                }
                 ErrorReport.report(ErrorCode.ERR_ACCESS_DENIED_ERROR, 
qualifiedUser, remoteIp, usePasswd);
                 return AuthenticateResponse.failedResponse;
             }
         } catch (Exception e) {
-            LOG.error("Check ldap password error.", e);
+            LOG.warn("internalAuthenticate failed: user={}", userName, e);
             return AuthenticateResponse.failedResponse;
         }
 
@@ -114,12 +127,17 @@ public class LdapAuthenticator implements Authenticator {
         AuthenticateResponse response = new AuthenticateResponse(true);
         if (userIdentities.isEmpty()) {
             response.setUserIdentity(tempUserIdentity);
+            response.setTemp(true);
             if (LOG.isDebugEnabled()) {
-                LOG.debug("User:{} does not exists in doris, login as 
temporary users.", userName);
+                LOG.debug("internalAuthenticate: user={}, tempUser=true, 
identity={}",
+                        userName, tempUserIdentity);
             }
-            response.setTemp(true);
         } else {
             response.setUserIdentity(userIdentities.get(0));
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("internalAuthenticate: user={}, tempUser=false, 
identity={}",
+                        userName, userIdentities.get(0));
+            }
         }
         if (LOG.isDebugEnabled()) {
             LOG.debug("ldap authentication success: identity:{}", 
response.getUserIdentity());
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/mysql/authenticate/ldap/LdapClient.java
 
b/fe/fe-core/src/main/java/org/apache/doris/mysql/authenticate/ldap/LdapClient.java
index 5d03917f0d3..36fa90cd31b 100644
--- 
a/fe/fe-core/src/main/java/org/apache/doris/mysql/authenticate/ldap/LdapClient.java
+++ 
b/fe/fe-core/src/main/java/org/apache/doris/mysql/authenticate/ldap/LdapClient.java
@@ -29,6 +29,7 @@ import com.google.common.collect.Lists;
 import lombok.Data;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.springframework.ldap.AuthenticationException;
 import org.springframework.ldap.core.DirContextOperations;
 import org.springframework.ldap.core.LdapTemplate;
 import org.springframework.ldap.core.support.AbstractContextMapper;
@@ -39,7 +40,9 @@ import org.springframework.ldap.query.LdapQuery;
 import org.springframework.ldap.support.LdapEncoder;
 import 
org.springframework.ldap.transaction.compensating.manager.TransactionAwareContextSourceProxy;
 
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 // This class is used to connect to the LDAP service.
 public class LdapClient {
@@ -60,7 +63,15 @@ public class LdapClient {
         public ClientInfo(String ldapPassword) {
             this.ldapPassword = ldapPassword;
             setLdapTemplateNoPool(ldapPassword);
-            setLdapTemplatePool(ldapPassword);
+            if (LdapConfig.ldap_search_use_pool) {
+                setLdapTemplatePool(ldapPassword);
+            }
+        }
+
+        // Returns the LdapTemplate for search operations:
+        // pooled if ldap_search_use_pool=true, non-pooled otherwise.
+        public LdapTemplate getSearchTemplate() {
+            return ldapTemplatePool != null ? ldapTemplatePool : 
ldapTemplateNoPool;
         }
 
         private void setLdapTemplateNoPool(String ldapPassword) {
@@ -71,6 +82,7 @@ public class LdapClient {
             contextSource.setUrl(url);
             contextSource.setUserDn(LdapConfig.ldap_admin_name);
             contextSource.setPassword(ldapPassword);
+            setLdapTimeoutProperties(contextSource);
             contextSource.afterPropertiesSet();
             ldapTemplateNoPool = new LdapTemplate(contextSource);
             ldapTemplateNoPool.setIgnorePartialResultException(true);
@@ -84,7 +96,7 @@ public class LdapClient {
             contextSource.setUrl(url);
             contextSource.setUserDn(LdapConfig.ldap_admin_name);
             contextSource.setPassword(ldapPassword);
-            contextSource.setPooled(true);
+            setLdapTimeoutProperties(contextSource);
             contextSource.afterPropertiesSet();
 
             PoolingContextSource poolingContextSource = new 
PoolingContextSource();
@@ -109,6 +121,21 @@ public class LdapClient {
             return this.ldapPassword == null || 
!this.ldapPassword.equals(ldapPassword);
         }
 
+        private void setLdapTimeoutProperties(LdapContextSource contextSource) 
{
+            Map<String, Object> baseEnv = new HashMap<>();
+            if (LdapConfig.ldap_read_timeout_ms > 0) {
+                baseEnv.put("com.sun.jndi.ldap.read.timeout",
+                        String.valueOf(LdapConfig.ldap_read_timeout_ms));
+            }
+            if (LdapConfig.ldap_connect_timeout_ms > 0) {
+                baseEnv.put("com.sun.jndi.ldap.connect.timeout",
+                        String.valueOf(LdapConfig.ldap_connect_timeout_ms));
+            }
+            if (!baseEnv.isEmpty()) {
+                contextSource.setBaseEnvironmentProperties(baseEnv);
+            }
+        }
+
     }
 
     private void init() {
@@ -143,19 +170,37 @@ public class LdapClient {
 
     boolean checkPassword(String userName, String password) {
         init();
+        long start = System.currentTimeMillis();
         try {
             
clientInfo.getLdapTemplateNoPool().authenticate(org.springframework.ldap.query.LdapQueryBuilder.query()
                     .base(LdapConfig.ldap_user_basedn)
                     .filter(applyLoginFilter(LdapConfig.ldap_user_filter, 
userName)), password);
+            long elapsed = System.currentTimeMillis() - start;
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("LdapClient.checkPassword: user={}, success=true, 
elapsed={}ms",
+                        userName, elapsed);
+            }
             return true;
+        } catch (AuthenticationException e) {
+            long elapsed = System.currentTimeMillis() - start;
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("LdapClient.checkPassword: user={}, success=false, 
elapsed={}ms, "
+                                + "errorClass={}, errorMessage={}",
+                        userName, elapsed, e.getClass().getSimpleName(), 
e.getMessage());
+            }
+            return false;
         } catch (Exception e) {
-            LOG.info("ldap client checkPassword failed, userName: {}", 
userName, e);
+            long elapsed = System.currentTimeMillis() - start;
+            LOG.warn("LdapClient.checkPassword failed: user={}, elapsed={}ms, "
+                            + "errorClass={}, errorMessage={}",
+                    userName, elapsed, e.getClass().getSimpleName(), 
e.getMessage(), e);
             return false;
         }
     }
 
     // Search group DNs by 'member' attribution.
     List<String> getGroups(String userName) {
+        long start = System.currentTimeMillis();
         List<String> groups = Lists.newArrayList();
         if (LdapConfig.ldap_group_basedn.isEmpty()) {
             return groups;
@@ -190,6 +235,11 @@ public class LdapClient {
                 groups.add(strings[1]);
             }
         }
+        long elapsed = System.currentTimeMillis() - start;
+        if (LOG.isDebugEnabled()) {
+            LOG.debug("LdapClient.getGroups: user={}, groups={}, elapsed={}ms",
+                    userName, groups.size(), elapsed);
+        }
         return groups;
     }
 
@@ -211,18 +261,27 @@ public class LdapClient {
 
     public List<String> getDn(LdapQuery query) {
         init();
+        long start = System.currentTimeMillis();
         try {
-            return clientInfo.getLdapTemplatePool().search(query,
+            List<String> result = clientInfo.getSearchTemplate().search(query,
                     new AbstractContextMapper<String>() {
                         protected String doMapFromContext(DirContextOperations 
ctx) {
                             return ctx.getNameInNamespace();
                         }
                     });
+            long elapsed = System.currentTimeMillis() - start;
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("LdapClient.getDn: base={}, elapsed={}ms, 
results={}",
+                        query.base(), elapsed, result == null ? 0 : 
result.size());
+            }
+            return result;
         } catch (Exception e) {
+            long elapsed = System.currentTimeMillis() - start;
             String msg
                     = "Failed to retrieve the user's Distinguished Name (DN),"
                     + "This may be due to incorrect LDAP configuration or an 
unset/incorrect LDAP admin password.";
-            LOG.error(msg, e);
+            LOG.warn("LdapClient.getDn failed: base={}, elapsed={}ms, 
error={}",
+                    query.base(), elapsed, e.getMessage(), e);
             ErrorReport.report(ErrorCode.ERROR_LDAP_CONFIGURATION_ERR);
             throw new RuntimeException(msg);
         }
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/mysql/authenticate/ldap/LdapManager.java
 
b/fe/fe-core/src/main/java/org/apache/doris/mysql/authenticate/ldap/LdapManager.java
index cf5adeb670f..f279647f2a5 100644
--- 
a/fe/fe-core/src/main/java/org/apache/doris/mysql/authenticate/ldap/LdapManager.java
+++ 
b/fe/fe-core/src/main/java/org/apache/doris/mysql/authenticate/ldap/LdapManager.java
@@ -80,14 +80,28 @@ public class LdapManager {
         if (!checkParam(fullName)) {
             return null;
         }
+        long start = System.currentTimeMillis();
         LdapUserInfo ldapUserInfo = getUserInfoFromCache(fullName);
         if (ldapUserInfo != null && !ldapUserInfo.checkTimeout()) {
+            long elapsed = System.currentTimeMillis() - start;
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("LdapManager.getUserInfo: user={}, cacheHit=true, 
elapsed={}ms",
+                        fullName, elapsed);
+            }
             return ldapUserInfo;
         }
         try {
-            return getUserInfoAndUpdateCache(fullName);
+            LdapUserInfo result = getUserInfoAndUpdateCache(fullName);
+            long elapsed = System.currentTimeMillis() - start;
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("LdapManager.getUserInfo: user={}, cacheHit=false, 
elapsed={}ms",
+                        fullName, elapsed);
+            }
+            return result;
         } catch (DdlException e) {
-            LOG.warn("getUserInfo for {} failed", fullName, e);
+            long elapsed = System.currentTimeMillis() - start;
+            LOG.warn("LdapManager.getUserInfo failed: user={}, elapsed={}ms",
+                    fullName, elapsed, e);
             return null;
         }
     }
@@ -96,11 +110,19 @@ public class LdapManager {
         if (!checkParam(fullName)) {
             return false;
         }
+        long start = System.currentTimeMillis();
         LdapUserInfo info = getUserInfo(fullName);
-        return !Objects.isNull(info) && info.isExists();
+        boolean exists = !Objects.isNull(info) && info.isExists();
+        long elapsed = System.currentTimeMillis() - start;
+        if (LOG.isDebugEnabled()) {
+            LOG.debug("LdapManager.doesUserExist: user={}, exists={}, 
elapsed={}ms",
+                    fullName, exists, elapsed);
+        }
+        return exists;
     }
 
     public boolean checkUserPasswd(String fullName, String passwd) {
+        long start = System.currentTimeMillis();
         String userName = fullName;
         if (AuthenticateType.getAuthTypeConfig() != AuthenticateType.LDAP || 
Strings.isNullOrEmpty(userName)
                 || Objects.isNull(passwd)) {
@@ -108,13 +130,28 @@ public class LdapManager {
         }
         LdapUserInfo ldapUserInfo = getUserInfo(fullName);
         if (Objects.isNull(ldapUserInfo) || !ldapUserInfo.isExists()) {
+            long elapsed = System.currentTimeMillis() - start;
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("LdapManager.checkUserPasswd: user={}, 
result=user_not_found, elapsed={}ms",
+                        fullName, elapsed);
+            }
             return false;
         }
 
         if (ldapUserInfo.isSetPasswd() && 
ldapUserInfo.getPasswd().equals(passwd)) {
+            long elapsed = System.currentTimeMillis() - start;
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("LdapManager.checkUserPasswd: user={}, 
result=cached_passwd_match, elapsed={}ms",
+                        fullName, elapsed);
+            }
             return true;
         }
         boolean isRightPasswd = ldapClient.checkPassword(userName, passwd);
+        long elapsed = System.currentTimeMillis() - start;
+        if (LOG.isDebugEnabled()) {
+            LOG.debug("LdapManager.checkUserPasswd: user={}, result={}, 
elapsed={}ms",
+                    fullName, isRightPasswd ? "ldap_auth_ok" : 
"ldap_auth_fail", elapsed);
+        }
         if (!isRightPasswd) {
             return false;
         }
@@ -131,8 +168,15 @@ public class LdapManager {
     }
 
     public Set<Role> getUserRoles(String fullName) {
+        long start = System.currentTimeMillis();
         LdapUserInfo info = getUserInfo(fullName);
-        return info == null ? Collections.emptySet() : info.getRoles();
+        Set<Role> roles = info == null ? Collections.emptySet() : 
info.getRoles();
+        long elapsed = System.currentTimeMillis() - start;
+        if (LOG.isDebugEnabled()) {
+            LOG.debug("LdapManager.getUserRoles: user={}, roles={}, 
elapsed={}ms",
+                    fullName, roles.size(), elapsed);
+        }
+        return roles;
     }
 
     private boolean checkParam(String fullName) {
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/UserPropertyMgr.java
 
b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/UserPropertyMgr.java
index 54cccca96a3..8958db17697 100644
--- 
a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/UserPropertyMgr.java
+++ 
b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/UserPropertyMgr.java
@@ -271,6 +271,11 @@ public class UserPropertyMgr implements Writable {
      * If the authentication type is LDAP and the user exists in LDAP, return 
DEFAULT_USER_PROPERTY.
      * If the authentication type is not the default type, return 
DEFAULT_USER_PROPERTY.
      * Otherwise, return existProperty.
+     *
+     * Note: Previously this method called LdapManager.doesUserExist() to 
check LDAP,
+     * but that was redundant: when authentication_type=LDAP (non-default), 
the fallback
+     * condition already returns DEFAULT_USER_PROPERTY. The LDAP call caused 
runtime
+     * network queries that could hang under Auth read lock, blocking all new 
connections.
      */
     private UserProperty getPropertyIfNull(String qualifiedUser, UserProperty 
existProperty) {
         if (null != existProperty) {
diff --git 
a/fe/fe-core/src/test/java/org/apache/doris/mysql/authenticate/TestLogAppender.java
 
b/fe/fe-core/src/test/java/org/apache/doris/mysql/authenticate/TestLogAppender.java
new file mode 100644
index 00000000000..c88e81b91ce
--- /dev/null
+++ 
b/fe/fe-core/src/test/java/org/apache/doris/mysql/authenticate/TestLogAppender.java
@@ -0,0 +1,77 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.mysql.authenticate;
+
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.Logger;
+import org.apache.logging.log4j.core.appender.AbstractAppender;
+import org.apache.logging.log4j.core.config.Property;
+import org.apache.logging.log4j.core.layout.PatternLayout;
+
+import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
+
+public class TestLogAppender extends AbstractAppender implements AutoCloseable 
{
+    private final Logger logger;
+    private final Level originalLevel;
+    private final List<LogEvent> events = new CopyOnWriteArrayList<>();
+
+    private TestLogAppender(String name, Logger logger) {
+        super(name, null, PatternLayout.createDefaultLayout(), false, 
Property.EMPTY_ARRAY);
+        this.logger = logger;
+        this.originalLevel = logger.getLevel();
+    }
+
+    public static TestLogAppender attach(Class<?> loggerClass) {
+        return attach(loggerClass, Level.DEBUG);
+    }
+
+    public static TestLogAppender attach(Class<?> loggerClass, Level level) {
+        Logger logger = (Logger) LogManager.getLogger(loggerClass);
+        TestLogAppender appender = new TestLogAppender(
+                "TestLogAppender-" + loggerClass.getSimpleName() + "-" + 
System.nanoTime(), logger);
+        appender.start();
+        logger.addAppender(appender);
+        logger.setLevel(level);
+        return appender;
+    }
+
+    @Override
+    public void append(LogEvent event) {
+        events.add(event.toImmutable());
+    }
+
+    public boolean contains(Level level, String messageFragment) {
+        for (LogEvent event : events) {
+            if (event.getLevel().equals(level)
+                    && 
event.getMessage().getFormattedMessage().contains(messageFragment)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    @Override
+    public void close() {
+        logger.removeAppender(this);
+        logger.setLevel(originalLevel);
+        stop();
+    }
+}
diff --git 
a/fe/fe-core/src/test/java/org/apache/doris/mysql/authenticate/ldap/LdapAuthenticatorTest.java
 
b/fe/fe-core/src/test/java/org/apache/doris/mysql/authenticate/ldap/LdapAuthenticatorTest.java
index 99cbcdb5fad..33073f1145a 100644
--- 
a/fe/fe-core/src/test/java/org/apache/doris/mysql/authenticate/ldap/LdapAuthenticatorTest.java
+++ 
b/fe/fe-core/src/test/java/org/apache/doris/mysql/authenticate/ldap/LdapAuthenticatorTest.java
@@ -20,6 +20,7 @@ package org.apache.doris.mysql.authenticate.ldap;
 import org.apache.doris.analysis.UserIdentity;
 import org.apache.doris.mysql.authenticate.AuthenticateRequest;
 import org.apache.doris.mysql.authenticate.AuthenticateResponse;
+import org.apache.doris.mysql.authenticate.TestLogAppender;
 import org.apache.doris.mysql.authenticate.password.ClearPassword;
 import org.apache.doris.mysql.authenticate.password.ClearPasswordResolver;
 import org.apache.doris.mysql.privilege.Auth;
@@ -27,6 +28,7 @@ import org.apache.doris.mysql.privilege.Auth;
 import com.google.common.collect.Lists;
 import mockit.Expectations;
 import mockit.Mocked;
+import org.apache.logging.log4j.Level;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -111,6 +113,20 @@ public class LdapAuthenticatorTest {
         Assert.assertFalse(response.isSuccess());
     }
 
+    @Test
+    public void testAuthenticateLogsInfoWithoutThreshold() throws IOException {
+        setCheckPassword(true);
+        setGetUserInDoris(true);
+        try (TestLogAppender appender = 
TestLogAppender.attach(LdapAuthenticator.class)) {
+            AuthenticateResponse response = 
ldapAuthenticator.authenticate(request);
+            Assert.assertTrue(response.isSuccess());
+            Assert.assertTrue(appender.contains(Level.DEBUG,
+                    "LdapAuthenticator.authenticate: user=user, success=true, 
elapsed="));
+            Assert.assertFalse(appender.contains(Level.WARN,
+                    "LdapAuthenticator.authenticate slow: user=user"));
+        }
+    }
+
     @Test
     public void testAuthenticateWithCheckPasswordException() throws 
IOException {
         setCheckPasswordException();
@@ -139,6 +155,18 @@ public class LdapAuthenticatorTest {
         Assert.assertFalse(ldapAuthenticator.canDeal("ss"));
     }
 
+    @Test
+    public void testCanDealLogsInfoWithoutThreshold() {
+        setLdapUserExist(true);
+        try (TestLogAppender appender = 
TestLogAppender.attach(LdapAuthenticator.class)) {
+            Assert.assertTrue(ldapAuthenticator.canDeal("ss"));
+            Assert.assertTrue(appender.contains(Level.DEBUG,
+                    "LdapAuthenticator.canDeal: user=ss, result=true, 
elapsed="));
+            Assert.assertFalse(appender.contains(Level.WARN,
+                    "LdapAuthenticator.canDeal slow: user=ss"));
+        }
+    }
+
     @Test
     public void testGetPasswordResolver() {
         Assert.assertTrue(ldapAuthenticator.getPasswordResolver() instanceof 
ClearPasswordResolver);
diff --git 
a/fe/fe-core/src/test/java/org/apache/doris/mysql/authenticate/ldap/LdapClientTest.java
 
b/fe/fe-core/src/test/java/org/apache/doris/mysql/authenticate/ldap/LdapClientTest.java
index 7790816856f..26695c687c7 100644
--- 
a/fe/fe-core/src/test/java/org/apache/doris/mysql/authenticate/ldap/LdapClientTest.java
+++ 
b/fe/fe-core/src/test/java/org/apache/doris/mysql/authenticate/ldap/LdapClientTest.java
@@ -20,16 +20,22 @@ package org.apache.doris.mysql.authenticate.ldap;
 import org.apache.doris.common.Config;
 import org.apache.doris.common.LdapConfig;
 import org.apache.doris.common.util.NetUtils;
+import org.apache.doris.mysql.authenticate.TestLogAppender;
 
 import mockit.Expectations;
 import mockit.Tested;
+import org.apache.logging.log4j.Level;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
+import org.springframework.ldap.core.LdapTemplate;
 import org.springframework.ldap.query.LdapQuery;
 import org.springframework.ldap.support.LdapEncoder;
 
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
 import java.util.Arrays;
 import java.util.List;
 
@@ -100,6 +106,45 @@ public class LdapClientTest {
         Assert.assertEquals(1, ldapClient.getGroups("zhangsan").size());
     }
 
+    @Test
+    public void testGetGroupsLogsInfoWithoutThreshold() {
+        List<String> userDns = Arrays.asList("uid=zhangsan,dc=example,dc=com");
+        List<String> groupDns = 
Arrays.asList("cn=groupName,ou=groups,dc=example,dc=com");
+        new Expectations(ldapClient) {
+            {
+                ldapClient.getDn((LdapQuery) any);
+                result = userDns;
+                result = groupDns;
+            }
+        };
+
+        try (TestLogAppender appender = 
TestLogAppender.attach(LdapClient.class)) {
+            Assert.assertEquals(1, ldapClient.getGroups("zhangsan").size());
+            Assert.assertTrue(appender.contains(Level.DEBUG,
+                    "LdapClient.getGroups: user=zhangsan, groups=1, 
elapsed="));
+            Assert.assertFalse(appender.contains(Level.WARN,
+                    "LdapClient.getGroups slow: user=zhangsan"));
+        }
+    }
+
+    @Test
+    public void testGetSearchTemplateUsesNoPoolWhenDisabled() throws Exception 
{
+        LdapConfig.ldap_search_use_pool = false;
+
+        Object clientInfo = newClientInfo("secret");
+        Assert.assertSame(getFieldValue(clientInfo, "ldapTemplateNoPool"), 
getSearchTemplate(clientInfo));
+        Assert.assertNull(getFieldValue(clientInfo, "ldapTemplatePool"));
+    }
+
+    @Test
+    public void testGetSearchTemplateUsesPoolWhenEnabled() throws Exception {
+        LdapConfig.ldap_search_use_pool = true;
+
+        Object clientInfo = newClientInfo("secret");
+        Assert.assertNotNull(getFieldValue(clientInfo, "ldapTemplatePool"));
+        Assert.assertSame(getFieldValue(clientInfo, "ldapTemplatePool"), 
getSearchTemplate(clientInfo));
+    }
+
     @Test
     public void testSecuredProtocolIsUsed() {
         //testing default case with not specified property ldap_use_ssl or it 
is specified as false
@@ -151,5 +196,25 @@ public class LdapClientTest {
     @After
     public void tearDown() {
         LdapConfig.ldap_use_ssl = false; // restoring default value for other 
tests
+        LdapConfig.ldap_search_use_pool = true;
+    }
+
+    private Object newClientInfo(String ldapPassword) throws Exception {
+        Class<?> clientInfoClass = 
Class.forName("org.apache.doris.mysql.authenticate.ldap.LdapClient$ClientInfo");
+        Constructor<?> constructor = 
clientInfoClass.getDeclaredConstructor(String.class);
+        constructor.setAccessible(true);
+        return constructor.newInstance(ldapPassword);
+    }
+
+    private LdapTemplate getSearchTemplate(Object clientInfo) throws Exception 
{
+        Method method = 
clientInfo.getClass().getDeclaredMethod("getSearchTemplate");
+        method.setAccessible(true);
+        return (LdapTemplate) method.invoke(clientInfo);
+    }
+
+    private Object getFieldValue(Object clientInfo, String fieldName) throws 
Exception {
+        Field field = clientInfo.getClass().getDeclaredField(fieldName);
+        field.setAccessible(true);
+        return field.get(clientInfo);
     }
 }
diff --git 
a/fe/fe-core/src/test/java/org/apache/doris/mysql/authenticate/ldap/LdapManagerTest.java
 
b/fe/fe-core/src/test/java/org/apache/doris/mysql/authenticate/ldap/LdapManagerTest.java
index 8af499bbbe8..70361675054 100644
--- 
a/fe/fe-core/src/test/java/org/apache/doris/mysql/authenticate/ldap/LdapManagerTest.java
+++ 
b/fe/fe-core/src/test/java/org/apache/doris/mysql/authenticate/ldap/LdapManagerTest.java
@@ -18,9 +18,11 @@
 package org.apache.doris.mysql.authenticate.ldap;
 
 import org.apache.doris.common.Config;
+import org.apache.doris.mysql.authenticate.TestLogAppender;
 
 import mockit.Expectations;
 import mockit.Mocked;
+import org.apache.logging.log4j.Level;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
@@ -85,4 +87,33 @@ public class LdapManagerTest {
         mockClient(true, false);
         Assert.assertFalse(ldapManager.checkUserPasswd(USER2, "123"));
     }
+
+    @Test
+    public void testCheckUserPasswdCachedPasswdMatchLogsInfoWithoutThreshold() 
{
+        LdapManager ldapManager = new LdapManager();
+        mockClient(true, true);
+        Assert.assertTrue(ldapManager.checkUserPasswd(USER1, "123"));
+
+        try (TestLogAppender appender = 
TestLogAppender.attach(LdapManager.class)) {
+            Assert.assertTrue(ldapManager.checkUserPasswd(USER1, "123"));
+            Assert.assertTrue(appender.contains(Level.DEBUG,
+                    "LdapManager.checkUserPasswd: user=user1, 
result=cached_passwd_match, elapsed="));
+            Assert.assertFalse(appender.contains(Level.WARN,
+                    "LdapManager.checkUserPasswd slow: user=user1"));
+        }
+    }
+
+    @Test
+    public void testGetUserInfoLogsInfoWithoutThreshold() {
+        LdapManager ldapManager = new LdapManager();
+        mockClient(true, true);
+
+        try (TestLogAppender appender = 
TestLogAppender.attach(LdapManager.class)) {
+            Assert.assertNotNull(ldapManager.getUserInfo(USER1));
+            Assert.assertTrue(appender.contains(Level.DEBUG,
+                    "LdapManager.getUserInfo: user=user1, cacheHit=false, 
elapsed="));
+            Assert.assertFalse(appender.contains(Level.WARN,
+                    "LdapManager.getUserInfo slow: user=user1"));
+        }
+    }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]


Reply via email to