github-actions[bot] commented on code in PR #70301:
URL: https://github.com/apache/doris/pull/70301#discussion_r4489672471
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/OlapTable.java:
##########
@@ -577,16 +577,17 @@ public void setIndexMeta(long indexId, String indexName,
List<Column> schema, in
indexNameToId.put(indexName, indexId);
}
- // rebuild the full schema of table
- // the order of columns in fullSchema is meaningless
+ // Keep base columns first, then append index columns in a deterministic
order.
public void rebuildFullSchema() {
fullSchema.clear();
nameToColumn.clear();
for (Column baseColumn : indexIdToMeta.get(baseIndexId).getSchema()) {
fullSchema.add(baseColumn);
nameToColumn.put(baseColumn.getName(), baseColumn);
}
- for (MaterializedIndexMeta indexMeta : getIndexIdToMeta().values()) {
+ // Fresh and Gson-restored metadata can have different map iteration
orders.
+ // Group commit producers and consumers planned on different FEs need
the same layout.
Review Comment:
[P1] Guard the layout change across mixed-version FEs
During a rolling FE upgrade, an old master can plan the consumer using
HashMap order while an upgraded follower produces blocks in this new sorted
order. For base index 1 and MV indexes 2 and 16, the old layout is [base, MV16,
MV2] and the new one is [base, MV2, MV16], although the schema version and
index count are identical. `GroupCommitTable::get_first_block_load_queue()`
checks only those two values. `GroupCommitOperatorX` fetches the raw producer
block, and `ScanOperatorX::get_block_after_projects()` bypasses projection
execution, so the consumer reads MV columns at its own expected positions.
Same-type columns can silently store each other's values; different types can
fail. Please negotiate/validate the layout or prevent incompatible
producer/consumer plans from sharing a queue, including draining queues created
before the upgrade. The acknowledged rollout limitation otherwise introduces
wrong data for previously matching FEs.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/OlapTable.java:
##########
@@ -577,16 +577,17 @@ public void setIndexMeta(long indexId, String indexName,
List<Column> schema, in
indexNameToId.put(indexName, indexId);
}
- // rebuild the full schema of table
- // the order of columns in fullSchema is meaningless
+ // Keep base columns first, then append index columns in a deterministic
order.
public void rebuildFullSchema() {
fullSchema.clear();
nameToColumn.clear();
for (Column baseColumn : indexIdToMeta.get(baseIndexId).getSchema()) {
fullSchema.add(baseColumn);
nameToColumn.put(baseColumn.getName(), baseColumn);
}
- for (MaterializedIndexMeta indexMeta : getIndexIdToMeta().values()) {
+ // Fresh and Gson-restored metadata can have different map iteration
orders.
+ // Group commit producers and consumers planned on different FEs need
the same layout.
+ for (MaterializedIndexMeta indexMeta : new
TreeMap<>(getIndexIdToMeta()).values()) {
Review Comment:
[P1] Rebuild the schema after RESTORE remaps index IDs
The sorted-order invariant also needs updating in `resetIdsForRestore()`: it
assigns new index IDs in an old-ID HashMap traversal but never rebuilds
`fullSchema`. The leader registers that same object in `RestoreJob`, whereas
followers deserialize it and `gsonPostProcess()` rebuilds the schema sorted by
the new IDs. For old IDs base=1, MV_A=2, MV_B=16, the leader keeps [base, A,
B], but remapping visits B before A, so followers rebuild [base, B, A]. These
same-version FEs then pass the unchanged schema-version/index-count queue
checks and disagree on positional MV columns, causing swapped values or type
errors. No upgrade is required. Rebuild the schema after remapping IDs and
before publishing the restored table, and add coverage comparing that object
with its Gson-restored copy.
--
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]
