[improvement](statistics)Remove analyze retry logic. (#41224)

Remove analyze retry logic when task failed. Because usually retry would fail again and retry would bring a long time of sleep, which cause the analyze job running too slow. Master pr: #33703
apache · Oct 8, 2024 · b839490 · b839490
1 parent 5418807
commit b839490
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 47 deletions.
diff --git a/fe/fe-core/src/main/java/org/apache/doris/statistics/AnalysisManager.java b/fe/fe-core/src/main/java/org/apache/doris/statistics/AnalysisManager.java
@@ -90,6 +90,7 @@
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.NavigableMap;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.StringJoiner;
@@ -949,7 +950,7 @@ public List<AnalysisInfo> findTasks(long jobId) {
     public List<AnalysisInfo> findTasksByTaskIds(long jobId) {
         AnalysisInfo jobInfo = analysisJobInfoMap.get(jobId);
         if (jobInfo != null && jobInfo.taskIds != null) {
-            return jobInfo.taskIds.stream().map(analysisTaskInfoMap::get).filter(i -> i != null)
+            return jobInfo.taskIds.stream().map(analysisTaskInfoMap::get).filter(Objects::nonNull)
                     .collect(Collectors.toList());
         }
         return null;
@@ -966,7 +967,7 @@ public void removeAll(List<AnalysisInfo> analysisInfos) {
     public void dropAnalyzeJob(DropAnalyzeJobStmt analyzeJobStmt) throws DdlException {
         AnalysisInfo jobInfo = analysisJobInfoMap.get(analyzeJobStmt.getJobId());
         if (jobInfo == null) {
-            throw new DdlException(String.format("Analyze job [%d] not exists", jobInfo.jobId));
+            throw new DdlException(String.format("Analyze job [%d] not exists", analyzeJobStmt.getJobId()));
         }
         checkPriv(jobInfo);
         long jobId = analyzeJobStmt.getJobId();
@@ -1003,12 +1004,12 @@ public static boolean needAbandon(AnalysisInfo analysisInfo) {
         if (analysisInfo == null) {
             return true;
         }
-        if ((AnalysisState.PENDING.equals(analysisInfo.state) || AnalysisState.RUNNING.equals(analysisInfo.state))
-                && ScheduleType.ONCE.equals(analysisInfo.scheduleType)
-                && JobType.MANUAL.equals(analysisInfo.jobType)) {
+        if (analysisInfo.scheduleType == null || analysisInfo.jobType == null) {
             return true;
         }
-        return false;
+        return (AnalysisState.PENDING.equals(analysisInfo.state) || AnalysisState.RUNNING.equals(analysisInfo.state))
+                && ScheduleType.ONCE.equals(analysisInfo.scheduleType)
+                && JobType.MANUAL.equals(analysisInfo.jobType);
     }
 
     private static void readIdToTblStats(DataInput in, Map<Long, TableStatsMeta> map) throws IOException {
@@ -1177,17 +1178,14 @@ public void removeJob(long id) {
 
     /**
      * Only OlapTable and Hive HMSExternalTable can sample for now.
-     * @param table
+     * @param table Table to check
      * @return Return true if the given table can do sample analyze. False otherwise.
      */
     public boolean canSample(TableIf table) {
         if (table instanceof OlapTable) {
             return true;
         }
-        if (table instanceof HMSExternalTable
-                && ((HMSExternalTable) table).getDlaType().equals(HMSExternalTable.DLAType.HIVE)) {
-            return true;
-        }
-        return false;
+        return table instanceof HMSExternalTable
+                && ((HMSExternalTable) table).getDlaType().equals(HMSExternalTable.DLAType.HIVE);
     }
 }
diff --git a/fe/fe-core/src/main/java/org/apache/doris/statistics/BaseAnalysisTask.java b/fe/fe-core/src/main/java/org/apache/doris/statistics/BaseAnalysisTask.java
@@ -40,7 +40,6 @@
 
 import java.text.MessageFormat;
 import java.util.Collections;
-import java.util.concurrent.TimeUnit;
 
 public abstract class BaseAnalysisTask {
 
@@ -195,39 +194,16 @@ protected void init(AnalysisInfo info) {
         }
     }
 
-    public void execute() {
+    public void execute() throws Exception {
         prepareExecution();
-        executeWithRetry();
+        doExecute();
         afterExecution();
     }
 
     protected void prepareExecution() {
         setTaskStateToRunning();
     }
 
-    protected void executeWithRetry() {
-        int retriedTimes = 0;
-        while (retriedTimes < StatisticConstants.ANALYZE_TASK_RETRY_TIMES) {
-            if (killed) {
-                throw new RuntimeException("Task is Killed or Timeout");
-            }
-            try {
-                doExecute();
-                break;
-            } catch (Throwable t) {
-                if (killed) {
-                    throw new RuntimeException(t);
-                }
-                LOG.warn("Failed to execute analysis task, retried times: {}", retriedTimes++, t);
-                if (retriedTimes >= StatisticConstants.ANALYZE_TASK_RETRY_TIMES) {
-                    job.taskFailed(this, t.getMessage());
-                    throw new RuntimeException(t);
-                }
-                StatisticsUtil.sleep(TimeUnit.SECONDS.toMillis(2 ^ retriedTimes) * 10);
-            }
-        }
-    }
-
     public abstract void doExecute() throws Exception;
 
     protected void afterExecution() {}
@@ -285,9 +261,8 @@ protected String getNdvFunction(String totalRows) {
         // (https://github.com/postgres/postgres/blob/master/src/backend/commands/analyze.c)
         // (http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.93.8637&rep=rep1&type=pdf)
         // sample_row * count_distinct / ( sample_row - once_count + once_count * sample_row / total_row)
-        String fn = MessageFormat.format("{0} * {1} / ({0} - {2} + {2} * {0} / {3})", sampleRows,
+        return MessageFormat.format("{0} * {1} / ({0} - {2} + {2} * {0} / {3})", sampleRows,
                 countDistinct, onceCount, totalRows);
-        return fn;
     }
 
     // Max value is not accurate while sample, so set it to NULL to avoid optimizer generate bad plan.
@@ -345,6 +320,9 @@ protected void runQuery(String sql) {
             Env.getCurrentEnv().getStatisticsCache().syncColStats(colStatsData);
             queryId = DebugUtil.printId(stmtExecutor.getContext().queryId());
             job.appendBuf(this, Collections.singletonList(colStatsData));
+        } catch (Exception e) {
+            LOG.warn("Failed to execute sql {}", sql);
+            throw e;
         } finally {
             if (LOG.isDebugEnabled()) {
                 LOG.debug("End cost time in millisec: " + (System.currentTimeMillis() - startTime)

diff --git a/fe/fe-core/src/main/java/org/apache/doris/statistics/HistogramTask.java b/fe/fe-core/src/main/java/org/apache/doris/statistics/HistogramTask.java
@@ -78,11 +78,6 @@ public void doExecute() throws Exception {
         Env.getCurrentEnv().getStatisticsCache().refreshHistogramSync(tbl.getId(), -1, col.getName());
     }
 
-    @Override
-    protected void afterExecution() {
-        // DO NOTHING
-    }
-
     private String getSampleRateFunction() {
         if (info.analysisMethod == AnalysisMethod.FULL) {
             return "0";

diff --git a/fe/fe-core/src/main/java/org/apache/doris/statistics/StatisticConstants.java b/fe/fe-core/src/main/java/org/apache/doris/statistics/StatisticConstants.java
@@ -64,8 +64,6 @@ public class StatisticConstants {
 
     public static List<String> SYSTEM_DBS = new ArrayList<>();
 
-    public static int ANALYZE_TASK_RETRY_TIMES = 5;
-
     public static final String DB_NAME = SystemInfoService.DEFAULT_CLUSTER + ":" + FeConstants.INTERNAL_DB_NAME;
 
     public static final String FULL_QUALIFIED_STATS_TBL_NAME = InternalCatalog.INTERNAL_CATALOG_NAME