github-actions[bot] commented on code in PR #66099:
URL: https://github.com/apache/doris/pull/66099#discussion_r3662567210


##########
fe/fe-core/src/main/java/org/apache/doris/load/routineload/kafka/KafkaTaskInfo.java:
##########
@@ -126,31 +141,56 @@ public TRoutineLoadTask createRoutineLoadTask() throws 
UserException {
 
     @Override
     public void updateAdaptiveTimeout(RoutineLoadJob routineLoadJob) {
-        if (!isEof) {
+        adaptiveMinBatchIntervalSnapshotS = 
Config.routine_load_adaptive_min_batch_interval_sec;
+        if (shouldUseAdaptiveBatch(routineLoadJob)) {
             long maxBatchIntervalS = 
Math.max(routineLoadJob.getMaxBatchIntervalS(),
-                    Config.routine_load_adaptive_min_batch_interval_sec);
+                    adaptiveMinBatchIntervalSnapshotS);
             long timeoutSec = maxBatchIntervalS * 
Config.routine_load_task_timeout_multiplier;
             long realTimeoutSec = Math.max(timeoutSec, 
Config.routine_load_task_min_timeout_sec);
-            this.timeoutMs = realTimeoutSec * 1000;
+            timeoutMs = realTimeoutSec * 1000;
         } else {
-            this.timeoutMs = routineLoadJob.getTimeout() * 1000;
+            timeoutMs = routineLoadJob.getTimeout() * 1000;
         }
     }
 
     private void adaptiveBatchParam(TRoutineLoadTask tRoutineLoadTask, 
RoutineLoadJob routineLoadJob) {
         long maxBatchIntervalS = routineLoadJob.getMaxBatchIntervalS();
         long maxBatchRows = routineLoadJob.getMaxBatchRows();
         long maxBatchSize = routineLoadJob.getMaxBatchSizeBytes();
-        if (!isEof) {
-            maxBatchIntervalS = Math.max(maxBatchIntervalS, 
Config.routine_load_adaptive_min_batch_interval_sec);
-            maxBatchRows = Math.max(maxBatchRows, 
RoutineLoadJob.DEFAULT_MAX_BATCH_ROWS);
-            maxBatchSize = Math.max(maxBatchSize, 
RoutineLoadJob.DEFAULT_MAX_BATCH_SIZE);
+        if (shouldUseAdaptiveBatch(routineLoadJob)) {
+            maxBatchIntervalS = Math.max(maxBatchIntervalS, 
adaptiveMinBatchIntervalSnapshotS);
+            maxBatchSize = Math.max(maxBatchSize, 
ADAPTIVE_BATCH_BYTES_THRESHOLD);
         }
         tRoutineLoadTask.setMaxIntervalS(maxBatchIntervalS);
         tRoutineLoadTask.setMaxBatchRows(maxBatchRows);
         tRoutineLoadTask.setMaxBatchSize(maxBatchSize);
     }
 
+    private boolean shouldUseAdaptiveBatch(RoutineLoadJob routineLoadJob) {
+        if (adaptiveMinBatchIntervalSnapshotS <= 0) {
+            throw new IllegalStateException(
+                    "adaptive batch interval must be captured before beginning 
the transaction");
+        }
+        if (isEof) {
+            return false;
+        }
+        if (DebugPointUtil.isEnable("KafkaTaskInfo.shouldUseAdaptiveBatch")) {
+            return true;
+        }
+        if (previousTaskExecutionTimeMs <= 0) {
+            return false;
+        }
+        long adaptiveBatchIntervalMs = 
Math.max(routineLoadJob.getMaxBatchIntervalS(),
+                adaptiveMinBatchIntervalSnapshotS) * 1000L;
+        // Estimate the data volume in an adaptive interval from the previous 
task throughput.
+        return reachesAdaptiveBytesThreshold(adaptiveBatchIntervalMs);

Review Comment:
   [P1] Keep the row side of adaptive batching
   
   This now admits only on projected bytes and also leaves `maxBatchRows` at 
the user value. For example, the new `highRowsOnly` sample has 2,000,000 rows 
in 20 seconds, so it projects 36,000,000 rows over 360 seconds and exceeds the 
stated internal 20M-row threshold, but this return still rejects it because its 
bytes are tiny. In the other direction, a byte-qualified task with the valid 
200,000-row minimum is sent that unchanged row cap; BE stops as soon as 
`left_rows` reaches zero, so the task can still end around the original short 
interval instead of realizing the adaptive batch. The old adaptive branch 
raised both row and byte limits. Please carry the previous total-row sample, 
admit on projected 20M rows **or** 1 GiB, restore the 20M adaptive row cap, and 
update the `highRowsOnly` oracle.



##########
fe/fe-core/src/main/java/org/apache/doris/load/routineload/kafka/KafkaTaskInfo.java:
##########
@@ -126,31 +141,56 @@ public TRoutineLoadTask createRoutineLoadTask() throws 
UserException {
 
     @Override
     public void updateAdaptiveTimeout(RoutineLoadJob routineLoadJob) {
-        if (!isEof) {
+        adaptiveMinBatchIntervalSnapshotS = 
Config.routine_load_adaptive_min_batch_interval_sec;
+        if (shouldUseAdaptiveBatch(routineLoadJob)) {
             long maxBatchIntervalS = 
Math.max(routineLoadJob.getMaxBatchIntervalS(),
-                    Config.routine_load_adaptive_min_batch_interval_sec);
+                    adaptiveMinBatchIntervalSnapshotS);
             long timeoutSec = maxBatchIntervalS * 
Config.routine_load_task_timeout_multiplier;
             long realTimeoutSec = Math.max(timeoutSec, 
Config.routine_load_task_min_timeout_sec);
-            this.timeoutMs = realTimeoutSec * 1000;
+            timeoutMs = realTimeoutSec * 1000;
         } else {
-            this.timeoutMs = routineLoadJob.getTimeout() * 1000;
+            timeoutMs = routineLoadJob.getTimeout() * 1000;
         }
     }
 
     private void adaptiveBatchParam(TRoutineLoadTask tRoutineLoadTask, 
RoutineLoadJob routineLoadJob) {
         long maxBatchIntervalS = routineLoadJob.getMaxBatchIntervalS();
         long maxBatchRows = routineLoadJob.getMaxBatchRows();
         long maxBatchSize = routineLoadJob.getMaxBatchSizeBytes();
-        if (!isEof) {
-            maxBatchIntervalS = Math.max(maxBatchIntervalS, 
Config.routine_load_adaptive_min_batch_interval_sec);
-            maxBatchRows = Math.max(maxBatchRows, 
RoutineLoadJob.DEFAULT_MAX_BATCH_ROWS);
-            maxBatchSize = Math.max(maxBatchSize, 
RoutineLoadJob.DEFAULT_MAX_BATCH_SIZE);
+        if (shouldUseAdaptiveBatch(routineLoadJob)) {
+            maxBatchIntervalS = Math.max(maxBatchIntervalS, 
adaptiveMinBatchIntervalSnapshotS);
+            maxBatchSize = Math.max(maxBatchSize, 
ADAPTIVE_BATCH_BYTES_THRESHOLD);
         }
         tRoutineLoadTask.setMaxIntervalS(maxBatchIntervalS);
         tRoutineLoadTask.setMaxBatchRows(maxBatchRows);
         tRoutineLoadTask.setMaxBatchSize(maxBatchSize);
     }
 
+    private boolean shouldUseAdaptiveBatch(RoutineLoadJob routineLoadJob) {
+        if (adaptiveMinBatchIntervalSnapshotS <= 0) {

Review Comment:
   [P1] Do not strand tasks on an accepted config value
   
   `routine_load_adaptive_min_batch_interval_sec` is mutable and its default 
config handler accepts `0` or a negative integer, so such an ADMIN SET reaches 
this check. By then `RoutineLoadTaskScheduler` has already removed the task 
from the queue and assigned its `beId`; `updateAdaptiveTimeout()` is outside 
the allocation/begin/create recovery blocks, and the outer loop only logs this 
exception. The task is never requeued, keeps `beId != -1`, has no 
transaction/start time so it never times out, and continues consuming a 
routine-load slot. Please separate the “snapshot captured” state from the 
interval value (or reject non-positive values before applying them) and ensure 
a failure here releases/requeues or deterministically pauses the task.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


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

Reply via email to