deardeng opened a new pull request, #69164: URL: https://github.com/apache/doris/pull/69164
pick form https://github.com/apache/doris/pull/67636 Problem Summary: A CloudReplica keeps one route entry per compute group. When a compute group is dropped, the master sweeps the stale entries out of its own heap in CloudTabletRebalancer, but that sweep writes no edit log. A follower only replays OP_UPDATE_CLOUD_REPLICA, which adds or overwrites the route of the compute group named in the log and never removes anything, so on a follower the route maps only ever grow. With a fixed set of replicas and repeated compute group create/drop rounds, the follower's route entries grow linearly with the number of rounds while the master stays flat. The generated checkpoint image has a related but separate gap. The checkpoint Env does loadImage -> replayJournal -> post-processing -> saveImage. gsonPostProcess() cleans what loadImage deserializes, but nothing cleans what the replay window then adds, and postProcessCloudMetadata() only copies tablet stats. So every image is written with the stale routes accumulated since the previous image. An earlier version of this PR fixed both by calling CloudReplica.removeInvalidRoutes() once per replayed UpdateCloudReplicaInfo journal entry, plus once before saving a checkpoint image. That works, but on a large catalog (tens of millions of tablets, many compute groups) it means every single route-update journal entry re-scans that replica's whole route map, which adds up across a replay batch and slows down replay itself -- the thing route cleanup should never be on the critical path for. This revision keeps the checkpoint-time sweep but replaces the per-journal-entry replay cleanup with a coalesced sweep over the active catalog, run at most once per replay batch, and adds an equivalent sweep to the master's rebalancer round (previously that sweep was piggybacked on the rebalancer's per-compute-group replica loop and had no way to run when the cluster's live compute-group set had just gone empty). It also fixes a correctness gap in the original per-journal-entry design: a journal written before a backend was dropped can be replayed after the drop, and the original code published that route unconditionally before the next cleanup pass could catch it, which could momentarily discard an already-valid secondary route for the same compute group. Three independent sweep triggers, all reusing CloudInternalCatalog.removeInvalidCloudReplicaRoutes(SystemInfoService) -- an instance method that walks this catalog's own db/table/partition/index(ALL, including shadow indexes)/tablet/replica tree and calls CloudReplica.removeInvalidRoutes(systemInfo) per replica: 1. CloudEnv.replayJournal() calls it once per replay batch, gated on CloudSystemInfoService.replayBackendRemovalVersion having changed since the last sweep (a counter bumped only by replayDropBackend()). Additions alone never strand a route, so they don't trigger a sweep. The very first replay after startup always sweeps once regardless of the counter. A failed sweep is retried on the next batch rather than being silently dropped. The checkpoint thread's own replayJournal() call skips this path entirely (Checkpoint.doCheckpoint() sweeps explicitly after replay finishes, see 3 below). 2. CloudTabletRebalancer.completeRouteInfo() sweeps once per round when the live backend set has shrunk since the last sweep, independent of the per-compute-group replica loop (the old code piggybacked the sweep on that loop by pinning it to one arbitrary compute group, which meant a topology with no live compute groups could never trigger a sweep at all). It runs for two consecutive rounds after a drop is detected, not one: a query thread can pick a backend in hashReplicaToBe() just before the drop and publish the route in getBackendIdImpl() after this round's pass has already visited that replica, so the extra round catches routes published while the first one was still in flight. This sweep can now also be confined to a configurable daily time window (cloud_tablet_rebalancer_stale_route_clean_start_time / cloud_tablet_rebalancer_stale_route_clean_end_time, HH:mm, both default to "00:00" which is unrestricted); outside the window the pending-sweep-round counter is left untouched rather than drained, so a drop detected outside the window still gets its two rounds once the window opens. 3. Checkpoint.doCheckpoint() still sweeps the checkpoint Env's own catalog against its own backend set after replay finishes and before saveImage(), now via an explicit SystemInfoService parameter (env.getClusterInfo()) threaded down through the whole call chain instead of being resolved internally via Env.getCurrentSystemInfo()/Env.getCurrentEnv(). This removes the implicit dependency on the checkpoint thread's identity to resolve the right Env, and keeps the catalog being swept and the backend set deciding staleness from ever being able to come from two different Envs. Late-journal protection: CloudReplica.replayUpdateClusterToPrimaryBe(clusterId, beId, systemInfo) replaces the old unconditional updateClusterToPrimaryBe() + removeInvalidRoutes() pair in CloudInternalCatalog.unprotectUpdateCloudReplica(). Before publishing a replayed primary-route update, it checks whether the target backend is still registered; if not, the update is dropped instead of being published and then possibly torn down by the next sweep, which previously had a window where it could also clear an already-valid secondary route for the same compute group before the primary write was reconciled. FrontendServiceImpl.getTabletReplicaInfos()'s lazy peer-discovery path now collects secondary backends via CloudReplica.getAllSecondaryBes() (which scans secondaryClusterToBackends directly) instead of only looking up a secondary for compute groups that still have a primary key present. A secondary-only route (primary key already swept, secondary still valid) was previously invisible to this path. The journal protocol is unchanged: no new opcode, no new field, no delete event, and no change to the meaning of existing logs. Existing cleanup semantics are reused as-is: enable_cloud_replica_stale_route_clean still gates every sweep call site, staleness is still decided by whether the backend is registered in the relevant Env (not by isAlive() or heartbeat), a dead primary whose compute group still has a live secondary is still kept for the failover and peer cache paths, and removal is still the conditional remove(key, expectedValue). Boundaries, deliberately not addressed here: - Cleanup remains a synchronous full-catalog scan at each trigger point; budgeted/incremental scheduling is deferred to a follow-up PR and should be based on measured replay latency at production tablet-count scale, not assumed here. - The rebalancer's sweep only runs on the master; on a Follower, only the replay-batch sweep and image-load's gsonPostProcess() reclaim routes. That asymmetry is unchanged from before this PR. - CatalogRecycleBin is intentionally out of scope: this PR only sweeps the active catalog. A recycled table or partition's routes are reclaimed through the normal recycle-bin lifecycle (erase, or restored and picked up by a later active-catalog sweep after RECOVER), not by this sweep. - A backend id that still exists but now belongs to a different compute group is not detected, as before. - run-fe-ut.sh across CloudEnvRouteCleanupTest, CloudReplicaTest, CloudTabletRebalancerTest, CloudInternalCatalogTest, CheckpointTest, FrontendServiceImplCloudTest, CatalogRecycleBinTest and RecoverTest: 106 tests, 0 failures, 0 errors. FE build with checkstyle passes with 0 violations (fe-common and fe-core). - CloudEnvRouteCleanupTest (new): coalescing multiple backend-removal journals into one sweep per batch, sweeping once on startup and once immediately after re-enabling the config, retrying a sweep that threw, and skipping the sweep entirely on the checkpoint thread. - CloudInternalCatalogTest: replaying a route update no longer scans or removes other routes itself (that responsibility moved to the batch sweep); a late journal to an already-dropped backend does not clear an existing valid secondary route. - CheckpointTest: stale route dropped while a live route is kept; a dead primary with a live secondary is kept; shadow-index and temporary-partition replicas are swept; a recycled table/partition's replicas are not swept and CatalogRecycleBin is never touched; cleanup disabled is a no-op; a new case asserts the sweep judges backend liveness against the checkpoint Env's own backend set, not the serving Env's, by giving the two a different opinion on the same backend id and checking which one wins. - CloudTabletRebalancerTest: the sweep now runs independent of the per-compute-group loop, including when the live compute-group set is empty; the two-round-after-a-drop behavior; the new HH:mm time window, including the midnight-wrapping case and that pending sweep rounds are preserved (not drained) while outside the window. - FrontendServiceImplCloudTest (new case): a secondary-only route (no primary key present for that compute group) is still returned by the lazy peer-discovery path. - Not covered by tests: no multi-FE cluster regression, and no large-scale (tens-of-millions-of-tablets) performance run reproducing the original replay-latency numbers. Coalesce stale CloudReplica route cleanup to run once per replay batch (and once per rebalancer round, now with an optional daily time window) instead of on every route-update journal entry, and stop a late journal from clearing an already-valid secondary route. ### What problem does this PR solve? Issue Number: close #xxx Related PR: #xxx Problem Summary: ### Release note None ### Check List (For Author) - Test <!-- At least one of them must be included. --> - [ ] Regression test - [ ] Unit Test - [ ] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - [ ] This is a refactor/code format and no logic has been changed. - [ ] Previous test can cover this change. - [ ] No code files have been changed. - [ ] Other reason <!-- Add your reason? --> - Behavior changed: - [ ] No. - [ ] Yes. <!-- Explain the behavior change --> - Does this need documentation? - [ ] No. - [ ] Yes. <!-- Add document PR link here. eg: https://github.com/apache/doris-website/pull/1214 --> ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into --> -- 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]
