Skip to content

[fix](cloud) Recycle empty rowsets without resource id - #64630

Merged
gavinchou merged 1 commit into
apache:masterfrom
liaoxin01:fix-recycler-empty-resource-id
Jun 18, 2026
Merged

[fix](cloud) Recycle empty rowsets without resource id#64630
gavinchou merged 1 commit into
apache:masterfrom
liaoxin01:fix-recycler-empty-resource-id

Conversation

@liaoxin01

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: None

Problem Summary:

When recycling a dropped tablet, empty rowsets may have num_segments == 0 and an empty resource_id. The protobuf field can be explicitly set to an empty string, so has_resource_id() returns true. The recycler then attempts to look up an accessor for the empty ID, fails the recycle round, and repeatedly leaves the dropped tablet/index metadata behind.

This PR skips object deletion for every rowset with zero segments because it has no segment objects to remove. Rowsets with segments still require a non-empty resource ID.

Release note

Fix cloud recycler failing to remove dropped tablet metadata when an empty rowset has no resource ID.

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test
    • No need to test or manual test.
  • Behavior changed:

    • No.
    • Yes. Empty rowsets are skipped during object deletion because they have no segment objects; non-empty rowsets still require a valid resource ID.
  • 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

CopilotAI review requested due to automatic review settings June 18, 2026 08:07
@hello-stephen

Copy link
Copy Markdown
Contributor

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:

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

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a cloud recycler failure mode where dropped tablet recycling can get stuck if a rowset protobuf has num_segments == 0 and resource_id is present but empty (so has_resource_id() is true). The recycler now skips object deletion for empty rowsets (since there are no segment objects to remove) and still enforces a non-empty resource ID for non-empty rowsets.

Changes:

  • Skip rowsets with num_segments == 0 during dropped tablet object deletion to avoid accessor lookup on empty resource IDs.
  • Tighten the resource ID validation for non-empty rowsets by checking resource_id().empty() (covers both “unset” and “set-to-empty” cases).
  • Add a unit test covering the explicitly-empty resource_id + zero-segment rowset scenario.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

FileDescription
cloud/src/recycler/recycler.cppSkip empty rowsets before accessor lookup; require non-empty resource_id for non-empty rowsets.
cloud/test/recycler_test.cppAdd unit test validating recycling succeeds when rowset has 0 segments and empty resource_id.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadcloud/src/recycler/recycler.cpp Outdated
Comment threadcloud/src/recycler/recycler.cpp Outdated
@liaoxin01
liaoxin01force-pushed the fix-recycler-empty-resource-id branch from 9937bdc to 6381178CompareJune 18, 2026 08:13
@liaoxin01

Copy link
Copy Markdown
ContributorAuthor

/review

@liaoxin01

Copy link
Copy Markdown
ContributorAuthor

run buildall

@wyxxxcatwyxxxcat left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

github-actions[bot]
github-actionsBot previously requested changes Jun 18, 2026

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found one blocking gap in the fix.

Critical checkpoints:

  • Goal/test: The normal internal_get_rowset path is fixed and covered by a unit test, but the same empty-rowset shape can still reach recycle_tablet via restore-job rowset KVs, so the dropped-tablet cleanup goal is incomplete.
  • Scope/focus: The change is small, but a functionally parallel path in the same function was not updated.
  • Concurrency/lifecycle: No new concurrency or lifetime management is introduced. The failure is retry/lifecycle related: restore-job recycle rounds can repeatedly fail before KV removal.
  • Config/compatibility/protocol: No config, format, or protocol change.
  • Transaction/persistence: Normal empty-rowset KV cleanup proceeds after object deletion; restore-job rowset KVs can still be left behind when the restore loop returns -1.
  • Data/object deletion: Skipping zero-segment normal rowsets is consistent with the lack of segment objects; packed-file ordering concerns were reviewed and I did not find a new issue for this path.
  • Tests: Added coverage exercises only the normal rowset response injection; restore-job rowset coverage is missing. I did not run unit tests; git diff --check for the two PR files passed.
  • User focus: No additional user-provided review focus.

Comment threadcloud/test/recycler_test.cpp
@github-actions

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@hello-stephen

Copy link
Copy Markdown
Contributor

Cloud UT Coverage Report

Increment line coverage 100.00% (6/6) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage77.32% (1889/2443)
Line Coverage64.38% (33944/52726)
Region Coverage64.77% (17454/26948)
Branch Coverage53.93% (9339/17316)

@gavinchou
gavinchou merged commit 6a268dc into apache:masterJun 18, 2026
38 checks passed
github-actionsBot pushed a commit that referenced this pull request Jun 18, 2026
When recycling a dropped tablet, empty rowsets may have `num_segments ==
0` and an empty `resource_id`. The protobuf field can be explicitly set
to an empty string, so `has_resource_id()` returns true. The recycler
then attempts to look up an accessor for the empty ID, fails the recycle
round, and repeatedly leaves the dropped tablet/index metadata behind.
This PR skips object deletion for every rowset with zero segments
because it has no segment objects to remove. Rowsets with segments still
require a non-empty resource ID.
github-actionsBot pushed a commit that referenced this pull request Jun 18, 2026
When recycling a dropped tablet, empty rowsets may have `num_segments ==
0` and an empty `resource_id`. The protobuf field can be explicitly set
to an empty string, so `has_resource_id()` returns true. The recycler
then attempts to look up an accessor for the empty ID, fails the recycle
round, and repeatedly leaves the dropped tablet/index metadata behind.
This PR skips object deletion for every rowset with zero segments
because it has no segment objects to remove. Rowsets with segments still
require a non-empty resource ID.
yiguolei pushed a commit that referenced this pull request Jun 20, 2026
…4630 (#64643)
Cherry-picked from #64630
Co-authored-by: Xin Liao <liaoxin@selectdb.com>
morningman pushed a commit that referenced this pull request Jun 24, 2026
…4630 (#64642)
Cherry-picked from #64630
Co-authored-by: Xin Liao <liaoxin@selectdb.com>
@yiguoleiyiguolei mentioned this pull request Jul 7, 2026
@morningmanmorningman mentioned this pull request Jul 9, 2026
liaoxin01 pushed a commit that referenced this pull request Jul 22, 2026
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #64630
Problem Summary: When a dropped tablet contains only empty rowsets, the
recycler
skips every rowset before collecting its resource ID. As a result,
resource_ids remains empty and no tablet directory deletion is
scheduled.
Metadata is then removed and the tablet is marked as recycled, leaving
temporary objects from failed writes orphaned in object storage.
Allow empty rowsets with a valid resource ID to participate in
tablet-level
directory cleanup, while continuing to skip empty rowsets without a
resource
ID and reject non-empty rowsets with a missing resource ID.
github-actionsBot pushed a commit that referenced this pull request Jul 22, 2026
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #64630
Problem Summary: When a dropped tablet contains only empty rowsets, the
recycler
skips every rowset before collecting its resource ID. As a result,
resource_ids remains empty and no tablet directory deletion is
scheduled.
Metadata is then removed and the tablet is marked as recycled, leaving
temporary objects from failed writes orphaned in object storage.
Allow empty rowsets with a valid resource ID to participate in
tablet-level
directory cleanup, while continuing to skip empty rowsets without a
resource
ID and reject non-empty rowsets with a missing resource ID.
github-actionsBot pushed a commit that referenced this pull request Jul 22, 2026
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #64630
Problem Summary: When a dropped tablet contains only empty rowsets, the
recycler
skips every rowset before collecting its resource ID. As a result,
resource_ids remains empty and no tablet directory deletion is
scheduled.
Metadata is then removed and the tablet is marked as recycled, leaving
temporary objects from failed writes orphaned in object storage.
Allow empty rowsets with a valid resource ID to participate in
tablet-level
directory cleanup, while continuing to skip empty rowsets without a
resource
ID and reject non-empty rowsets with a missing resource ID.
0AyanamiRei pushed a commit to 0AyanamiRei/doris that referenced this pull request Jul 27, 2026
…he#65862)
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: apache#64630
Problem Summary: When a dropped tablet contains only empty rowsets, the
recycler
skips every rowset before collecting its resource ID. As a result,
resource_ids remains empty and no tablet directory deletion is
scheduled.
Metadata is then removed and the tablet is marked as recycled, leaving
temporary objects from failed writes orphaned in object storage.
Allow empty rowsets with a valid resource ID to participate in
tablet-level
directory cleanup, while continuing to skip empty rowsets without a
resource
ID and reject non-empty rowsets with a missing resource ID.
wyxxxcat added a commit to wyxxxcat/doris that referenced this pull request Aug 17, 2026
…he#65862)
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: apache#64630
Problem Summary: When a dropped tablet contains only empty rowsets, the
recycler
skips every rowset before collecting its resource ID. As a result,
resource_ids remains empty and no tablet directory deletion is
scheduled.
Metadata is then removed and the tablet is marked as recycled, leaving
temporary objects from failed writes orphaned in object storage.
Allow empty rowsets with a valid resource ID to participate in
tablet-level
directory cleanup, while continuing to skip empty rowsets without a
resource
ID and reject non-empty rowsets with a missing resource ID.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@liaoxin01@hello-stephen@gavinchou@wyxxxcat@morningman@yiguolei