github-actions[bot] commented on code in PR #69205:
URL: https://github.com/apache/doris/pull/69205#discussion_r4305847939
##########
be/src/cloud/cloud_tablet.cpp:
##########
@@ -840,6 +840,9 @@ Result<std::unique_ptr<RowsetWriter>>
CloudTablet::create_rowset_writer(
context.partition_id = partition_id();
context.enable_unique_key_merge_on_write =
enable_unique_key_merge_on_write();
context.encrypt_algorithm = tablet_meta()->encryption_algorithm();
+ context.inverted_index_storage_format =
tablet_meta()->inverted_index_storage_format();
+ context.persist_inverted_index_storage_format =
Review Comment:
[P1] Preserve rowset formats for tablets created before rollout
This flag stays false for every tablet created before rollout was enabled. A
supported sequence is: create a V3 table/partition with rollout off; enable
rollout; light ADD COLUMN; set the partition default to SNII; add a new
partition before loading the old one. The new partition seeds the fresh
`(index_id, schema_version)` schema KV with SNII. A later load into the old
tablet still writes V3, but this gate omits its rowset format; `commit_rowset`
strips the embedded schema and keeps the existing SNII schema KV. A cold read
then hydrates the V3 rowset as SNII and selects the wrong index reader.
Preserve the physical format for these legacy tablets' new rowsets too, and
cover this sequence with a cold-read regression.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/OlapTable.java:
##########
@@ -3045,6 +3059,23 @@ public TInvertedIndexFileStorageFormat
getInvertedIndexFileStorageFormat() {
return tableProperty.getInvertedIndexFileStorageFormat();
}
+ public TInvertedIndexFileStorageFormat
getPartitionInvertedIndexFileStorageFormat() {
+ if (!Config.enable_partition_inverted_index_storage_format_rollout) {
+ return getInvertedIndexFileStorageFormat();
+ }
+ TInvertedIndexFileStorageFormat format = tableProperty == null
+ ? null :
tableProperty.getPartitionInvertedIndexFileStorageFormat();
+ return format != null ? format : getInvertedIndexFileStorageFormat();
+ }
+
+ public TInvertedIndexFileStorageFormat
getInvertedIndexFileStorageFormatForPartition(long partitionId) {
+ if (!Config.enable_partition_inverted_index_storage_format_rollout) {
Review Comment:
[P2] Honor persisted partition formats after master changes
The rollout flag is process-local and defaults to false: ordinary `ADMIN SET
FRONTEND CONFIG` changes only the master, and does not persist the setting.
After enabling rollout on master A and adding an SNII partition to a V3 table,
failover to an existing follower B makes this getter return V3 despite the
replicated SNII partition entry. The sibling default getter also ignores the
stored SNII default; new partitions revert to V3, and a heavy schema change
recreates the existing partition's shadow base tablets as V3. Restart after
SQL-only enabling has the same effect. Honor persisted format identities
independently of this local gate, and ensure the one-way rollout state survives
failover/restart.
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/datasource/CloudInternalCatalog.java:
##########
@@ -119,6 +119,8 @@ protected Partition createPartitionWithIndices(long dbId,
OlapTable tbl, long pa
}
MaterializedIndex baseIndex = new
MaterializedIndex(tbl.getBaseIndexId(), IndexState.NORMAL);
+ TInvertedIndexFileStorageFormat
partitionInvertedIndexFileStorageFormat =
Review Comment:
[P2] Use the captured partition format for physical tablet creation
`InternalCatalog.addPartition` already snapshots the format under its read
lock, but calls this method after unlocking and later records that original
snapshot in PartitionInfo/the journal. This reread can pick another format:
with concurrent ALTER SET changes V2 -> SNII before this read and SNII -> V2
before the final validation, ADD succeeds with physical SNII tablets recorded
as V2. The final equality check cannot detect that schedule. Pass the captured
format into creation (or use an immutable table snapshot) so tablet metadata
and the persisted partition entry use the same value.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/OlapTable.java:
##########
@@ -2218,6 +2223,8 @@ public Partition replacePartition(Partition newPartition,
partitionInfo.addPartition(newPartition.getId(), dataProperty,
replicaAlloc, isInMemory, isMutable);
}
+ partitionInfo.setInvertedIndexFileStorageFormat(newPartition.getId(),
Review Comment:
[P2] Keep the original format when recycling a truncated partition
`replacePartition` fills `recyclePartitionParam` manually but never copies
the old partition's `invertedIndexFileStorageFormat` before `dropPartition`
removes it. For a V2 table with an SNII partition, TRUNCATE without FORCE,
FORCE-dropping the replacement, then RECOVER PARTITION restores the original
SNII tablets without their format entry. SHOW reports V2 and later shadow-base
creation also uses V2. Save the old format in the recycle parameter before
removing its PartitionInfo entry; the new-partition setter here only preserves
the replacement's format.
--
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]
