github-actions[bot] commented on code in PR #69585:
URL: https://github.com/apache/doris/pull/69585#discussion_r4375160171
##########
fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java:
##########
@@ -2434,6 +2446,26 @@ private boolean
commitRemoteTxnImpl(TCommitRemoteTxnRequest request) throws User
}
TableIf tableIf = db.getTableOrMetaException(request.getTbl(),
TableType.OLAP);
OlapTable table = (OlapTable) tableIf;
+ try {
+ if (!request.isSetWriteSchemas() || request.getWriteSchemasSize()
!= 1) {
Review Comment:
[P1] Accept legacy remote commits for ordinary tables
An `INSERT INTO` a Doris-catalog table from a pre-PR source FE to an
upgraded destination writes its rowsets but sends `TCommitRemoteTxnRequest`
without `writeSchemas`: the old `RemoteOlapInsertExecutor.onComplete` fills
only the existing transaction/target/commit fields. This unconditional new
requirement then rejects the commit with `Remote commit requires exactly one
prepared table schema`, even when the target has no changelog. It also runs for
cloud targets before the branch that skips snapshot installation. Thus
upgrading the destination breaks existing remote writes for unaffected ordinary
tables. Preserve their legacy commit path and negotiate snapshot preparation
where changelog requires it; add coverage using an old-format request.
##########
be/src/storage/task/engine_publish_version_task.cpp:
##########
@@ -194,6 +208,16 @@ Status EnginePublishVersionTask::execute() {
tablet_info.tablet_id);
continue;
}
+ auto attach_changelog = txn_info_it->second->attach_changelog;
+ auto mapping_it = mappings.find(rowset->rowset_meta()->index_id());
+ attach_changelog.column_mappings =
+ mapping_it == mappings.end() ? nullptr :
mapping_it->second;
+ if (attach_changelog.rowset != nullptr &&
attach_changelog.column_mappings == nullptr) {
Review Comment:
[P1] Preserve publication of transactions committed before upgrade
Consider a changelog table with valid positive column UIDs and a transaction
committed by the pre-PR version but not yet published when the cluster is
upgraded/restarted. Its persisted `TransactionState` has no `writeSchemas`;
Gson leaves the new map empty, so `PublishVersionDaemon` resends an empty
mapping list. `DataDir` still recovers the attached changelog rowset, and this
branch rejects the publish before any tablet task runs. `TxnManager` rejects
null mappings too, so normal/async retries cannot make that committed data
visible and later partition versions remain blocked. This also occurs after a
full upgrade, independently of the existing negative-UID finding. Add
compatible recovery/backfill for legacy committed transactions and
pending-publish records before requiring their snapshot, with an old-format
replay test.
--
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]
