[fix](cloud) Retain instance tombstones after recycle cleanup by wyxxxcat · Pull Request #66870 · apache/doris

wyxxxcat · 2026-08-18T03:27:20Z

What problem does this PR solve?

Issue Number: close #xxx

Related PR: #66519

Problem Summary:
The recycler previously removed the instance key after the recycle state reached INSTANCE_RECYCLE_STATE_CLEANUP_COMPLETED. This made the final recycle state unavailable and prevented successor-chain checks from distinguishing a completed successor from a missing instance. This change retains the deleted instance record as a tombstone, skips completed tombstones during scanning, excludes deleted instances from ResourceManager runtime indexes and caches, and allows predecessors to continue once their successor has completed cleanup.

Release note

None

Check List (For Author)

Test
- 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
Behavior changed:
- No.
- Yes.
Does this need documentation?
- No.
- Yes.

Check List (For Reviewer who merge this PR)

Confirm the release note
Confirm test cases
Confirm document
Add branch pick label

hello-stephen · 2026-08-18T03:27:40Z

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

What problem was fixed (it's best to include specific error reporting information). How it was fixed.
Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
What features were added. Why was this function added?
Which code was refactored and why was this part of the code refactored?
Which functions were optimized and what is the difference before and after the optimization?

wyxxxcat · 2026-08-18T03:35:19Z

/review

github-actions

This PR makes the deleted-instance metadata cleanup and terminal recycle-state transition atomic, preserves forward-only successor gating, and adds useful direct unit coverage. However, I found three blocking lifecycle and compatibility issues and requested changes inline:

Completed records remain in every active-instance range scan forever; ignored scan failures can leave later live or deleting instances unscheduled.
Any terminal marker retained under the legacy instance key is unsafe for pre-upgrade meta-servers: old cache misses accept key existence and can recreate metadata, while full predecessor records also enable stale cache hits.
Several mutating instance operations can now rewrite a DELETED/CLEANUP_COMPLETED record indefinitely.

Critical checkpoint conclusions:

Goal and tests: Partially achieved. Retained completion state, immediate-successor gating, missing-successor compatibility, current-code cache clearing, and scanner queue filtering have direct tests; scale/partial-scan behavior, base-reader/current-writer restart, concurrent multi-hop chains, and post-terminal mutation are not covered.
Scope and parallel paths: The code change is focused, but the lifecycle change was not propagated to all full-instance scanners or to a shared terminal-state mutation guard.
Concurrency and lifecycle: Recycler queue/map locking and ResourceManager locking are sound. Recycle states are monotonic, and metadata deletion plus CLEANUP_COMPLETED commit atomically. The unresolved lifecycle problem is permanent records in the legacy active keyspace and mutable APIs.
Configuration and compatibility: No new configuration is introduced. The existing watcher is asynchronous and disableable, so it cannot serve as a rolling-upgrade compatibility fence for older binaries.
Persistence and data correctness: Successor ordering and retry/restart behavior are safe under the persisted chain invariant. Retaining the completion marker under the legacy instance key lets old writers recreate metadata after cleanup; retaining the full predecessor payload also enables stale cluster cache hits and terminal mutations.
Performance and observability: Periodic reads, deserialization, allocation, and INFO logging now scale with all historically deleted tenants; partial full-range errors are ignored by scanner callers.
Protocol and storage format: No FE/BE variable, RPC schema, or durable data-format change is introduced beyond the instance-record lifecycle.
Test execution: Per the review-run contract, I did not build or run tests; the added tests were reviewed statically.
User focus: No additional user-provided focus was supplied.

wyxxxcat · 2026-08-18T08:49:06Z

run buildall

wyxxxcat · 2026-08-18T13:40:32Z

run buildall

hello-stephen · 2026-08-18T15:08:42Z

Cloud UT Coverage Report

Increment line coverage 40.19% (43/107) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	77.56% (2018/2602)
Line Coverage	64.98% (36895/56782)
Region Coverage	65.56% (18758/28613)
Branch Coverage	55.18% (10219/18520)

gavinchou · 2026-08-19T01:08:19Z

 << ", code=" << format_as(c0) << ", info=" + m0;
+ return false;
 }
- return c0 == TxnErrorCode::TXN_OK;


this is a bug... if c0 is not OK, we should not return false
return false IFFc0 == NOT_FOUND

wyxxxcat · 2026-08-19T03:25:14Z

run buildall

wyxxxcat · 2026-08-19T07:26:04Z

run buildall

hello-stephen · 2026-08-19T08:56:33Z

Cloud UT Coverage Report

Increment line coverage 43.68% (38/87) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	77.56% (2018/2602)
Line Coverage	65.04% (36924/56767)
Region Coverage	65.57% (18763/28614)
Branch Coverage	55.23% (10231/18524)

wyxxxcat · 2026-08-20T01:06:37Z

run buildall

hello-stephen · 2026-08-20T03:04:02Z

Cloud UT Coverage Report

Increment line coverage 43.68% (38/87) 🎉

Increment coverage report
Complete coverage report

Category	Coverage
Function Coverage	77.01% (2010/2610)
Line Coverage	64.86% (36644/56498)
Region Coverage	51.97% (33940/65312)
Branch Coverage	55.09% (10842/19682)

a

github-actions · 2026-08-20T09:47:27Z

PR approved by anyone and no changes requested.

github-actions · 2026-08-20T09:47:30Z

PR approved by at least one committer and no changes requested.

Related PR: #66519 Problem Summary: The recycler previously removed the instance key after the recycle state reached INSTANCE_RECYCLE_STATE_CLEANUP_COMPLETED. This made the final recycle state unavailable and prevented successor-chain checks from distinguishing a completed successor from a missing instance. This change retains the deleted instance record as a tombstone, skips completed tombstones during scanning, excludes deleted instances from ResourceManager runtime indexes and caches, and allows predecessors to continue once their successor has completed cleanup.

…anup #66870 (#66996) Cherry-picked from #66870 Co-authored-by: Yixuan Wang <wangyixuan@selectdb.com>

wyxxxcat requested review from deardeng, gavinchou, liaoxin01 and luwei16 as code owners August 18, 2026 03:27

wyxxxcat added the dev/4.1.x label Aug 18, 2026

wyxxxcat force-pushed the save_instance_key branch from df66e7d to 8ae3c7cCompare August 18, 2026 03:32

github-actionsBot previously requested changes Aug 18, 2026
View reviewed changes

Comment threadcloud/src/recycler/recycler.cpp Outdated
Comment threadcloud/src/resource-manager/resource_manager.cpp
Comment threadcloud/src/recycler/recycler.cpp Outdated

wyxxxcat force-pushed the save_instance_key branch from 8ae3c7c to bbae92aCompare August 18, 2026 13:40

gavinchou requested changes Aug 19, 2026
View reviewed changes

wyxxxcat force-pushed the save_instance_key branch 8 times, most recently from 3a0b8c1 to 477c111Compare August 19, 2026 03:25

wyxxxcat force-pushed the save_instance_key branch 2 times, most recently from e0ef181 to 2ab5a46Compare August 19, 2026 07:25

1
844216b

wyxxxcat force-pushed the save_instance_key branch from 2ab5a46 to 844216bCompare August 20, 2026 01:06

gavinchou approved these changes Aug 20, 2026
View reviewed changes

gavinchou merged commit 9dbb0e9 into apache:masterAug 20, 2026
31 of 32 checks passed

github-actionsBot added the reviewed label Aug 20, 2026

github-actionsBot added the approved Indicates a PR has been approved by one committer. label Aug 20, 2026

github-actionsBot mentioned this pull request Aug 20, 2026
branch-4.1: [fix](cloud) Retain instance tombstones after recycle cleanup #66870 #66996
Merged

yiguolei pushed a commit that referenced this pull request Aug 21, 2026
branch-4.1: [fix](cloud) Retain instance tombstones after recycle cle…
e475546
…anup #66870 (#66996) Cherry-picked from #66870 Co-authored-by: Yixuan Wang <wangyixuan@selectdb.com>

yiguolei added dev/4.1.4-merged and removed dev/4.1.x labels Aug 21, 2026

yiguolei mentioned this pull request Sep 1, 2026
4.1.4 Release Note #67355
Open

Uh oh!

Conversation

wyxxxcat commented Aug 18, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

Release note

Check List (For Author)

Check List (For Reviewer who merge this PR)

Uh oh!

hello-stephen commented Aug 18, 2026

Uh oh!

wyxxxcat commented Aug 18, 2026

Uh oh!

github-actionsBot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wyxxxcat commented Aug 18, 2026

Uh oh!

wyxxxcat commented Aug 18, 2026

Uh oh!

hello-stephen commented Aug 18, 2026

Cloud UT Coverage Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gavinchouAug 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wyxxxcat commented Aug 19, 2026

Uh oh!

wyxxxcat commented Aug 19, 2026

Uh oh!

hello-stephen commented Aug 19, 2026

Cloud UT Coverage Report

Uh oh!

wyxxxcat commented Aug 20, 2026

Uh oh!

hello-stephen commented Aug 20, 2026

Cloud UT Coverage Report

Uh oh!

Uh oh!

github-actionsBot commented Aug 20, 2026

Uh oh!

github-actionsBot commented Aug 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

wyxxxcat commented Aug 18, 2026•
edited
Loading