liaoxin01 opened a new pull request, #68675:
URL: https://github.com/apache/doris/pull/68675
### What problem does this PR solve?
Issue Number: None
Related PR: #46570
Problem Summary:
`BetaRowset::get_segment_num_rows()` loads the row count of each segment
through `DorisCallOnce<Status>`. `DorisCallOnce` stores the result of the first
call **whether it succeeded or not**, so if the first load fails, every later
call returns the same failed `Status` without doing any IO.
A `BetaRowset` object lives as long as its tablet version. If the first load
hits a transient IO error (e.g. S3 `SlowDown`/503 while reading the segment
footer), the error is stuck on that rowset until the BE restarts. Compaction on
merge-on-write tablets needs segment rows for rowid conversion
(`BetaRowsetReader::get_segment_iterators`), so it keeps failing on that
rowset. The failure also stops the rowset from ever being compacted away.
We hit this in a cloud cluster. After a burst of S3 503s, compaction on a
few MOW tablets kept failing every ~5s (`min_compaction_failure_interval_ms`)
for hours:
- each failure reported the same file path and the same S3 `request_id`;
- each failed only ~10ms after compaction started, which is too fast for a
real S3 request;
- lowering compaction permits did not help, because the cached error is
returned before any data is read.
Only restarting the BE recovered it.
Queries that go through `ParallelScannerBuilder` call the same function, so
they can hit the stale error too. The first failure can come from a query as
well as from compaction.
Segment-level lazy loading does not have this problem: `SegmentLoader`
checks `Segment::healthy_status()` and reloads an unhealthy cached segment.
Rowset-level segment rows are the only place without such a fallback.
This PR replaces `DorisCallOnce` with a double-checked `std::atomic<bool>`
flag and a mutex:
- The flag is set (release) only after a successful load, so a failed load
is retried by the next caller, and a successful result is still cached.
- Like `DorisCallOnce`, the fast path reads the flag (acquire) without
taking the lock. After loading, `_segments_rows` is never modified, so it is
read without the lock. This matters because `ParallelScannerBuilder` calls this
for every rowset of every query.
- `_segments_rows` is cleared at the start of each attempt, so rows left
over from a failed attempt are never reused.
Rowsets written after #59476 usually take their segment rows from the rowset
meta and don't read footers here, which makes this less likely on master.
Rowsets written by older versions still read footers.
### Release note
Fix compaction and queries repeatedly failing with the same stale error
after a transient IO error (e.g. S3 SlowDown) while loading segment rows of a
rowset.
### 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:
- [ ] No.
- [x] Yes. A failed load of segment rows is no longer cached on the
rowset; the next caller retries it.
- 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]
