deardeng commented on code in PR #67621:
URL: https://github.com/apache/doris/pull/67621#discussion_r4022000147
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/TabletSlidingWindowAccessStats.java:
##########
@@ -18,294 +18,193 @@
package org.apache.doris.catalog;
import org.apache.doris.common.Config;
-import org.apache.doris.common.util.MasterDaemon;
+import org.apache.doris.thrift.TActiveTabletStat;
-import com.google.common.hash.HashFunction;
-import com.google.common.hash.Hashing;
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
+import com.google.common.collect.Maps;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
+import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
-import java.util.PriorityQueue;
import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.LinkedBlockingQueue;
-import java.util.concurrent.ThreadPoolExecutor;
-import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
-import java.util.concurrent.atomic.AtomicLongArray;
/**
- * Sliding window access statistics utility class.
- * Supports tracking access statistics for different types of IDs (tablet,
replica, backend, etc.)
+ * Active tablet access statistics reported by backends.
*/
public class TabletSlidingWindowAccessStats {
- private static final Logger LOG =
LogManager.getLogger(TabletSlidingWindowAccessStats.class);
-
private static volatile TabletSlidingWindowAccessStats instance;
- private static final HashFunction SHARD_HASH = Hashing.murmur3_128();
-
- // Sort active IDs by accessCount desc, then lastAccessTime desc
- private static final Comparator<AccessStatsResult> TOPN_ACTIVE_COMPARATOR =
- Comparator.comparingLong((AccessStatsResult r) ->
r.accessCount).reversed()
- .thenComparing(Comparator.comparingLong((AccessStatsResult
r) -> r.lastAccessTime).reversed());
-
- // Time window in milliseconds (default: 1 hour)
- private final long timeWindowMs;
+ // Hottest first, most recently touched breaking a tie. Reversing the
whole chain is the
+ // same as reversing each key, and reads as the one sentence above.
+ private static final Comparator<AccessStatsResult> QUERY_RATE_COMPARATOR =
+ Comparator.comparingDouble((AccessStatsResult r) -> r.scanRate)
+ .thenComparingLong(r -> r.lastAccessTime)
+ .reversed();
+ private static final Comparator<AccessStatsResult> LOAD_RATE_COMPARATOR =
+ Comparator.comparingDouble((AccessStatsResult r) -> r.loadRate)
+ .thenComparingLong(r -> r.lastAccessTime)
+ .reversed();
+
+ // beId -> (tabletId -> stats). A report updates the tablets it carries
and ages out the
+ // rest by active_tablet_sliding_window_time_window_second. Reads also
filter expired entries
+ // and reclaim expired snapshots when reports stop; backend removal calls
removeBackend().
+ private final ConcurrentHashMap<Long, Map<Long, AccessStatsResult>>
beToStats = new ConcurrentHashMap<>();
Review Comment:
Correct, and worse than the commit message said: the BE has already
committed the deltas the old master acked, so a new master cannot rebuild the
history.
Not fixing it. This is a scheduling hint, not accounting. With an empty map
`hasActiveStats` is false and the rebalancer falls back to its pre-feature
selection, so it degrades rather than misbehaves. Journaling the snapshot or
adding an epoch handshake costs far more than the precision is worth here.
##########
gensrc/thrift/MasterService.thrift:
##########
@@ -123,6 +140,12 @@ struct TReportRequest {
15: optional list<AgentService.TIndexPolicy> index_policy
// Running query/loading tasks
16: optional i64 running_tasks
+ // Top-N tablets by query scan count on this BE since the previous report.
+ // Entries set only tablet_id / scan_count_delta / last_query_time_ms /
delta_window_ms.
+ 17: optional list<TActiveTabletStat> top_query_tablets
Review Comment:
Correct, and "upgrade order does not matter" in the commit message is wrong:
BE-first means an old FE answers OK without storing, so deltas are discarded
until FE catches up.
Not fixing it either, same reasoning. This is a scheduling hint, it
self-heals once both sides are upgraded, and until then the rebalancer falls
back to its pre-feature selection. Upgrade FE first if it matters.
--
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]
