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

yiguolei 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 fe36aa170cc [fix](profile) Fix profile reject  (#38030)
fe36aa170cc is described below

commit fe36aa170cce7ddc70d73812b8f1e9d975bfebfc
Author: zhiqiang <[email protected]>
AuthorDate: Thu Jul 18 23:15:39 2024 +0800

    [fix](profile) Fix profile reject  (#38030)
    
    1. We should only eject execution profiles after query has finished for
    a long enough time.
    2. Only add execution to profile manager if enable_profile is true
---
 .../apache/doris/common/util/ProfileManager.java   | 27 ++++++++++++++++------
 .../java/org/apache/doris/qe/QeProcessorImpl.java  |  5 ++--
 2 files changed, 23 insertions(+), 9 deletions(-)

diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/common/util/ProfileManager.java 
b/fe/fe-core/src/main/java/org/apache/doris/common/util/ProfileManager.java
index 3609b9890cc..9c3490b4c96 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/common/util/ProfileManager.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/common/util/ProfileManager.java
@@ -173,22 +173,27 @@ public class ProfileManager {
                 LOG.debug("Add execution profile {} to profile manager",
                         DebugUtil.printId(executionProfile.getQueryId()));
             }
-            // Check if there are some query profiles that not finish 
collecting, should
-            // remove them to release memory.
+            // This branch has two purposes:
+            // 1. discard profile collecting if its collection not finished in 
5 seconds after query finished.
+            // 2. prevent execution profile from leakage. If we have too many 
execution profiles in memory,
+            // we will remove execution profiles of query that has finished in 
5 seconds ago.
             if (queryIdToExecutionProfiles.size() > 2 * 
Config.max_query_profile_num) {
                 List<ExecutionProfile> finishOrExpireExecutionProfiles = 
Lists.newArrayList();
                 for (ExecutionProfile tmpProfile : 
queryIdToExecutionProfiles.values()) {
-                    if (System.currentTimeMillis() - 
tmpProfile.getQueryFinishTime()
-                            > Config.profile_async_collect_expire_time_secs * 
1000) {
+                    boolean queryFinishedLongEnough = 
tmpProfile.getQueryFinishTime() > 0
+                            && System.currentTimeMillis() - 
tmpProfile.getQueryFinishTime()
+                            > Config.profile_async_collect_expire_time_secs * 
1000;
+
+                    if (queryFinishedLongEnough) {
                         finishOrExpireExecutionProfiles.add(tmpProfile);
                     }
                 }
+                StringBuilder stringBuilder = new StringBuilder();
                 for (ExecutionProfile tmp : finishOrExpireExecutionProfiles) {
+                    
stringBuilder.append(DebugUtil.printId(tmp.getQueryId())).append(",");
                     queryIdToExecutionProfiles.remove(tmp.getQueryId());
-                    if (LOG.isDebugEnabled()) {
-                        LOG.debug("Remove expired execution profile {}", 
DebugUtil.printId(tmp.getQueryId()));
-                    }
                 }
+                LOG.warn("Remove expired execution profiles {}", 
stringBuilder.toString());
             }
         } finally {
             writeLock.unlock();
@@ -223,9 +228,17 @@ public class ProfileManager {
                     ProfileElement profileElementRemoved = 
queryIdToProfileMap.remove(queryIdDeque.getFirst());
                     // If the Profile object is removed from manager, then 
related execution profile is also useless.
                     if (profileElementRemoved != null) {
+                        StringBuilder sb = new StringBuilder();
                         for (ExecutionProfile executionProfile : 
profileElementRemoved.profile.getExecutionProfiles()) {
+                            
sb.append(executionProfile.getQueryId()).append(",");
                             
this.queryIdToExecutionProfiles.remove(executionProfile.getQueryId());
                         }
+                        LOG.warn("Remove expired profile {}, execution 
profiles {},"
+                                    + " queryIdDeque size {}, profile count 
{},"
+                                    + " execution profile count {} 
max_query_profile_num {}",
+                                    
profileElementRemoved.profile.getSummaryProfile().getProfileId(),
+                                    sb.toString(), queryIdDeque.size(), 
queryIdToProfileMap.size(),
+                                    queryIdToExecutionProfiles.size(), 
Config.max_query_profile_num);
                     }
                     queryIdDeque.removeFirst();
                 }
diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/QeProcessorImpl.java 
b/fe/fe-core/src/main/java/org/apache/doris/qe/QeProcessorImpl.java
index b762fdda87c..38b0ac192d2 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/qe/QeProcessorImpl.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/qe/QeProcessorImpl.java
@@ -122,10 +122,11 @@ public final class QeProcessorImpl implements QeProcessor 
{
         if (result != null) {
             throw new UserException("queryId " + queryId + " already exists");
         }
-
         // Should add the execution profile to profile manager, BE will report 
the profile to FE and FE
         // will update it in ProfileManager
-        
ProfileManager.getInstance().addExecutionProfile(info.getCoord().getExecutionProfile());
+        if (info.coord.getQueryOptions().enable_profile) {
+            
ProfileManager.getInstance().addExecutionProfile(info.getCoord().getExecutionProfile());
+        }
     }
 
     @Override


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

Reply via email to