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:
Confirmed, and sharper than the commit message claimed. It described the
exposure as "an empty map for up to one report interval"; that understates it.
FE answers OK before the snapshot is durable anywhere, the BE reads OK as
delivery and commits its baseline, so the retained window history exists only
in the old master's heap. A new master cannot reconstruct it — the next report
carries only the delta accumulated since that commit, not the hour of history
that was already consumed.
Not fixing it in this PR. The two realistic options:
- Journal `beToStats`. That is `backendCount` writes into the edit log every
60s, for data whose entire value proposition is being cheap and disposable.
Rejected.
- An epoch handshake: FE carries its term in the heartbeat, and a BE that
observes a new term reports every tablet inside
`active_tablet_sliding_window_time_window_second` regardless of delta,
rebuilding the active set in one round. This is the right fix, but it is a new
protocol mechanism with its own payload-size and correctness questions, and it
does not belong bolted onto this change.
What this PR leans on instead is that the degradation is safe rather than
wrong. `CloudTabletRebalancer` already computes `preferCold =
Config.enable_cloud_active_tablet_priority_scheduling && hasActiveStats`, so an
empty active set disables cold-first selection entirely and the scheduler
behaves exactly as it did before this feature existed. It schedules less well
for one window; it does not schedule on bad data.
Recording it as a known limitation, with the operational note that
scale-in/out should not be triggered immediately after a failover.
--
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]
