Doris-Breakwater commented on issue #70405:
URL: https://github.com/apache/doris/issues/70405#issuecomment-6736831059

   Breakwater-GitHub-Analysis-Slot: slot_5174365e680b
   
   **Assessment:** The reported error-preservation defect is confirmed by 
source inspection in both 4.1.4 and the cited master commit `14fde1c1dc0`. The 
proposed best-effort diagnostic listing is an appropriate narrowly scoped fix. 
Suggested maintainer area: BE / inverted index / CLucene; the issue currently 
has no labels or assignee.
   
   **Verified behavior**
   
   - In both 
[4.1.4](https://github.com/apache/doris/blob/4accb51bc1f27d84980668f3a2eaf0ac96fb3cba/be/src/storage/index/inverted/inverted_index_searcher.cpp#L30-L46)
 and [the cited master 
commit](https://github.com/apache/doris/blob/14fde1c1dc096f09c17af019c5e0cff3175da6c9/be/src/storage/index/inverted/clucene/inverted_index_searcher.cpp#L30-L46),
 the diagnostic `directory->list()` executes inside the handler without its own 
exception boundary. If it throws, execution bypasses construction of the 
original message and both intended `Status` returns. `get_index_searcher()` 
calls `build()` directly and does not catch that second exception. Strictly, 
the builder throws the listing error; it does not return a listing-error 
`Status` itself.
   - This builder serves CLucene `FULLTEXT` and `STRING_TYPE` readers. The BKD 
builder has a separate handler. Preserving the corruption classification can 
affect behavior: the cited master's [compound-predicate fallback 
check](https://github.com/apache/doris/blob/14fde1c1dc096f09c17af019c5e0cff3175da6c9/be/src/exprs/vcompound_pred.h#L47-L53)
 accepts `INVERTED_INDEX_FILE_CORRUPTED`, but does not accept 
`INVERTED_INDEX_CLUCENE_ERROR`. This establishes that the distinction matters, 
without proving a particular query's outcome.
   - A listing exception is possible: 
[DorisFSDirectory::list](https://github.com/apache/doris/blob/14fde1c1dc096f09c17af019c5e0cff3175da6c9/be/src/storage/index/index_fs_directory.cpp#L531-L549)
 converts a failed filesystem listing to a CLucene I/O exception. However, 
[DorisCompoundReader::list](https://github.com/apache/doris/blob/14fde1c1dc096f09c17af019c5e0cff3175da6c9/be/src/storage/index/index_compound_reader.cpp#L280-L288)
 enumerates in-memory entries and explicitly throws when closed or missing its 
entries. A remote read failure alone therefore does **not** establish that the 
subsequent compound-directory listing also fails.
   
   **Next steps and missing evidence**
   
   1. Please link the proposed PR and unit test, including the focused test 
command and results. Keep the original exception as the source of the returned 
code and message; catch the diagnostic listing's `CLuceneError` separately and 
log it as a secondary failure. This preserves failure reporting; it does not 
repair the underlying index or storage failure.
   2. Cover original `CL_ERR_EmptyIndexSegment` and an ordinary CLucene error, 
each with listing succeeding and throwing. Assert the exact expected `Status` 
code, preservation of the original message, and absence of an escaping listing 
exception. Retain successful-open coverage.
   3. Ensure the test actually fails the **diagnostic** listing. The [pinned 
CLucene 
implementation](https://github.com/apache/doris-thirdparty/blob/08d45e541fef1e48dc4ab00d2d5fde9569b49ee3/src/core/CLucene/index/SegmentInfos.cpp#L881-L949)
 lists during segment discovery and can retry opening. An always-throwing 
`list()` may fail before `openInput()` is reached and would not demonstrate 
preservation of the intended original open error. Use distinct 
original/secondary messages and verify the exercised path.
   4. No production profile is needed to review this localized fix. To 
establish production reachability or query-level impact, supply the exact build 
commit, index storage format/concrete directory type, minimal reproduction or 
fault sequence, and sanitized BE logs/stack showing where opening failed and 
where listing failed. The read fault alone may fail earlier during index-file 
initialization or null-bitmap reading, before this builder is reached.
   
   Validation here was read-only source inspection, including the CLucene 
revision pinned by both versions. No Doris code was changed, and the proposed 
patch/unit test was not available to execute. The exception-masking mechanism 
is verified; its operational frequency and the underlying storage/corruption 
cause remain unverified.
   


-- 
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]

Reply via email to