github-actions[bot] commented on code in PR #66225:
URL: https://github.com/apache/doris/pull/66225#discussion_r3694904634
##########
cloud/src/meta-service/meta_service.cpp:
##########
@@ -4947,7 +4951,7 @@ void MetaServiceImpl::get_delete_bitmap_update_lock_v2(
msg = "failed to parse DeleteBitmapUpdateLockPB";
return;
}
- if (urgent) {
+ if (urgent &&
can_load_force_take_delete_bitmap_lock(lock_info.lock_id())) {
Review Comment:
[P1] Preserve legacy schema-change locks in the v2 fallback
When `-2` was acquired through v1, the live owner is stored only in
`lock_info.initiators()`/`expiration()`; no `mow_tablet_job_key` exists. After
`delete_bitmap_lock_v2_white_list` switches this instance to v2, this condition
correctly skips the force-take, but the following job-key scan finds an empty
range, labels all jobs expired, and overwrites the still-live schema lock with
the urgent load. The schema change's next delete-bitmap ownership recheck then
fails. Please recognize and honor the v1 aggregate representation (or
atomically migrate it) before treating an empty v2 job range as stale, and
cover v1-acquire to v2-urgent.
##########
regression-test/suites/fault_injection_p0/cloud/test_cloud_mow_load_preserves_schema_change_lock.groovy:
##########
@@ -0,0 +1,101 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+import org.apache.doris.regression.suite.ClusterOptions
+
+suite("test_cloud_mow_load_preserves_schema_change_lock", "docker") {
+ def options = new ClusterOptions()
+ options.setFeNum(1)
+ options.setBeNum(1)
+ options.cloudMode = true
+ options.enableDebugPoints()
+ options.feConfigs += [
+ "delete_bitmap_lock_expiration_seconds=60",
+ "enable_mow_load_force_take_ms_lock=true",
+ "mow_load_force_take_ms_lock_threshold_ms=0",
+ "meta_service_rpc_retry_times=2",
+ "enable_schema_change_retry=false"
+ ]
+ options.beConfigs += [
+ "delete_bitmap_lock_expiration_seconds=60"
+ ]
+
+ docker(options) {
+ GetDebugPoint().clearDebugPointsForAllBEs()
+
+ try {
+ sql "DROP TABLE IF EXISTS
test_cloud_mow_load_preserves_schema_change_lock"
+ sql """
+ CREATE TABLE test_cloud_mow_load_preserves_schema_change_lock (
+ k INT NOT NULL,
+ v INT
+ )
+ UNIQUE KEY(k)
+ DISTRIBUTED BY HASH(k) BUCKETS 1
+ PROPERTIES (
+ "enable_unique_key_merge_on_write" = "true",
+ "disable_auto_compaction" = "true",
+ "light_schema_change" = "false",
+ "replication_num" = "1"
+ )
+ """
+ sql "INSERT INTO test_cloud_mow_load_preserves_schema_change_lock
VALUES (1, 10)"
+
+ GetDebugPoint().enableDebugPointForAllBEs(
+
"CloudSchemaChangeJob::_process_delete_bitmap.inject_sleep",
+ [percent: "1.0", sleep: "10"])
+ def alterFuture = thread {
+ sql """
+ ALTER TABLE
test_cloud_mow_load_preserves_schema_change_lock
+ MODIFY COLUMN v BIGINT
+ """
+ }
+ // This debug point is after the schema change acquires the delete
bitmap lock.
+ // The debug point runs after schema change acquires the delete
bitmap lock.
+ // The table has only one rowset, so three seconds is ample for
reaching it.
+ sleep(3000)
Review Comment:
[P2] Rendezvous with the post-lock debug point before inserting
The ALTER runs on another thread, but this fixed three-second delay does not
prove that it has reached the debug point. On a slow runner the insert can run
before `get_delete_bitmap_update_lock()` and succeed, so the expected-exception
assertion fails without exercising this regression. Please expose and wait for
a bounded post-acquisition signal, then run the insert and release the
schema-change worker.
##########
cloud/src/meta-service/meta_service.cpp:
##########
@@ -5200,7 +5204,7 @@ void MetaServiceImpl::get_delete_bitmap_update_lock_v1(
msg = "failed to parse DeleteBitmapUpdateLockPB";
return;
}
- if (urgent) {
+ if (urgent &&
can_load_force_take_delete_bitmap_lock(lock_info.lock_id())) {
Review Comment:
[P1] Honor live v2 job keys when v1 sees a stale aggregate
V2 can leave this aggregate timestamp stale while the schema-change owner is
still live: same-ID acquisition refreshes only the `mow_tablet_job_key`, and v2
removal leaves the aggregate record behind. If a later urgent load is routed
through v1, this condition skips direct force-take but the following
aggregate-expiration branch still overwrites the live `-2` job without
consulting its fresh v2 key. Please reconcile or consult v2 job-key state when
v1 consumes this representation, and cover v2-live to v1-urgent.
--
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]
