You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "matanper (via GitHub)" <gi...@apache.org> on 2026/10/19 12:43:43 UTC
[I] [Bug](cloud) Checker inverted check is skipped for instances with more than one storage vault [doris]
matanper opened a new issue, #69289:
URL: https://github.com/apache/doris/issues/69289
### Search before asking
- [x] I had searched in the [issues](https://github.com/apache/doris/issues?q=is%3Aissue) and found no similar issues.
### Version
master at 61179b65580 (2026-10-19); the code is unchanged since 2.1/3.0.
### What's Wrong?
The cloud checker's inverted check (`enable_inverted_check = true`) is the leak detector. It lists the objects of every storage location and reports those that no longer belong to any rowset, i.e. data the recycler failed to delete. It silently does nothing for any instance that has more than one storage vault:
```cpp
// cloud/src/recycler/checker.cpp
int InstanceChecker::do_inverted_check() {
if (accessor_map_.size() > 1) {
LOG(INFO) << "currently not support inverted check for multi accessor. instance_id="
<< instance_id_;
return 0;
}
```
The checker creates one accessor per storage vault, plus the built-in vault. So as soon as an instance has a second vault, the check never runs for any of its vaults, the log shows only the line above, and `num_inverted_check_failed` stays at 0. We hit this while end-to-end testing #69278 on a cluster with two S3 vaults: the forward check ran every minute, while the inverted check was skipped on every run.
There is also a smaller inconsistency in the same function. In `check_segment_file`, a segment whose rowset no longer exists is reported as a leak (`return 1`) only on a tablet-cache miss. On a cache hit (the previous object belonged to the same tablet), the same situation returns `-1` ("rowset not exists"). That is counted as a check error rather than a leak, so whether a leak is reported depends on listing order.
### What You Expected?
The inverted check runs for every storage vault of an instance and reports each leaked object exactly once.
### How to Reproduce?
1. Cloud mode, recycler with `enable_checker = true` and `enable_inverted_check = true`.
2. Create two S3 storage vaults, a table on each, and load some data.
3. Put an orphan object under one vault, e.g. `<prefix>/data/<tablet_id>/<unknown_rowset_id>_0.dat`.
4. The recycler log shows `currently not support inverted check for multi accessor` on every round, and the orphan is never reported.
### Anything Else?
The guard was added in #33280 ("Support hdfs accessor in Checker") without a stated reason. The loop below it already iterates over all accessors, and nothing in the check seems to depend on a single accessor: objects are matched to rowset meta by tablet id and rowset id only. @plat1ko, was there a specific concern behind it? For example, resources backed by the same bucket and prefix being listed twice, or the listing cost.
Proposed fix (PR to follow):
- remove the guard;
- list each distinct storage location (accessor `uri()`) once, so resources sharing a bucket and prefix are not scanned or counted twice;
- report a leak on the cache-hit path too.
### Are you willing to submit PR?
- [x] Yes I am willing to submit a PR!
### Code of Conduct
- [x] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct)
--
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: commits-unsubscribe@doris.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [I] [Bug](cloud) Checker inverted check is skipped for instances with more than one storage vault [doris]
Posted by "Doris-Breakwater (via GitHub)" <gi...@apache.org>.
Doris-Breakwater commented on issue #69289:
URL: https://github.com/apache/doris/issues/69289#issuecomment-6267762023
Breakwater-GitHub-Analysis-Slot: slot_149dc6a49809
Source review confirms both reported behaviors at `61179b655805b6ec150cbf7ef50144f8eb6f3ad6`. This is a cloud checker coverage and leak-accounting defect; it does not by itself establish why the recycler left any particular object behind. The issue currently has no labels; cloud/recycler-checker is the relevant ownership area.
**Verified findings**
- The guard returns success before listing any accessor and before installing the deferred metric updates. The worker treats that return as successful. Therefore a skipped run supplies no evidence that storage is leak-free, and `num_inverted_check_failed` is not refreshed by that run—it can remain at its default or previous value. See [the guard and metrics](https://github.com/apache/doris/blob/61179b655805b6ec150cbf7ef50144f8eb6f3ad6/cloud/src/recycler/checker.cpp#L886-L910) and [the caller](https://github.com/apache/doris/blob/61179b655805b6ec150cbf7ef50144f8eb6f3ad6/cloud/src/recycler/checker.cpp#L251-L262).
- The precise trigger is **more than one entry in `accessor_map_`**, rather than a count of SQL-visible vaults or distinct physical locations. Initialization inserts legacy `instance.obj_info()` resources and configured storage-vault resources, keyed by resource ID. Multiple IDs pointing to one location also trigger the guard. A separate built-in accessor is not unconditionally created. See [initialization](https://github.com/apache/doris/blob/61179b655805b6ec150cbf7ef50144f8eb6f3ad6/cloud/src/recycler/checker.cpp#L533-L630).
- For a successfully populated tablet cache, an absent rowset returns `-1` on a cache hit and `1` on a miss. Only `1` increments `num_file_leak`. Consecutive orphan segments of one tablet can therefore be undercounted, and a valid segment preceding an orphan can leave the leak count at zero while returning an error. See [cache handling](https://github.com/apache/doris/blob/61179b655805b6ec150cbf7ef50144f8eb6f3ad6/cloud/src/recycler/checker.cpp#L949-L1003) and [aggregation](https://github.com/apache/doris/blob/61179b655805b6ec150cbf7ef50144f8eb6f3ad6/cloud/src/recycler/checker.cpp#L1052-L1092).
**Historical context and patch considerations**
[PR #33280](https://github.com/apache/doris/pull/33280/files) did add the guard. Its patch also explicitly skipped HDFS accessors, with a FIXME about excessive listing memory for large tablet counts. That establishes a historical HDFS listing concern, but does **not** establish the reason for the multi-accessor guard. The current [HDFS iterator](https://github.com/apache/doris/blob/61179b655805b6ec150cbf7ef50144f8eb6f3ad6/cloud/src/recycler/hdfs_accessor.cpp#L229-L325) traverses recursively and retains directory-entry lists, so HDFS scale deserves validation when enabling broader scans. I found no directly matching historical resolution establishing a different root cause or a ready-made fix.
Removing the guard, deduplicating locations, and fixing the cache-hit result is a reasonable direction, with these review points:
- Keep resource IDs intact for metadata lookups; deduplicate only the listing pass. [S3 `uri()`](https://github.com/apache/doris/blob/61179b655805b6ec150cbf7ef50144f8eb6f3ad6/cloud/src/recycler/s3_accessor.cpp#L424-L430) includes endpoint, bucket and prefix. Exact URI equality handles identical configured locations; it does not prove that differently named endpoints refer to different storage. Do not deduplicate by bucket/prefix alone across different endpoints.
- Preserve genuine metadata/listing errors as errors. The tablet cache currently sets its tablet ID before the metadata read completes; a transaction/range-read/parse failure can leave an empty or partial cache. Publish the cache only after a complete successful read, or invalidate it on failure, so changing the hit path to `return 1` cannot turn a failed read into a leak report on the next object.
- Coordinate with [#69278](https://github.com/apache/doris/pull/69278): this revision's inverted parser assumes the version-0 key layout. Removing the guard alone does not add support for sharded version-1 paths. Packed files also have a dedicated checker and are deliberately excluded here.
**Suggested validation and remaining evidence**
Add deterministic tests that execute the actual listing loop: two distinct locations with an orphan in each; two resource IDs with the same URI and a single scan/count; valid-then-orphan and consecutive orphan segments of the same tablet in both listing orders; metadata-read and listing failures. Assert scan/leak counters as well as the return value—`ASSERT_NE(..., 0)` alone cannot distinguish a leak from an error. The existing [segment tests](https://github.com/apache/doris/blob/61179b655805b6ec150cbf7ef50144f8eb6f3ad6/cloud/test/recycler_test.cpp#L5166-L5271) include a normal test that returns through a sync point before listing and a disabled abnormal test, so they do not establish this behavior. Include mixed S3/HDFS and mixed path-format coverage where supported.
No further logs are needed to establish these source-level defects. For end-to-end acceptance, please attach the exact tested commits (including any #69278 changes), sanitized resource-ID-to-location mapping and path formats, the injected object keys and listing order, and one full checker round with scan/leak counters. Run against quiescent test data after applicable recycler retention has elapsed; this function checks visible rowset metadata, so absence there alone does not prove that a newly written or pending-recycle object is an actual recycler failure. Confirm older release branches separately before selecting backports; I have verified the cited master revision, not the claim that every 2.1/3.0 branch is unchanged.
Validation performed: source and public PR-history inspection; no Doris build, unit-test execution, or cluster reproduction. No repository code was changed.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org