github-actions[bot] commented on code in PR #64522:
URL: https://github.com/apache/doris/pull/64522#discussion_r3679929957
##########
regression-test/suites/inverted_index_p0/test_compound_reader_fault_injection.groovy:
##########
@@ -35,7 +35,7 @@ suite("test_compound_reader_fault_injection",
"nonConcurrent") {
DISTRIBUTED BY HASH(`@timestamp`) BUCKETS 1
PROPERTIES (
"replication_allocation" = "tag.location.default: 1",
- "inverted_index_storage_format" = "V1"
+ "inverted_index_storage_format" = "V2"
Review Comment:
This V2 conversion makes the fault injection above a silent no-op.
`construct_DorisCompoundReader_failed` exists only in the V1
`DorisCompoundReader(IndexInput*, int32_t, ...)` constructor used by the V1
branch of `IndexFileReader::_open`; V2 uses the clone/entries constructor,
which has no such debug point. Because the query is only wrapped in
`try/catch`, a normal V2 success also passes the suite. Please keep a
deterministic legacy-V1 fixture for this compatibility test, or add an
equivalent V2 injection point, and express the query as a required-error test
so unreachable injections cannot pass.
##########
regression-test/suites/inverted_index_p0/index_format_v2/test_index_change_format.groovy:
##########
@@ -46,6 +46,5 @@ suite("test_index_change_format", "inverted_index_format_v2"){
}
}
- createAndAlterTable("test_index_change_format_v1", "V1", "V2")
createAndAlterTable("test_index_change_format_v2", "V2", "V1")
Review Comment:
Please make this retained V2→V1 case require the ALTER to fail.
`createAndAlterTable` only checks the message inside `catch`; if the ALTER
unexpectedly succeeds, the helper returns normally and this now-sole active
case still passes. Use the standard `test { sql ...; exception "Property
inverted_index_storage_format is not allowed to change" }` form (or explicitly
fail after a successful ALTER).
##########
regression-test/suites/fault_injection_p0/test_skip_index_compaction_fault_injection.groovy:
##########
@@ -177,12 +156,6 @@ suite("test_skip_index_compaction_fault_injection",
"nonConcurrent") {
has_update_be_config = true
check_config.call("inverted_index_compaction_enable", "true");
- try {
-
GetDebugPoint().enableDebugPointForAllBEs("Compaction::open_inverted_index_file_reader")
- run_test.call(tableName1)
Review Comment:
Please keep this reader-open recovery test on a V2 fixture instead of
deleting it with the V1 fixture. `Compaction::open_inverted_index_file_reader`
is injected after `IndexFileReader::open(index_meta)` without a V1/V2 guard,
and it covers a different error path from the retained writer-open injection.
`run_test` already verifies both that the injected compaction preserves the 11
rowsets and that a later normal compaction recovers. Please run the reader and
writer cases on separate/reset V2 fixtures (or recreate the table between
them), because reusing the already-compacted table for a second `run_test`
would break its initial 11-rowset assertion.
--
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]
