github-actions[bot] commented on code in PR #67342:
URL: https://github.com/apache/doris/pull/67342#discussion_r3900606874
##########
fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java:
##########
@@ -1283,25 +1283,23 @@ public static TInvertedIndexFileStorageFormat
analyzeInvertedIndexFileStorageFor
invertedIndexFileStorageFormat =
properties.get(PROPERTIES_INVERTED_INDEX_STORAGE_FORMAT);
properties.remove(PROPERTIES_INVERTED_INDEX_STORAGE_FORMAT);
} else {
- if (Config.inverted_index_storage_format.equalsIgnoreCase("V1")) {
- return TInvertedIndexFileStorageFormat.V1;
- } else if
(Config.inverted_index_storage_format.equalsIgnoreCase("V2")) {
+ if (Config.inverted_index_storage_format.equalsIgnoreCase("V2")) {
return TInvertedIndexFileStorageFormat.V2;
} else {
return TInvertedIndexFileStorageFormat.V3;
}
}
if (invertedIndexFileStorageFormat.equalsIgnoreCase("v1")) {
- return TInvertedIndexFileStorageFormat.V1;
+ throw new AnalysisException(
Review Comment:
[P1] Preserve synchronized replay of existing V1 tables
`Env.getSyncedDdlStmt` serializes an existing table's persisted
`inverted_index_storage_format` together with `is_being_synced=true`, but the
destination invokes this analyzer before synchronization-specific handling can
run. An existing V1 table therefore now fails CCR/binlog CREATE replay at this
throw, and synchronized ADD INDEX hits the same compatibility break in
`processAddIndex`'s exact-V1 guard. Please route trusted sync/CCR execution
through a compatibility path that preserves explicit V1 (the SQL property alone
is user-settable and cannot be trusted), and cover replay of an explicit-V1
CREATE and ADD INDEX.
##########
fe/fe-common/src/main/java/org/apache/doris/common/Config.java:
##########
@@ -3264,10 +3264,12 @@ public class Config extends ConfigBase {
@ConfField(mutable = true)
public static boolean fix_tablet_partition_id_eq_0 = false;
- @ConfField(mutable = true, masterOnly = true, description = {
- "倒排索引默认存储格式",
- "Default storage format of inverted index, the default value is
V3."
- })
+ @ConfField(mutable = true, masterOnly = true,
+ callback =
InvertedIndexStorageFormatValidator.RuntimeConfigHandler.class,
Review Comment:
[P1] Keep a rejected V1 update from resetting persisted configs
For direct `_set_config?inverted_index_storage_format=V1&persist=true`, this
callback rejects the only value, leaving `setConfigs` empty, but
`SetConfigAction` still calls `persistConfig(setConfigs, true)`. That reset
path truncates `fe_custom.conf`, so a failed V1 update can erase every
unrelated persisted override (and may expose a V1 value from `fe.conf` at the
next startup). Please avoid reset persistence when validation produced errors,
or make the request transactional, and add a test that the custom file is
unchanged after this rejection.
##########
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:
[P2] Replace the V1-only compound-reader fault hook
This conversion selects the V2 clone/entries `DorisCompoundReader`
constructor, but `construct_DorisCompoundReader_failed` exists only in the
parsing constructor reached by the V1 branch. The injected query therefore
succeeds normally, and because the suite never requires the catch block to run,
the test still passes without testing recovery. Please add or use a
V2-reachable reader hook and assert that the injected query actually fails.
--
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]
