airborne12 commented on code in PR #67134:
URL: https://github.com/apache/doris/pull/67134#discussion_r3870448636
##########
be/src/storage/index/snii/format/core_metadata.cpp:
##########
@@ -207,13 +207,16 @@ Status decode_core_pb(const
doris::snii::SniiCoreMetadataPB& input, CoreMetadata
return corrupted("core metadata: scoring index requires a norms
region");
}
}
- if (has_scoring_tier ||
- (out->common_grams_metadata.has_value() &&
- out->common_grams_metadata->scoring_coverage ==
ScoringCoverage::kComplete)) {
+ // The scoring tier no longer implies CommonGrams: an ordinary analyzed
index
Review Comment:
Re-evaluated this against the actual production baseline rather than the
unpublished Apache master baseline.
The original claim remains technically reproducible against current Apache
master `1197ae1fa9800b7026317e3bd2fd2f26b527053b`: that decoder validates
metadata for every scoring T3 and rejects a metadata-free segment. However,
that reader is not deployed. The actual production baseline is SelectDB
[`branch-hotfix-selectdb-cloud-4.1.7-minimax-rows`](https://github.com/selectdb/selectdb-core/commits/branch-hotfix-selectdb-cloud-4.1.7-minimax-rows/)
at `ef649d804bc5a214829474715b4a3c4a2c696cd6`, with CommonGrams disabled. Its
reader accepts only `IndexConfig` 0/1, while this PR writes plain scoring as
config 2, so it rejects the new rowset before scoring metadata can affect
compatibility. The latest-master rebase restores the shipped protobuf field
numbers, but that does not remove this decisive index-config boundary.
Persisting plain metadata therefore still cannot make a new rowset readable by
the actual production reader.
Revised assessment under this production premise:
- Severity if the stated scenario existed: **9/10**
- Confidence that the comment scenario matches the real rollout: **1/10**
- Probability of encountering that scenario in production: **0/10**
- Normalized priority: **4.40/10** (`0.45×9 + 0.35×1 + 0.20×0`), below the
fix threshold
The proposed metadata/fingerprint/compaction identity work is therefore not
a low-cost compatibility correction. I removed the plain scoring metadata,
builtin analyzer fingerprint, plain-T3 compaction identity changes, and the
test that used the wrong old-reader baseline. CommonGrams-disabled plain
scoring remains metadata-free, and the original SNII compaction behavior is
unchanged.
If a future mixed-version rollout from `ef649d8` is required, it needs a
separate format/capability strategy, such as a writer gate, old-reader
backport, or format-version bump, plus a cross-version golden test. Plain
metadata alone is insufficient.
The amended follow-up is local commit `6ab16d07f22`, rebased onto current
master, and has not been pushed. Verification after rebase and rollback: 52
focused tests pass; clang-format/check-format and the ASAN BE build pass.
--
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]
