deardeng opened a new pull request, #67636: URL: 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. This PR closes both, reusing the existing CloudReplica.removeInvalidRoutes() in two places: 1. CloudInternalCatalog.unprotectUpdateCloudReplica() calls it right after each updateClusterToPrimaryBe(), covering the single-replica branch and the batch branch (both the explicit replicaIds shape and the empty-replicaIds shape). The cleanup scans the replica's whole route map, so replaying an update for one compute group also reclaims entries left behind by groups dropped earlier. 2. Checkpoint.doCheckpoint() sweeps every CloudReplica in the checkpoint Env after post-processing and before saveImage(). At that point the journal is fully replayed and Env.getCurrentSystemInfo() resolves to the checkpoint Env's own backend set, so the sweep is consistent with the image being written and needs no coordination with the serving Env. The Env is private to the checkpoint thread, so no locking is needed, the same as postProcessCloudMetadata(). The sweep walks IndexExtState.ALL, not VISIBLE: shadow indexes hold routes too. It deliberately does not piggyback on postProcessCloudMetadata()'s traversal, which returns early when the serving Env is missing and skips any object absent from it; route cleanup must not inherit those skips. The journal protocol is unchanged: no new opcode, no new field, no delete event, and no change to the meaning of existing logs. No new thread, timer, HTTP endpoint or config is introduced. Existing cleanup semantics are reused as-is: enable_cloud_replica_stale_route_clean still gates both call sites, staleness is still decided by whether the backend is registered in the current 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: - Replay-time cleanup is triggered by an update, so it only visits replicas that a replay touches. Entries stranded after the last route log that covers an index stay in a running follower's heap until that index is updated again, the FE restarts (image load runs gsonPostProcess), or the follower is promoted and runs a rebalancer sweep. How much is left over depends on each index's update and drop ordering; it is not bounded to one compute group. - The master's sweep is triggered by a backend disappearing and covers every replica; the follower-side replay cleanup is triggered by a route log and covers one index at a time. That asymmetry remains. - A backend id that still exists but now belongs to a different compute group is not detected, as before. ### Test - Four tests in CloudInternalCatalogTest, all driven through replayUpdateCloudReplica(): single-replica replay dropping a stale group while keeping the live one; batch replay over two replicas in both log shapes; cleanup disabled leaving the stale entry in place; and a replayed backend that is already gone, which must not survive while a dead primary with a live secondary is kept. - Seven tests in the new CheckpointTest for the pre-save sweep: stale route dropped and live route kept, plus idempotence on a second pass; dead primary with a live secondary kept; non-cloud mode short-circuits without walking the catalog; non-managed table skipped; shadow index replicas swept; non-CloudReplica skipped; cleanup disabled is a no-op. - run-fe-ut.sh on CheckpointTest, CloudInternalCatalogTest and CloudReplicaTest: 32 tests, 0 failures. FE build with checkstyle passes with no violations. - Mutation checks: reverting the two replay calls fails 3 of the 4 replay tests; changing the sweep from IndexExtState.ALL to VISIBLE fails 3 of the 7 checkpoint tests. The tests that still pass in each case are the disabled-switch ones, which are expected to pass either way. - Not covered by tests: that doCheckpoint() actually calls the sweep before saveImage(). The tests exercise the sweep method directly; driving a real checkpoint needs an EditLog, journal and image directories, which is out of proportion here. - No cluster-scale verification was run. The journal lag, heap object count and image size numbers from the original report are not reproduced here, and image size in particular was not measured. ### Release note Reclaim route entries of dropped compute groups when replaying cloud replica route updates and before writing checkpoint images. Claude-Session: https://claude.ai/code/session_01KS8nURA47VZ1t3LAcsfGRB ### 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 - [x] 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: - [x] No. - [ ] Yes. <!-- Explain the behavior change --> - Does this need documentation? - [x] 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]
