github-actions[bot] commented on code in PR #68398:
URL: https://github.com/apache/doris/pull/68398#discussion_r4268548620
##########
cloud/src/meta-service/meta_service_job.cpp:
##########
@@ -293,6 +298,25 @@ void start_compaction_job(MetaServiceCode& code,
std::string& msg, std::stringst
// compaction operations and canceling other types of compaction.
compactions.Clear();
} else if (compaction.type() == TabletCompactionJobPB::STOP_TOKEN) {
+ // A STOP_TOKEN outlives the schema change run that registered it
when its BE dies
+ // before unregistering it: nothing renews or removes the token
until its lease
+ // expires (lease_compaction_interval_seconds * 4). FE guarantees
at most one schema
+ // change job per tablet and every run uses a fresh
delete_bitmap_lock_initiator, so
+ // an incoming STOP_TOKEN always belongs to the live run. Replace
the stale token and
+ // release the schema change delete bitmap lock its dead owner
still holds; otherwise
+ // the re-sent ALTER task is rejected here and FE cancels the
whole job.
+ for (const auto& c : compactions) {
Review Comment:
[P1] Fence superseded STOP_TOKEN registration retries
A different initiator does not establish that the incoming request is newer.
An old run A's `StartTabletJobRequest` can remain in
`MetaServiceProxy::call_impl`'s KV retry loop while FE resends the same logical
ALTER to run B after a restart or master failover. Once B registers its token,
acquires the `-2` lock and uploads its bitmap, a late A retry removes B's live
lock and replaces B's token here. B's schema-change commit then returns
`LOCK_EXPIRED`, which `CloudMetaMgr::commit_tablet_job` exposes as
`INTERNAL_ERROR`; `AlterJobV2.getRetryTimes` does not retry that error, so FE
cancels ALTER. One logical FE job per tablet and a local BE mutex do not fence
outstanding MS RPCs from an older execution. Reject superseded registrations
before revoking the current owner, and cover A registration -> B
registration/lock -> late A replay.
##########
cloud/src/meta-service/meta_service_job.cpp:
##########
@@ -293,6 +298,25 @@ void start_compaction_job(MetaServiceCode& code,
std::string& msg, std::stringst
// compaction operations and canceling other types of compaction.
compactions.Clear();
} else if (compaction.type() == TabletCompactionJobPB::STOP_TOKEN) {
+ // A STOP_TOKEN outlives the schema change run that registered it
when its BE dies
+ // before unregistering it: nothing renews or removes the token
until its lease
+ // expires (lease_compaction_interval_seconds * 4). FE guarantees
at most one schema
+ // change job per tablet and every run uses a fresh
delete_bitmap_lock_initiator, so
+ // an incoming STOP_TOKEN always belongs to the live run. Replace
the stale token and
+ // release the schema change delete bitmap lock its dead owner
still holds; otherwise
+ // the re-sent ALTER task is rejected here and FE cancels the
whole job.
+ for (const auto& c : compactions) {
+ if (!is_stop_token(c) || !c.has_delete_bitmap_lock_initiator()
||
+ c.delete_bitmap_lock_initiator() ==
compaction.delete_bitmap_lock_initiator()) {
+ continue;
+ }
+ INSTANCE_LOG(INFO)
+ << "replace stale STOP_TOKEN, tablet_id=" << tablet_id
+ << " new_job_id=" << compaction.id() << " stale_job="
<< proto_to_json(c);
+ remove_delete_bitmap_update_lock(txn, instance_id, table_id,
tablet_id,
Review Comment:
[P2] Clear the stale owner from both lock encodings
This helper's v2 branch calls v1 cleanup only when `mow_tablet_job(A)` is
absent. During a forward v1-to-v2 rollout, A's get-lock RPC can commit its
initiator in the v1 `-2` header, lose its reply, and retry on a v2 MS. The v2
same-lock-id path creates `mow_tablet_job(A)` without clearing header A, so
both encodings can legitimately remain when A's BE dies. This replacement then
deletes only the v2 key and loses A's STOP_TOKEN while leaving the legacy
initiator. B can acquire its v2 key and upload the bitmap, but
`check_and_remove_delete_bitmap_update_lock` rejects the nonempty header on
schema-change commit with `LOCK_EXPIRED`, so the restarted ALTER still gets
cancelled. Remove matching stale owner A from both encodings in the takeover
transaction while preserving unrelated owners, and cover the dual-encoding
state in the new 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]
