From 7524eb5c0bdd837d48b983b74bddadb27449d6f0 Mon Sep 17 00:00:00 2001 From: Kould Date: Tue, 22 Jul 2025 13:10:58 +0800 Subject: [PATCH 01/28] feat: impl `WarehouseOptions` for Private Task --- .../src/task_from_to_protobuf_impl.rs | 1 - .../proto-conv/tests/it/v137_task_message.rs | 156 ++++++++++++++++++ src/meta/protos/proto/task.proto | 2 +- 3 files changed, 157 insertions(+), 2 deletions(-) create mode 100644 src/meta/proto-conv/tests/it/v137_task_message.rs diff --git a/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs b/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs index 381aa50a8712f..79b38bb5dc60d 100644 --- a/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs +++ b/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs @@ -17,7 +17,6 @@ use chrono::Utc; use databend_common_meta_app::principal as mt; use databend_common_meta_app::principal::task::Status; use databend_common_protos::pb; -use databend_common_protos::pb::task_message::DeleteTask; use databend_common_protos::pb::task_message::Message; use crate::reader_check_msg; diff --git a/src/meta/proto-conv/tests/it/v137_task_message.rs b/src/meta/proto-conv/tests/it/v137_task_message.rs new file mode 100644 index 0000000000000..958a63e23caf0 --- /dev/null +++ b/src/meta/proto-conv/tests/it/v137_task_message.rs @@ -0,0 +1,156 @@ +// Copyright 2023 Datafuse Labs. +// +// Licensed 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. + +use chrono::DateTime; +use databend_common_meta_app::principal as mt; +use databend_common_meta_app::principal::ScheduleOptions; +use databend_common_meta_app::principal::ScheduleType; +use databend_common_meta_app::principal::WarehouseOptions; +use fastrace::func_name; +use maplit::btreemap; + +use crate::common; + +#[test] +fn test_decode_v137_task_message() -> anyhow::Result<()> { + let want_task = || mt::Task { + task_id: 11, + task_name: "task_c".to_string(), + query_text: "SELECT * FROM t1".to_string(), + when_condition: Some("c1 > 1".to_string()), + after: vec!["task_a".to_string(), "task_b".to_string()], + comment: Some("comment".to_string()), + owner: "public".to_string(), + owner_user: "me".to_string(), + schedule_options: Some(ScheduleOptions { + interval: Some(11), + cron: Some("30 12 * * *".to_string()), + time_zone: Some("UTC".to_string()), + schedule_type: ScheduleType::IntervalType, + milliseconds_interval: Some(11), + }), + warehouse_options: Some(WarehouseOptions { + warehouse: Some("warehouse_a".to_string()), + using_warehouse_size: Some("10".to_string()), + }), + next_scheduled_at: Some(DateTime::from_timestamp(10, 0).unwrap()), + suspend_task_after_num_failures: Some(10), + error_integration: None, + status: mt::Status::Suspended, + created_at: DateTime::from_timestamp(11, 0).unwrap(), + updated_at: DateTime::from_timestamp(12, 0).unwrap(), + last_suspended_at: Some(DateTime::from_timestamp(13, 0).unwrap()), + session_params: btreemap! { s("a") => s("b") }, + }; + + { + let task_message_execute_v137 = vec![ + 10, 239, 1, 8, 11, 18, 6, 116, 97, 115, 107, 95, 99, 34, 16, 83, 69, 76, 69, 67, 84, + 32, 42, 32, 70, 82, 79, 77, 32, 116, 49, 42, 7, 99, 111, 109, 109, 101, 110, 116, 50, + 6, 112, 117, 98, 108, 105, 99, 58, 22, 8, 11, 18, 11, 51, 48, 32, 49, 50, 32, 42, 32, + 42, 32, 42, 26, 3, 85, 84, 67, 40, 11, 66, 17, 10, 11, 119, 97, 114, 101, 104, 111, + 117, 115, 101, 95, 97, 18, 2, 49, 48, 74, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, + 32, 48, 48, 58, 48, 48, 58, 49, 48, 32, 85, 84, 67, 80, 10, 114, 23, 49, 57, 55, 48, + 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 49, 32, 85, 84, 67, 122, 23, + 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 50, 32, 85, 84, + 67, 130, 1, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, + 51, 32, 85, 84, 67, 138, 1, 6, 116, 97, 115, 107, 95, 97, 138, 1, 6, 116, 97, 115, 107, + 95, 98, 146, 1, 6, 99, 49, 32, 62, 32, 49, 154, 1, 6, 10, 1, 97, 18, 1, 98, 170, 1, 2, + 109, 101, 160, 6, 137, 1, 168, 6, 24, 160, 6, 137, 1, 168, 6, 24, + ]; + let want_execute = || mt::TaskMessage::ExecuteTask(want_task()); + + common::test_pb_from_to(func_name!(), want_execute())?; + common::test_load_old( + func_name!(), + task_message_execute_v137.as_slice(), + 137, + want_execute(), + )?; + } + { + let task_message_schedule_v137 = vec![ + 18, 239, 1, 8, 11, 18, 6, 116, 97, 115, 107, 95, 99, 34, 16, 83, 69, 76, 69, 67, 84, + 32, 42, 32, 70, 82, 79, 77, 32, 116, 49, 42, 7, 99, 111, 109, 109, 101, 110, 116, 50, + 6, 112, 117, 98, 108, 105, 99, 58, 22, 8, 11, 18, 11, 51, 48, 32, 49, 50, 32, 42, 32, + 42, 32, 42, 26, 3, 85, 84, 67, 40, 11, 66, 17, 10, 11, 119, 97, 114, 101, 104, 111, + 117, 115, 101, 95, 97, 18, 2, 49, 48, 74, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, + 32, 48, 48, 58, 48, 48, 58, 49, 48, 32, 85, 84, 67, 80, 10, 114, 23, 49, 57, 55, 48, + 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 49, 32, 85, 84, 67, 122, 23, + 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 50, 32, 85, 84, + 67, 130, 1, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, + 51, 32, 85, 84, 67, 138, 1, 6, 116, 97, 115, 107, 95, 97, 138, 1, 6, 116, 97, 115, 107, + 95, 98, 146, 1, 6, 99, 49, 32, 62, 32, 49, 154, 1, 6, 10, 1, 97, 18, 1, 98, 170, 1, 2, + 109, 101, 160, 6, 137, 1, 168, 6, 24, 160, 6, 137, 1, 168, 6, 24, + ]; + let want_schedule = || mt::TaskMessage::ScheduleTask(want_task()); + + common::test_pb_from_to(func_name!(), want_schedule())?; + common::test_load_old( + func_name!(), + task_message_schedule_v137.as_slice(), + 137, + want_schedule(), + )?; + } + { + let task_message_after_v137 = vec![ + 34, 239, 1, 8, 11, 18, 6, 116, 97, 115, 107, 95, 99, 34, 16, 83, 69, 76, 69, 67, 84, + 32, 42, 32, 70, 82, 79, 77, 32, 116, 49, 42, 7, 99, 111, 109, 109, 101, 110, 116, 50, + 6, 112, 117, 98, 108, 105, 99, 58, 22, 8, 11, 18, 11, 51, 48, 32, 49, 50, 32, 42, 32, + 42, 32, 42, 26, 3, 85, 84, 67, 40, 11, 66, 17, 10, 11, 119, 97, 114, 101, 104, 111, + 117, 115, 101, 95, 97, 18, 2, 49, 48, 74, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, + 32, 48, 48, 58, 48, 48, 58, 49, 48, 32, 85, 84, 67, 80, 10, 114, 23, 49, 57, 55, 48, + 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 49, 32, 85, 84, 67, 122, 23, + 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 50, 32, 85, 84, + 67, 130, 1, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, + 51, 32, 85, 84, 67, 138, 1, 6, 116, 97, 115, 107, 95, 97, 138, 1, 6, 116, 97, 115, 107, + 95, 98, 146, 1, 6, 99, 49, 32, 62, 32, 49, 154, 1, 6, 10, 1, 97, 18, 1, 98, 170, 1, 2, + 109, 101, 160, 6, 137, 1, 168, 6, 24, 160, 6, 137, 1, 168, 6, 24, + ]; + let want_after = || mt::TaskMessage::AfterTask(want_task()); + + common::test_pb_from_to(func_name!(), want_after())?; + common::test_load_old( + func_name!(), + task_message_after_v137.as_slice(), + 137, + want_after(), + )?; + } + { + let task_message_delete_v137 = vec![ + 26, 27, 10, 6, 116, 97, 115, 107, 95, 99, 18, 17, 10, 11, 119, 97, 114, 101, 104, 111, + 117, 115, 101, 95, 97, 18, 2, 49, 48, 160, 6, 137, 1, 168, 6, 24, + ]; + let want_delete = || { + let task = want_task(); + mt::TaskMessage::DeleteTask(task.task_name, task.warehouse_options) + }; + + common::test_pb_from_to(func_name!(), want_delete())?; + common::test_load_old( + func_name!(), + task_message_delete_v137.as_slice(), + 137, + want_delete(), + )?; + } + + Ok(()) +} + +fn s(ss: impl ToString) -> String { + ss.to_string() +} diff --git a/src/meta/protos/proto/task.proto b/src/meta/protos/proto/task.proto index 27c3e8f99aa8c..2842bd22bd384 100644 --- a/src/meta/protos/proto/task.proto +++ b/src/meta/protos/proto/task.proto @@ -45,7 +45,7 @@ message TaskMessage { oneof message { Task execute_task = 1; Task schedule_task = 2; - string delete_task = 3; + DeleteTask delete_task = 3; Task after_task = 4; DeleteTask delete_task_v2 = 5; } From 4d3c1d3ae546183711010f8b0fbf2cda94ae49eb Mon Sep 17 00:00:00 2001 From: kould Date: Fri, 25 Jul 2025 16:19:59 +0800 Subject: [PATCH 02/28] test: add test-private-task-warehouse.sh --- .../proto-conv/tests/it/v138_task_message.rs | 156 ++++++++++++++++++ 1 file changed, 156 insertions(+) create mode 100644 src/meta/proto-conv/tests/it/v138_task_message.rs diff --git a/src/meta/proto-conv/tests/it/v138_task_message.rs b/src/meta/proto-conv/tests/it/v138_task_message.rs new file mode 100644 index 0000000000000..6a1d808604435 --- /dev/null +++ b/src/meta/proto-conv/tests/it/v138_task_message.rs @@ -0,0 +1,156 @@ +// Copyright 2023 Datafuse Labs. +// +// Licensed 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. + +use chrono::DateTime; +use databend_common_meta_app::principal as mt; +use databend_common_meta_app::principal::ScheduleOptions; +use databend_common_meta_app::principal::ScheduleType; +use databend_common_meta_app::principal::WarehouseOptions; +use fastrace::func_name; +use maplit::btreemap; + +use crate::common; + +#[test] +fn test_decode_v138_task_message() -> anyhow::Result<()> { + let want_task = || mt::Task { + task_id: 11, + task_name: "task_c".to_string(), + query_text: "SELECT * FROM t1".to_string(), + when_condition: Some("c1 > 1".to_string()), + after: vec!["task_a".to_string(), "task_b".to_string()], + comment: Some("comment".to_string()), + owner: "public".to_string(), + owner_user: "me".to_string(), + schedule_options: Some(ScheduleOptions { + interval: Some(11), + cron: Some("30 12 * * *".to_string()), + time_zone: Some("UTC".to_string()), + schedule_type: ScheduleType::IntervalType, + milliseconds_interval: Some(11), + }), + warehouse_options: Some(WarehouseOptions { + warehouse: Some("warehouse_a".to_string()), + using_warehouse_size: Some("10".to_string()), + }), + next_scheduled_at: Some(DateTime::from_timestamp(10, 0).unwrap()), + suspend_task_after_num_failures: Some(10), + error_integration: None, + status: mt::Status::Suspended, + created_at: DateTime::from_timestamp(11, 0).unwrap(), + updated_at: DateTime::from_timestamp(12, 0).unwrap(), + last_suspended_at: Some(DateTime::from_timestamp(13, 0).unwrap()), + session_params: btreemap! { s("a") => s("b") }, + }; + + { + let task_message_execute_v138 = vec![ + 10, 239, 1, 8, 11, 18, 6, 116, 97, 115, 107, 95, 99, 34, 16, 83, 69, 76, 69, 67, 84, + 32, 42, 32, 70, 82, 79, 77, 32, 116, 49, 42, 7, 99, 111, 109, 109, 101, 110, 116, 50, + 6, 112, 117, 98, 108, 105, 99, 58, 22, 8, 11, 18, 11, 51, 48, 32, 49, 50, 32, 42, 32, + 42, 32, 42, 26, 3, 85, 84, 67, 40, 11, 66, 17, 10, 11, 119, 97, 114, 101, 104, 111, + 117, 115, 101, 95, 97, 18, 2, 49, 48, 74, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, + 32, 48, 48, 58, 48, 48, 58, 49, 48, 32, 85, 84, 67, 80, 10, 114, 23, 49, 57, 55, 48, + 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 49, 32, 85, 84, 67, 122, 23, + 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 50, 32, 85, 84, + 67, 130, 1, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, + 51, 32, 85, 84, 67, 138, 1, 6, 116, 97, 115, 107, 95, 97, 138, 1, 6, 116, 97, 115, 107, + 95, 98, 146, 1, 6, 99, 49, 32, 62, 32, 49, 154, 1, 6, 10, 1, 97, 18, 1, 98, 170, 1, 2, + 109, 101, 160, 6, 137, 1, 168, 6, 24, 160, 6, 137, 1, 168, 6, 24, + ]; + let want_execute = || mt::TaskMessage::ExecuteTask(want_task()); + + common::test_pb_from_to(func_name!(), want_execute())?; + common::test_load_old( + func_name!(), + task_message_execute_v138.as_slice(), + 138, + want_execute(), + )?; + } + { + let task_message_schedule_v138 = vec![ + 18, 239, 1, 8, 11, 18, 6, 116, 97, 115, 107, 95, 99, 34, 16, 83, 69, 76, 69, 67, 84, + 32, 42, 32, 70, 82, 79, 77, 32, 116, 49, 42, 7, 99, 111, 109, 109, 101, 110, 116, 50, + 6, 112, 117, 98, 108, 105, 99, 58, 22, 8, 11, 18, 11, 51, 48, 32, 49, 50, 32, 42, 32, + 42, 32, 42, 26, 3, 85, 84, 67, 40, 11, 66, 17, 10, 11, 119, 97, 114, 101, 104, 111, + 117, 115, 101, 95, 97, 18, 2, 49, 48, 74, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, + 32, 48, 48, 58, 48, 48, 58, 49, 48, 32, 85, 84, 67, 80, 10, 114, 23, 49, 57, 55, 48, + 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 49, 32, 85, 84, 67, 122, 23, + 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 50, 32, 85, 84, + 67, 130, 1, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, + 51, 32, 85, 84, 67, 138, 1, 6, 116, 97, 115, 107, 95, 97, 138, 1, 6, 116, 97, 115, 107, + 95, 98, 146, 1, 6, 99, 49, 32, 62, 32, 49, 154, 1, 6, 10, 1, 97, 18, 1, 98, 170, 1, 2, + 109, 101, 160, 6, 137, 1, 168, 6, 24, 160, 6, 137, 1, 168, 6, 24, + ]; + let want_schedule = || mt::TaskMessage::ScheduleTask(want_task()); + + common::test_pb_from_to(func_name!(), want_schedule())?; + common::test_load_old( + func_name!(), + task_message_schedule_v138.as_slice(), + 138, + want_schedule(), + )?; + } + { + let task_message_after_v138 = vec![ + 34, 239, 1, 8, 11, 18, 6, 116, 97, 115, 107, 95, 99, 34, 16, 83, 69, 76, 69, 67, 84, + 32, 42, 32, 70, 82, 79, 77, 32, 116, 49, 42, 7, 99, 111, 109, 109, 101, 110, 116, 50, + 6, 112, 117, 98, 108, 105, 99, 58, 22, 8, 11, 18, 11, 51, 48, 32, 49, 50, 32, 42, 32, + 42, 32, 42, 26, 3, 85, 84, 67, 40, 11, 66, 17, 10, 11, 119, 97, 114, 101, 104, 111, + 117, 115, 101, 95, 97, 18, 2, 49, 48, 74, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, + 32, 48, 48, 58, 48, 48, 58, 49, 48, 32, 85, 84, 67, 80, 10, 114, 23, 49, 57, 55, 48, + 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 49, 32, 85, 84, 67, 122, 23, + 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 50, 32, 85, 84, + 67, 130, 1, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, + 51, 32, 85, 84, 67, 138, 1, 6, 116, 97, 115, 107, 95, 97, 138, 1, 6, 116, 97, 115, 107, + 95, 98, 146, 1, 6, 99, 49, 32, 62, 32, 49, 154, 1, 6, 10, 1, 97, 18, 1, 98, 170, 1, 2, + 109, 101, 160, 6, 137, 1, 168, 6, 24, 160, 6, 137, 1, 168, 6, 24, + ]; + let want_after = || mt::TaskMessage::AfterTask(want_task()); + + common::test_pb_from_to(func_name!(), want_after())?; + common::test_load_old( + func_name!(), + task_message_after_v138.as_slice(), + 138, + want_after(), + )?; + } + { + let task_message_delete_v138 = vec![ + 26, 27, 10, 6, 116, 97, 115, 107, 95, 99, 18, 17, 10, 11, 119, 97, 114, 101, 104, 111, + 117, 115, 101, 95, 97, 18, 2, 49, 48, 160, 6, 137, 1, 168, 6, 24, + ]; + let want_delete = || { + let task = want_task(); + mt::TaskMessage::DeleteTask(task.task_name, task.warehouse_options) + }; + + common::test_pb_from_to(func_name!(), want_delete())?; + common::test_load_old( + func_name!(), + task_message_delete_v138.as_slice(), + 138, + want_delete(), + )?; + } + + Ok(()) +} + +fn s(ss: impl ToString) -> String { + ss.to_string() +} From 9a5a48d0c88308fb004e5cab76e4cb49574d6674 Mon Sep 17 00:00:00 2001 From: kould Date: Fri, 25 Jul 2025 16:35:05 +0800 Subject: [PATCH 03/28] chore: rebase --- .../proto-conv/tests/it/v138_task_message.rs | 156 ------------------ ...7_task_message.rs => v139_task_message.rs} | 34 ++-- 2 files changed, 17 insertions(+), 173 deletions(-) delete mode 100644 src/meta/proto-conv/tests/it/v138_task_message.rs rename src/meta/proto-conv/tests/it/{v137_task_message.rs => v139_task_message.rs} (89%) diff --git a/src/meta/proto-conv/tests/it/v138_task_message.rs b/src/meta/proto-conv/tests/it/v138_task_message.rs deleted file mode 100644 index 6a1d808604435..0000000000000 --- a/src/meta/proto-conv/tests/it/v138_task_message.rs +++ /dev/null @@ -1,156 +0,0 @@ -// Copyright 2023 Datafuse Labs. -// -// Licensed 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. - -use chrono::DateTime; -use databend_common_meta_app::principal as mt; -use databend_common_meta_app::principal::ScheduleOptions; -use databend_common_meta_app::principal::ScheduleType; -use databend_common_meta_app::principal::WarehouseOptions; -use fastrace::func_name; -use maplit::btreemap; - -use crate::common; - -#[test] -fn test_decode_v138_task_message() -> anyhow::Result<()> { - let want_task = || mt::Task { - task_id: 11, - task_name: "task_c".to_string(), - query_text: "SELECT * FROM t1".to_string(), - when_condition: Some("c1 > 1".to_string()), - after: vec!["task_a".to_string(), "task_b".to_string()], - comment: Some("comment".to_string()), - owner: "public".to_string(), - owner_user: "me".to_string(), - schedule_options: Some(ScheduleOptions { - interval: Some(11), - cron: Some("30 12 * * *".to_string()), - time_zone: Some("UTC".to_string()), - schedule_type: ScheduleType::IntervalType, - milliseconds_interval: Some(11), - }), - warehouse_options: Some(WarehouseOptions { - warehouse: Some("warehouse_a".to_string()), - using_warehouse_size: Some("10".to_string()), - }), - next_scheduled_at: Some(DateTime::from_timestamp(10, 0).unwrap()), - suspend_task_after_num_failures: Some(10), - error_integration: None, - status: mt::Status::Suspended, - created_at: DateTime::from_timestamp(11, 0).unwrap(), - updated_at: DateTime::from_timestamp(12, 0).unwrap(), - last_suspended_at: Some(DateTime::from_timestamp(13, 0).unwrap()), - session_params: btreemap! { s("a") => s("b") }, - }; - - { - let task_message_execute_v138 = vec![ - 10, 239, 1, 8, 11, 18, 6, 116, 97, 115, 107, 95, 99, 34, 16, 83, 69, 76, 69, 67, 84, - 32, 42, 32, 70, 82, 79, 77, 32, 116, 49, 42, 7, 99, 111, 109, 109, 101, 110, 116, 50, - 6, 112, 117, 98, 108, 105, 99, 58, 22, 8, 11, 18, 11, 51, 48, 32, 49, 50, 32, 42, 32, - 42, 32, 42, 26, 3, 85, 84, 67, 40, 11, 66, 17, 10, 11, 119, 97, 114, 101, 104, 111, - 117, 115, 101, 95, 97, 18, 2, 49, 48, 74, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, - 32, 48, 48, 58, 48, 48, 58, 49, 48, 32, 85, 84, 67, 80, 10, 114, 23, 49, 57, 55, 48, - 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 49, 32, 85, 84, 67, 122, 23, - 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 50, 32, 85, 84, - 67, 130, 1, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, - 51, 32, 85, 84, 67, 138, 1, 6, 116, 97, 115, 107, 95, 97, 138, 1, 6, 116, 97, 115, 107, - 95, 98, 146, 1, 6, 99, 49, 32, 62, 32, 49, 154, 1, 6, 10, 1, 97, 18, 1, 98, 170, 1, 2, - 109, 101, 160, 6, 137, 1, 168, 6, 24, 160, 6, 137, 1, 168, 6, 24, - ]; - let want_execute = || mt::TaskMessage::ExecuteTask(want_task()); - - common::test_pb_from_to(func_name!(), want_execute())?; - common::test_load_old( - func_name!(), - task_message_execute_v138.as_slice(), - 138, - want_execute(), - )?; - } - { - let task_message_schedule_v138 = vec![ - 18, 239, 1, 8, 11, 18, 6, 116, 97, 115, 107, 95, 99, 34, 16, 83, 69, 76, 69, 67, 84, - 32, 42, 32, 70, 82, 79, 77, 32, 116, 49, 42, 7, 99, 111, 109, 109, 101, 110, 116, 50, - 6, 112, 117, 98, 108, 105, 99, 58, 22, 8, 11, 18, 11, 51, 48, 32, 49, 50, 32, 42, 32, - 42, 32, 42, 26, 3, 85, 84, 67, 40, 11, 66, 17, 10, 11, 119, 97, 114, 101, 104, 111, - 117, 115, 101, 95, 97, 18, 2, 49, 48, 74, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, - 32, 48, 48, 58, 48, 48, 58, 49, 48, 32, 85, 84, 67, 80, 10, 114, 23, 49, 57, 55, 48, - 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 49, 32, 85, 84, 67, 122, 23, - 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 50, 32, 85, 84, - 67, 130, 1, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, - 51, 32, 85, 84, 67, 138, 1, 6, 116, 97, 115, 107, 95, 97, 138, 1, 6, 116, 97, 115, 107, - 95, 98, 146, 1, 6, 99, 49, 32, 62, 32, 49, 154, 1, 6, 10, 1, 97, 18, 1, 98, 170, 1, 2, - 109, 101, 160, 6, 137, 1, 168, 6, 24, 160, 6, 137, 1, 168, 6, 24, - ]; - let want_schedule = || mt::TaskMessage::ScheduleTask(want_task()); - - common::test_pb_from_to(func_name!(), want_schedule())?; - common::test_load_old( - func_name!(), - task_message_schedule_v138.as_slice(), - 138, - want_schedule(), - )?; - } - { - let task_message_after_v138 = vec![ - 34, 239, 1, 8, 11, 18, 6, 116, 97, 115, 107, 95, 99, 34, 16, 83, 69, 76, 69, 67, 84, - 32, 42, 32, 70, 82, 79, 77, 32, 116, 49, 42, 7, 99, 111, 109, 109, 101, 110, 116, 50, - 6, 112, 117, 98, 108, 105, 99, 58, 22, 8, 11, 18, 11, 51, 48, 32, 49, 50, 32, 42, 32, - 42, 32, 42, 26, 3, 85, 84, 67, 40, 11, 66, 17, 10, 11, 119, 97, 114, 101, 104, 111, - 117, 115, 101, 95, 97, 18, 2, 49, 48, 74, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, - 32, 48, 48, 58, 48, 48, 58, 49, 48, 32, 85, 84, 67, 80, 10, 114, 23, 49, 57, 55, 48, - 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 49, 32, 85, 84, 67, 122, 23, - 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 50, 32, 85, 84, - 67, 130, 1, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, - 51, 32, 85, 84, 67, 138, 1, 6, 116, 97, 115, 107, 95, 97, 138, 1, 6, 116, 97, 115, 107, - 95, 98, 146, 1, 6, 99, 49, 32, 62, 32, 49, 154, 1, 6, 10, 1, 97, 18, 1, 98, 170, 1, 2, - 109, 101, 160, 6, 137, 1, 168, 6, 24, 160, 6, 137, 1, 168, 6, 24, - ]; - let want_after = || mt::TaskMessage::AfterTask(want_task()); - - common::test_pb_from_to(func_name!(), want_after())?; - common::test_load_old( - func_name!(), - task_message_after_v138.as_slice(), - 138, - want_after(), - )?; - } - { - let task_message_delete_v138 = vec![ - 26, 27, 10, 6, 116, 97, 115, 107, 95, 99, 18, 17, 10, 11, 119, 97, 114, 101, 104, 111, - 117, 115, 101, 95, 97, 18, 2, 49, 48, 160, 6, 137, 1, 168, 6, 24, - ]; - let want_delete = || { - let task = want_task(); - mt::TaskMessage::DeleteTask(task.task_name, task.warehouse_options) - }; - - common::test_pb_from_to(func_name!(), want_delete())?; - common::test_load_old( - func_name!(), - task_message_delete_v138.as_slice(), - 138, - want_delete(), - )?; - } - - Ok(()) -} - -fn s(ss: impl ToString) -> String { - ss.to_string() -} diff --git a/src/meta/proto-conv/tests/it/v137_task_message.rs b/src/meta/proto-conv/tests/it/v139_task_message.rs similarity index 89% rename from src/meta/proto-conv/tests/it/v137_task_message.rs rename to src/meta/proto-conv/tests/it/v139_task_message.rs index 958a63e23caf0..f0263abc080f1 100644 --- a/src/meta/proto-conv/tests/it/v137_task_message.rs +++ b/src/meta/proto-conv/tests/it/v139_task_message.rs @@ -23,7 +23,7 @@ use maplit::btreemap; use crate::common; #[test] -fn test_decode_v137_task_message() -> anyhow::Result<()> { +fn test_decode_v139_task_message() -> anyhow::Result<()> { let want_task = || mt::Task { task_id: 11, task_name: "task_c".to_string(), @@ -55,7 +55,7 @@ fn test_decode_v137_task_message() -> anyhow::Result<()> { }; { - let task_message_execute_v137 = vec![ + let task_message_execute_v139 = vec![ 10, 239, 1, 8, 11, 18, 6, 116, 97, 115, 107, 95, 99, 34, 16, 83, 69, 76, 69, 67, 84, 32, 42, 32, 70, 82, 79, 77, 32, 116, 49, 42, 7, 99, 111, 109, 109, 101, 110, 116, 50, 6, 112, 117, 98, 108, 105, 99, 58, 22, 8, 11, 18, 11, 51, 48, 32, 49, 50, 32, 42, 32, @@ -67,20 +67,20 @@ fn test_decode_v137_task_message() -> anyhow::Result<()> { 67, 130, 1, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 51, 32, 85, 84, 67, 138, 1, 6, 116, 97, 115, 107, 95, 97, 138, 1, 6, 116, 97, 115, 107, 95, 98, 146, 1, 6, 99, 49, 32, 62, 32, 49, 154, 1, 6, 10, 1, 97, 18, 1, 98, 170, 1, 2, - 109, 101, 160, 6, 137, 1, 168, 6, 24, 160, 6, 137, 1, 168, 6, 24, + 109, 101, 160, 6, 139, 1, 168, 6, 24, 160, 6, 139, 1, 168, 6, 24, ]; let want_execute = || mt::TaskMessage::ExecuteTask(want_task()); common::test_pb_from_to(func_name!(), want_execute())?; common::test_load_old( func_name!(), - task_message_execute_v137.as_slice(), - 137, + task_message_execute_v139.as_slice(), + 139, want_execute(), )?; } { - let task_message_schedule_v137 = vec![ + let task_message_schedule_v139 = vec![ 18, 239, 1, 8, 11, 18, 6, 116, 97, 115, 107, 95, 99, 34, 16, 83, 69, 76, 69, 67, 84, 32, 42, 32, 70, 82, 79, 77, 32, 116, 49, 42, 7, 99, 111, 109, 109, 101, 110, 116, 50, 6, 112, 117, 98, 108, 105, 99, 58, 22, 8, 11, 18, 11, 51, 48, 32, 49, 50, 32, 42, 32, @@ -92,20 +92,20 @@ fn test_decode_v137_task_message() -> anyhow::Result<()> { 67, 130, 1, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 51, 32, 85, 84, 67, 138, 1, 6, 116, 97, 115, 107, 95, 97, 138, 1, 6, 116, 97, 115, 107, 95, 98, 146, 1, 6, 99, 49, 32, 62, 32, 49, 154, 1, 6, 10, 1, 97, 18, 1, 98, 170, 1, 2, - 109, 101, 160, 6, 137, 1, 168, 6, 24, 160, 6, 137, 1, 168, 6, 24, + 109, 101, 160, 6, 139, 1, 168, 6, 24, 160, 6, 139, 1, 168, 6, 24, ]; let want_schedule = || mt::TaskMessage::ScheduleTask(want_task()); common::test_pb_from_to(func_name!(), want_schedule())?; common::test_load_old( func_name!(), - task_message_schedule_v137.as_slice(), - 137, + task_message_schedule_v139.as_slice(), + 139, want_schedule(), )?; } { - let task_message_after_v137 = vec![ + let task_message_after_v139 = vec![ 34, 239, 1, 8, 11, 18, 6, 116, 97, 115, 107, 95, 99, 34, 16, 83, 69, 76, 69, 67, 84, 32, 42, 32, 70, 82, 79, 77, 32, 116, 49, 42, 7, 99, 111, 109, 109, 101, 110, 116, 50, 6, 112, 117, 98, 108, 105, 99, 58, 22, 8, 11, 18, 11, 51, 48, 32, 49, 50, 32, 42, 32, @@ -117,22 +117,22 @@ fn test_decode_v137_task_message() -> anyhow::Result<()> { 67, 130, 1, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 51, 32, 85, 84, 67, 138, 1, 6, 116, 97, 115, 107, 95, 97, 138, 1, 6, 116, 97, 115, 107, 95, 98, 146, 1, 6, 99, 49, 32, 62, 32, 49, 154, 1, 6, 10, 1, 97, 18, 1, 98, 170, 1, 2, - 109, 101, 160, 6, 137, 1, 168, 6, 24, 160, 6, 137, 1, 168, 6, 24, + 109, 101, 160, 6, 139, 1, 168, 6, 24, 160, 6, 139, 1, 168, 6, 24, ]; let want_after = || mt::TaskMessage::AfterTask(want_task()); common::test_pb_from_to(func_name!(), want_after())?; common::test_load_old( func_name!(), - task_message_after_v137.as_slice(), - 137, + task_message_after_v139.as_slice(), + 139, want_after(), )?; } { - let task_message_delete_v137 = vec![ + let task_message_delete_v139 = vec![ 26, 27, 10, 6, 116, 97, 115, 107, 95, 99, 18, 17, 10, 11, 119, 97, 114, 101, 104, 111, - 117, 115, 101, 95, 97, 18, 2, 49, 48, 160, 6, 137, 1, 168, 6, 24, + 117, 115, 101, 95, 97, 18, 2, 49, 48, 160, 6, 139, 1, 168, 6, 24, ]; let want_delete = || { let task = want_task(); @@ -142,8 +142,8 @@ fn test_decode_v137_task_message() -> anyhow::Result<()> { common::test_pb_from_to(func_name!(), want_delete())?; common::test_load_old( func_name!(), - task_message_delete_v137.as_slice(), - 137, + task_message_delete_v139.as_slice(), + 139, want_delete(), )?; } From a519250810c1e83f08db8db5694e6585730ec016 Mon Sep 17 00:00:00 2001 From: kould Date: Tue, 29 Jul 2025 16:08:11 +0800 Subject: [PATCH 04/28] chore: rebase --- .../proto-conv/tests/it/v139_task_message.rs | 156 ------------------ src/meta/protos/proto/task.proto | 2 +- 2 files changed, 1 insertion(+), 157 deletions(-) delete mode 100644 src/meta/proto-conv/tests/it/v139_task_message.rs diff --git a/src/meta/proto-conv/tests/it/v139_task_message.rs b/src/meta/proto-conv/tests/it/v139_task_message.rs deleted file mode 100644 index f0263abc080f1..0000000000000 --- a/src/meta/proto-conv/tests/it/v139_task_message.rs +++ /dev/null @@ -1,156 +0,0 @@ -// Copyright 2023 Datafuse Labs. -// -// Licensed 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. - -use chrono::DateTime; -use databend_common_meta_app::principal as mt; -use databend_common_meta_app::principal::ScheduleOptions; -use databend_common_meta_app::principal::ScheduleType; -use databend_common_meta_app::principal::WarehouseOptions; -use fastrace::func_name; -use maplit::btreemap; - -use crate::common; - -#[test] -fn test_decode_v139_task_message() -> anyhow::Result<()> { - let want_task = || mt::Task { - task_id: 11, - task_name: "task_c".to_string(), - query_text: "SELECT * FROM t1".to_string(), - when_condition: Some("c1 > 1".to_string()), - after: vec!["task_a".to_string(), "task_b".to_string()], - comment: Some("comment".to_string()), - owner: "public".to_string(), - owner_user: "me".to_string(), - schedule_options: Some(ScheduleOptions { - interval: Some(11), - cron: Some("30 12 * * *".to_string()), - time_zone: Some("UTC".to_string()), - schedule_type: ScheduleType::IntervalType, - milliseconds_interval: Some(11), - }), - warehouse_options: Some(WarehouseOptions { - warehouse: Some("warehouse_a".to_string()), - using_warehouse_size: Some("10".to_string()), - }), - next_scheduled_at: Some(DateTime::from_timestamp(10, 0).unwrap()), - suspend_task_after_num_failures: Some(10), - error_integration: None, - status: mt::Status::Suspended, - created_at: DateTime::from_timestamp(11, 0).unwrap(), - updated_at: DateTime::from_timestamp(12, 0).unwrap(), - last_suspended_at: Some(DateTime::from_timestamp(13, 0).unwrap()), - session_params: btreemap! { s("a") => s("b") }, - }; - - { - let task_message_execute_v139 = vec![ - 10, 239, 1, 8, 11, 18, 6, 116, 97, 115, 107, 95, 99, 34, 16, 83, 69, 76, 69, 67, 84, - 32, 42, 32, 70, 82, 79, 77, 32, 116, 49, 42, 7, 99, 111, 109, 109, 101, 110, 116, 50, - 6, 112, 117, 98, 108, 105, 99, 58, 22, 8, 11, 18, 11, 51, 48, 32, 49, 50, 32, 42, 32, - 42, 32, 42, 26, 3, 85, 84, 67, 40, 11, 66, 17, 10, 11, 119, 97, 114, 101, 104, 111, - 117, 115, 101, 95, 97, 18, 2, 49, 48, 74, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, - 32, 48, 48, 58, 48, 48, 58, 49, 48, 32, 85, 84, 67, 80, 10, 114, 23, 49, 57, 55, 48, - 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 49, 32, 85, 84, 67, 122, 23, - 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 50, 32, 85, 84, - 67, 130, 1, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, - 51, 32, 85, 84, 67, 138, 1, 6, 116, 97, 115, 107, 95, 97, 138, 1, 6, 116, 97, 115, 107, - 95, 98, 146, 1, 6, 99, 49, 32, 62, 32, 49, 154, 1, 6, 10, 1, 97, 18, 1, 98, 170, 1, 2, - 109, 101, 160, 6, 139, 1, 168, 6, 24, 160, 6, 139, 1, 168, 6, 24, - ]; - let want_execute = || mt::TaskMessage::ExecuteTask(want_task()); - - common::test_pb_from_to(func_name!(), want_execute())?; - common::test_load_old( - func_name!(), - task_message_execute_v139.as_slice(), - 139, - want_execute(), - )?; - } - { - let task_message_schedule_v139 = vec![ - 18, 239, 1, 8, 11, 18, 6, 116, 97, 115, 107, 95, 99, 34, 16, 83, 69, 76, 69, 67, 84, - 32, 42, 32, 70, 82, 79, 77, 32, 116, 49, 42, 7, 99, 111, 109, 109, 101, 110, 116, 50, - 6, 112, 117, 98, 108, 105, 99, 58, 22, 8, 11, 18, 11, 51, 48, 32, 49, 50, 32, 42, 32, - 42, 32, 42, 26, 3, 85, 84, 67, 40, 11, 66, 17, 10, 11, 119, 97, 114, 101, 104, 111, - 117, 115, 101, 95, 97, 18, 2, 49, 48, 74, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, - 32, 48, 48, 58, 48, 48, 58, 49, 48, 32, 85, 84, 67, 80, 10, 114, 23, 49, 57, 55, 48, - 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 49, 32, 85, 84, 67, 122, 23, - 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 50, 32, 85, 84, - 67, 130, 1, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, - 51, 32, 85, 84, 67, 138, 1, 6, 116, 97, 115, 107, 95, 97, 138, 1, 6, 116, 97, 115, 107, - 95, 98, 146, 1, 6, 99, 49, 32, 62, 32, 49, 154, 1, 6, 10, 1, 97, 18, 1, 98, 170, 1, 2, - 109, 101, 160, 6, 139, 1, 168, 6, 24, 160, 6, 139, 1, 168, 6, 24, - ]; - let want_schedule = || mt::TaskMessage::ScheduleTask(want_task()); - - common::test_pb_from_to(func_name!(), want_schedule())?; - common::test_load_old( - func_name!(), - task_message_schedule_v139.as_slice(), - 139, - want_schedule(), - )?; - } - { - let task_message_after_v139 = vec![ - 34, 239, 1, 8, 11, 18, 6, 116, 97, 115, 107, 95, 99, 34, 16, 83, 69, 76, 69, 67, 84, - 32, 42, 32, 70, 82, 79, 77, 32, 116, 49, 42, 7, 99, 111, 109, 109, 101, 110, 116, 50, - 6, 112, 117, 98, 108, 105, 99, 58, 22, 8, 11, 18, 11, 51, 48, 32, 49, 50, 32, 42, 32, - 42, 32, 42, 26, 3, 85, 84, 67, 40, 11, 66, 17, 10, 11, 119, 97, 114, 101, 104, 111, - 117, 115, 101, 95, 97, 18, 2, 49, 48, 74, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, - 32, 48, 48, 58, 48, 48, 58, 49, 48, 32, 85, 84, 67, 80, 10, 114, 23, 49, 57, 55, 48, - 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 49, 32, 85, 84, 67, 122, 23, - 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, 50, 32, 85, 84, - 67, 130, 1, 23, 49, 57, 55, 48, 45, 48, 49, 45, 48, 49, 32, 48, 48, 58, 48, 48, 58, 49, - 51, 32, 85, 84, 67, 138, 1, 6, 116, 97, 115, 107, 95, 97, 138, 1, 6, 116, 97, 115, 107, - 95, 98, 146, 1, 6, 99, 49, 32, 62, 32, 49, 154, 1, 6, 10, 1, 97, 18, 1, 98, 170, 1, 2, - 109, 101, 160, 6, 139, 1, 168, 6, 24, 160, 6, 139, 1, 168, 6, 24, - ]; - let want_after = || mt::TaskMessage::AfterTask(want_task()); - - common::test_pb_from_to(func_name!(), want_after())?; - common::test_load_old( - func_name!(), - task_message_after_v139.as_slice(), - 139, - want_after(), - )?; - } - { - let task_message_delete_v139 = vec![ - 26, 27, 10, 6, 116, 97, 115, 107, 95, 99, 18, 17, 10, 11, 119, 97, 114, 101, 104, 111, - 117, 115, 101, 95, 97, 18, 2, 49, 48, 160, 6, 139, 1, 168, 6, 24, - ]; - let want_delete = || { - let task = want_task(); - mt::TaskMessage::DeleteTask(task.task_name, task.warehouse_options) - }; - - common::test_pb_from_to(func_name!(), want_delete())?; - common::test_load_old( - func_name!(), - task_message_delete_v139.as_slice(), - 139, - want_delete(), - )?; - } - - Ok(()) -} - -fn s(ss: impl ToString) -> String { - ss.to_string() -} diff --git a/src/meta/protos/proto/task.proto b/src/meta/protos/proto/task.proto index 2842bd22bd384..27c3e8f99aa8c 100644 --- a/src/meta/protos/proto/task.proto +++ b/src/meta/protos/proto/task.proto @@ -45,7 +45,7 @@ message TaskMessage { oneof message { Task execute_task = 1; Task schedule_task = 2; - DeleteTask delete_task = 3; + string delete_task = 3; Task after_task = 4; DeleteTask delete_task_v2 = 5; } From 67b74272059512b0d1b907a0f9dce54e26286e62 Mon Sep 17 00:00:00 2001 From: kould Date: Wed, 30 Jul 2025 11:16:40 +0800 Subject: [PATCH 05/28] refactor: transfer the after check sql to meta --- src/common/exception/src/exception_code.rs | 2 + src/meta/app/src/principal/mod.rs | 5 + src/meta/app/src/principal/task.rs | 60 +++++++ .../app/src/principal/task_dependent_ident.rs | 45 +++++ .../app/src/principal/task_state_ident.rs | 45 +++++ .../src/task_from_to_protobuf_impl.rs | 56 ++++++ src/meta/proto-conv/src/util.rs | 1 + src/meta/proto-conv/tests/it/main.rs | 1 + .../proto-conv/tests/it/v141_task_state.rs | 48 +++++ src/meta/protos/proto/task.proto | 20 +++ src/query/management/src/task/errors.rs | 34 ++-- src/query/management/src/task/task_mgr.rs | 170 +++++++++++++++++- .../service/src/interpreters/task/private.rs | 18 ++ src/query/service/src/task/service.rs | 127 ++----------- 14 files changed, 506 insertions(+), 126 deletions(-) create mode 100644 src/meta/app/src/principal/task_dependent_ident.rs create mode 100644 src/meta/app/src/principal/task_state_ident.rs create mode 100644 src/meta/proto-conv/tests/it/v141_task_state.rs diff --git a/src/common/exception/src/exception_code.rs b/src/common/exception/src/exception_code.rs index 28a9d88e4b921..7e7028dfc9d6d 100644 --- a/src/common/exception/src/exception_code.rs +++ b/src/common/exception/src/exception_code.rs @@ -405,6 +405,8 @@ build_exceptions! { TaskScheduleAndAfterConflict(2615), /// Task when condition not met TaskWhenConditionNotMet(2616), + /// Task Running when modifying after + TaskRunningWhenModifyingAfter(2617), } // Search and External Service Errors [1901-1903, 1910] diff --git a/src/meta/app/src/principal/mod.rs b/src/meta/app/src/principal/mod.rs index b0402f5eefe27..c2a3f32e4bc3f 100644 --- a/src/meta/app/src/principal/mod.rs +++ b/src/meta/app/src/principal/mod.rs @@ -49,8 +49,10 @@ pub mod procedure_identity; pub mod procedure_name_ident; pub mod stage_file_ident; pub mod task; +pub mod task_dependent_ident; pub mod task_ident; pub mod task_message_ident; +pub mod task_state_ident; pub mod tenant_ownership_object_ident; pub mod tenant_user_ident; pub mod user_defined_file_format_ident; @@ -89,13 +91,16 @@ pub use role_info::RoleInfo; pub use role_info::RoleInfoSerdeError; pub use stage_file_ident::StageFileIdent; pub use stage_file_path::StageFilePath; +pub use task::DependentType; pub use task::ScheduleOptions; pub use task::ScheduleType; pub use task::State; pub use task::Status; pub use task::Task; +pub use task::TaskDependent; pub use task::TaskMessage; pub use task::TaskRun; +pub use task::TaskState; pub use task::WarehouseOptions; pub use task_ident::TaskIdent; pub use task_ident::TaskIdentRaw; diff --git a/src/meta/app/src/principal/task.rs b/src/meta/app/src/principal/task.rs index 4b45c6ab3f118..a13b52989912f 100644 --- a/src/meta/app/src/principal/task.rs +++ b/src/meta/app/src/principal/task.rs @@ -172,3 +172,63 @@ impl TaskMessage { } } } + +#[derive(Debug, Clone, PartialEq)] +pub struct TaskState { + pub is_succeeded: bool, +} + +#[derive(Debug, Clone, Copy, PartialEq)] +pub enum DependentType { + After = 0, + Before = 1, +} + +#[derive(Debug, Clone, PartialEq)] +pub struct TaskDependent { + pub ty: DependentType, + pub source: String, + pub target: String, +} + +impl TaskDependent { + pub fn new(ty: DependentType, source: String, target: String) -> Self { + Self { ty, source, target } + } +} + +mod kvapi_key_impl { + use databend_common_meta_kvapi::kvapi; + use databend_common_meta_kvapi::kvapi::KeyError; + + use crate::principal::DependentType; + use crate::principal::TaskDependent; + + impl kvapi::KeyCodec for TaskDependent { + fn encode_key(&self, b: kvapi::KeyBuilder) -> kvapi::KeyBuilder { + match self.ty { + DependentType::After => b.push_str("After"), + DependentType::Before => b.push_str("Before"), + } + .push_str(self.source.as_str()) + .push_str(self.target.as_str()) + } + + fn decode_key(parser: &mut kvapi::KeyParser) -> Result { + let ty = match parser.next_str()?.as_str() { + "After" => DependentType::After, + "Before" => DependentType::Before, + str => { + return Err(KeyError::InvalidId { + s: str.to_string(), + reason: "Invalid Dependent Type".to_string(), + }) + } + }; + let source = parser.next_str()?; + let target = parser.next_str()?; + + Ok(Self { ty, source, target }) + } + } +} diff --git a/src/meta/app/src/principal/task_dependent_ident.rs b/src/meta/app/src/principal/task_dependent_ident.rs new file mode 100644 index 0000000000000..f5e60a4dcb938 --- /dev/null +++ b/src/meta/app/src/principal/task_dependent_ident.rs @@ -0,0 +1,45 @@ +// Copyright 2021 Datafuse Labs +// +// Licensed 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. + +use crate::tenant_key::ident::TIdent; + +pub type TaskDependentIdent = TIdent; + +pub use kvapi_impl::TaskDependentResource; + +use crate::principal::TaskDependent; + +mod kvapi_impl { + use databend_common_meta_kvapi::kvapi; + + use crate::principal::task::TaskDependent; + use crate::principal::task_dependent_ident::TaskDependentIdent; + use crate::tenant_key::resource::TenantResource; + + pub struct TaskDependentResource; + impl TenantResource for TaskDependentResource { + const PREFIX: &'static str = "__fd_task_dependents"; + const TYPE: &'static str = "TaskDependentIdent"; + const HAS_TENANT: bool = true; + type ValueType = TaskDependent; + } + + impl kvapi::Value for TaskDependent { + type KeyType = TaskDependentIdent; + + fn dependency_keys(&self, _key: &Self::KeyType) -> impl IntoIterator { + [] + } + } +} diff --git a/src/meta/app/src/principal/task_state_ident.rs b/src/meta/app/src/principal/task_state_ident.rs new file mode 100644 index 0000000000000..7b973e2b50fb4 --- /dev/null +++ b/src/meta/app/src/principal/task_state_ident.rs @@ -0,0 +1,45 @@ +// Copyright 2021 Datafuse Labs +// +// Licensed 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. + +use crate::tenant_key::ident::TIdent; + +pub type TaskStateIdent = TIdent; + +pub type TaskStateIdentRaw = TIdent; + +pub use kvapi_impl::Resource; + +mod kvapi_impl { + use databend_common_meta_kvapi::kvapi; + + use crate::principal::task::TaskState; + use crate::principal::task_state_ident::TaskStateIdent; + use crate::tenant_key::resource::TenantResource; + + pub struct Resource; + impl TenantResource for Resource { + const PREFIX: &'static str = "__fd_task_states"; + const TYPE: &'static str = "TaskStateIdent"; + const HAS_TENANT: bool = true; + type ValueType = TaskState; + } + + impl kvapi::Value for TaskState { + type KeyType = TaskStateIdent; + + fn dependency_keys(&self, _key: &Self::KeyType) -> impl IntoIterator { + [] + } + } +} diff --git a/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs b/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs index 79b38bb5dc60d..1d19a71c70527 100644 --- a/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs +++ b/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs @@ -16,6 +16,7 @@ use chrono::DateTime; use chrono::Utc; use databend_common_meta_app::principal as mt; use databend_common_meta_app::principal::task::Status; +use databend_common_meta_app::principal::DependentType; use databend_common_protos::pb; use databend_common_protos::pb::task_message::Message; @@ -194,3 +195,58 @@ impl FromToProto for mt::TaskMessage { }) } } + +impl FromToProto for mt::TaskDependent { + type PB = pb::TaskDependent; + + fn get_pb_ver(p: &Self::PB) -> u64 { + p.ver + } + + fn from_pb(p: Self::PB) -> Result + where Self: Sized { + Ok(Self { + ty: match p.ty { + 0 => DependentType::After, + 1 => DependentType::Before, + _ => return Err(Incompatible::new(format!("invalid task type {}", p.ty))), + }, + source: p.source, + target: p.target, + }) + } + + fn to_pb(&self) -> Result { + Ok(pb::TaskDependent { + ver: VER, + min_reader_ver: MIN_READER_VER, + + source: self.source.clone(), + target: self.target.clone(), + ty: self.ty as i32, + }) + } +} + +impl FromToProto for mt::TaskState { + type PB = pb::TaskState; + + fn get_pb_ver(p: &Self::PB) -> u64 { + p.ver + } + + fn from_pb(p: Self::PB) -> Result + where Self: Sized { + Ok(Self { + is_succeeded: p.is_succeeded, + }) + } + + fn to_pb(&self) -> Result { + Ok(pb::TaskState { + ver: VER, + min_reader_ver: MIN_READER_VER, + is_succeeded: self.is_succeeded, + }) + } +} diff --git a/src/meta/proto-conv/src/util.rs b/src/meta/proto-conv/src/util.rs index 6b21d5d4fe020..a54e20f65b718 100644 --- a/src/meta/proto-conv/src/util.rs +++ b/src/meta/proto-conv/src/util.rs @@ -170,6 +170,7 @@ const META_CHANGE_LOG: &[(u64, &str)] = &[ (138, "2025-07-23: Add: TableStatistics add index size"), (139, "2025-07-25: Add: Grant/OwnershipSequenceObject and UserPrivilegeType AccessSequence, AccessSequence"), (140, "2025-07-24: Add: TaskMessage::Delete add WarehouseOptions"), + (141, "2025-07-27: Add: TaskState and TaskDependent"), // Dear developer: // If you're gonna add a new metadata version, you'll have to add a test for it. // You could just copy an existing test file(e.g., `../tests/it/v024_table_meta.rs`) diff --git a/src/meta/proto-conv/tests/it/main.rs b/src/meta/proto-conv/tests/it/main.rs index 8581d843ac051..1eda7d6426ce9 100644 --- a/src/meta/proto-conv/tests/it/main.rs +++ b/src/meta/proto-conv/tests/it/main.rs @@ -132,3 +132,4 @@ mod v137_add_grant_object_connection; mod v138_table_statistics; mod v139_add_grant_ownership_object_sequence; mod v140_task_message; +mod v141_task_state; diff --git a/src/meta/proto-conv/tests/it/v141_task_state.rs b/src/meta/proto-conv/tests/it/v141_task_state.rs new file mode 100644 index 0000000000000..c89df8f8d5060 --- /dev/null +++ b/src/meta/proto-conv/tests/it/v141_task_state.rs @@ -0,0 +1,48 @@ +// Copyright 2023 Datafuse Labs. +// +// Licensed 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. + +use databend_common_meta_app::principal as mt; +use fastrace::func_name; + +use crate::common; + +#[test] +fn test_decode_v141_task_state() -> anyhow::Result<()> { + let task_state_v141 = vec![]; + + let want = || mt::TaskState { is_succeeded: true }; + common::test_pb_from_to(func_name!(), want())?; + common::test_load_old(func_name!(), task_state_v141.as_slice(), 141, want())?; + + Ok(()) +} + +#[test] +fn test_decode_v141_task_dependent() -> anyhow::Result<()> { + let task_dependent_v141 = vec![]; + + let want = || mt::TaskDependent { + ty: mt::DependentType::After, + source: s("a"), + target: s("c"), + }; + common::test_pb_from_to(func_name!(), want())?; + common::test_load_old(func_name!(), task_dependent_v141.as_slice(), 141, want())?; + + Ok(()) +} + +fn s(ss: impl ToString) -> String { + ss.to_string() +} diff --git a/src/meta/protos/proto/task.proto b/src/meta/protos/proto/task.proto index 27c3e8f99aa8c..38a5e90dbe38a 100644 --- a/src/meta/protos/proto/task.proto +++ b/src/meta/protos/proto/task.proto @@ -78,3 +78,23 @@ message Task { optional string error_integration = 20; string owner_user = 21; } + +message TaskState { + uint64 ver = 100; + uint64 min_reader_ver = 101; + + bool is_succeeded = 1; +} + +message TaskDependent { + uint64 ver = 100; + uint64 min_reader_ver = 101; + + enum DependentType { + After = 0; + Before = 1; + } + string source = 1; + string target = 2; + DependentType ty = 3; +} diff --git a/src/query/management/src/task/errors.rs b/src/query/management/src/task/errors.rs index 703728729ad1c..bc74fe880acf9 100644 --- a/src/query/management/src/task/errors.rs +++ b/src/query/management/src/task/errors.rs @@ -12,10 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::fmt::Display; - use databend_common_exception::ErrorCode; use databend_common_meta_types::MetaError; +use databend_common_proto_conv::Incompatible; use crate::errors::TenantError; @@ -84,6 +83,12 @@ pub enum TaskApiError { meta_err: MetaError, context: String, }, + + #[error("Incompatible error: {inner}")] + Incompatible { inner: Incompatible }, + + #[error("There are simultaneous update to task: {task_name} afters: {after}")] + SimultaneousUpdateTaskAfter { task_name: String, after: String }, } impl From for TaskApiError { @@ -95,6 +100,12 @@ impl From for TaskApiError { } } +impl From for TaskApiError { + fn from(value: Incompatible) -> Self { + TaskApiError::Incompatible { inner: value } + } +} + impl From for ErrorCode { fn from(value: TaskApiError) -> Self { match value { @@ -102,21 +113,10 @@ impl From for ErrorCode { TaskApiError::MetaError { meta_err, context } => { ErrorCode::from(meta_err).add_message_back(context) } - } - } -} - -impl TaskApiError { - pub fn append_context(self, context: impl Display) -> Self { - match self { - TaskApiError::TenantError(e) => TaskApiError::TenantError(e.append_context(context)), - TaskApiError::MetaError { - meta_err, - context: old, - } => TaskApiError::MetaError { - meta_err, - context: format!("{}; {}", old, context), - }, + TaskApiError::Incompatible { inner } => ErrorCode::from_std_error(inner), + TaskApiError::SimultaneousUpdateTaskAfter { .. } => { + ErrorCode::from_string(value.to_string()) + } } } } diff --git a/src/query/management/src/task/task_mgr.rs b/src/query/management/src/task/task_mgr.rs index 65949a6d83935..404fb06cbe3ae 100644 --- a/src/query/management/src/task/task_mgr.rs +++ b/src/query/management/src/task/task_mgr.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::collections::HashSet; use std::str::FromStr; use std::sync::Arc; @@ -24,19 +25,33 @@ use databend_common_meta_api::kv_pb_api::KVPbApi; use databend_common_meta_api::kv_pb_api::UpsertPB; use databend_common_meta_app::principal::task; use databend_common_meta_app::principal::task::TaskMessage; +use databend_common_meta_app::principal::task::TaskState; +use databend_common_meta_app::principal::task_dependent_ident::TaskDependentIdent; use databend_common_meta_app::principal::task_message_ident::TaskMessageIdent; +use databend_common_meta_app::principal::task_state_ident::TaskStateIdent; +use databend_common_meta_app::principal::DependentType; use databend_common_meta_app::principal::ScheduleType; use databend_common_meta_app::principal::Status; use databend_common_meta_app::principal::Task; +use databend_common_meta_app::principal::TaskDependent; use databend_common_meta_app::principal::TaskIdent; use databend_common_meta_app::schema::CreateOption; use databend_common_meta_app::tenant::Tenant; use databend_common_meta_kvapi::kvapi; use databend_common_meta_kvapi::kvapi::DirName; +use databend_common_meta_kvapi::kvapi::Key; +use databend_common_meta_types::txn_condition::Target; +use databend_common_meta_types::ConditionResult; use databend_common_meta_types::MatchSeq; use databend_common_meta_types::MetaError; +use databend_common_meta_types::TxnCondition; +use databend_common_meta_types::TxnOp; +use databend_common_meta_types::TxnRequest; use databend_common_meta_types::With; +use databend_common_proto_conv::FromToProto; +use futures::StreamExt; use futures::TryStreamExt; +use prost::Message; use crate::task::errors::TaskApiError; use crate::task::errors::TaskError; @@ -251,6 +266,157 @@ impl TaskMgr { Ok(change.is_changed()) } + #[async_backtrace::framed] + #[fastrace::trace] + pub async fn update_after( + &self, + task_name: &str, + task_after: &[String], + ) -> Result, TaskApiError> { + let task_after_ident = DirName::new(TaskDependentIdent::new_generic( + &self.tenant, + TaskDependent::new(DependentType::After, task_name.to_string(), "".to_string()), + )); + + let mut update_ops = Vec::new(); + + let mut new_afters: HashSet<&String> = task_after.iter().collect(); + let mut remove_afters: Vec = Vec::new(); + let mut task_after_stream = self.kv_api.list_pb_values(&task_after_ident).await?; + + while let Some(after_task_dependent) = task_after_stream.next().await { + let after_task_dependent = after_task_dependent?; + + debug_assert_eq!(after_task_dependent.ty, DependentType::After); + + if !new_afters.remove(&after_task_dependent.target) { + remove_afters.push(after_task_dependent.target.clone()); + } + } + let mut check_ops = Vec::with_capacity(new_afters.len()); + for after in new_afters { + let after_dependent = + TaskDependent::new(DependentType::After, task_name.to_string(), after.clone()); + let after_dependent_ident = + TaskDependentIdent::new_generic(&self.tenant, after_dependent.clone()); + + let before_dependent = + TaskDependent::new(DependentType::Before, after.clone(), task_name.to_string()); + let before_dependent_ident = + TaskDependentIdent::new_generic(&self.tenant, before_dependent.clone()); + + update_ops.push(TxnOp::put( + after_dependent_ident.to_string_key(), + after_dependent.to_pb()?.encode_to_vec(), + )); + update_ops.push(TxnOp::put( + before_dependent_ident.to_string_key(), + before_dependent.to_pb()?.encode_to_vec(), + )); + // Ensure that the related tasks still exist + check_ops.push(TxnCondition { + key: TaskStateIdent::new(&self.tenant, after).to_string_key(), + expected: ConditionResult::Ge as i32, + target: Some(Target::Seq(0)), + }); + } + for after in remove_afters { + let before_dependent_ident = TaskDependentIdent::new_generic( + &self.tenant, + TaskDependent::new(DependentType::Before, after.clone(), task_name.to_string()), + ); + + update_ops.push(TxnOp::delete(before_dependent_ident.to_string_key())); + } + let request = TxnRequest::new(check_ops, update_ops); + let reply = self.kv_api.transaction(request).await?; + + if reply.success { + return Err(TaskApiError::SimultaneousUpdateTaskAfter { + task_name: task_name.to_string(), + after: task_after.join(","), + }); + } + + Ok(Ok(())) + } + + #[async_backtrace::framed] + #[fastrace::trace] + pub async fn task_succeeded( + &self, + task_name: &str, + ) -> Result, TaskError>, TaskApiError> { + let task_before_ident = DirName::new(TaskDependentIdent::new_generic( + &self.tenant, + TaskDependent::new(DependentType::Before, task_name.to_string(), "".to_string()), + )); + let task_state_key = TaskStateIdent::new(&self.tenant, task_name); + let succeeded_value = TaskState { is_succeeded: true }.to_pb()?.encode_to_vec(); + let not_succeeded_value = TaskState { + is_succeeded: false, + } + .to_pb()? + .encode_to_vec(); + + let mut ready_tasks = Vec::new(); + let mut task_before_stream = self.kv_api.list_pb_values(&task_before_ident).await?; + + while let Some(task_before_dependent) = task_before_stream.next().await { + let task_before_dependent = task_before_dependent?; + + let before_task_after_ident = DirName::new(TaskDependentIdent::new_generic( + &self.tenant, + TaskDependent::new( + DependentType::After, + task_before_dependent.target.to_string(), + "".to_string(), + ), + )); + let mut before_task_after_stream = + self.kv_api.list_pb_values(&before_task_after_ident).await?; + + let mut conditions = Vec::new(); + let mut if_ops = Vec::new(); + while let Some(before_task_after_dependent) = before_task_after_stream.next().await { + let before_task_after_dependent = before_task_after_dependent?; + let task_ident = + TaskStateIdent::new(&self.tenant, &before_task_after_dependent.target); + // Only care about the predecessors of this task's successor tasks, excluding this task itself. + if before_task_after_dependent.target != task_name { + conditions.push(TxnCondition::eq_value( + task_ident.to_string_key(), + succeeded_value.clone(), + )); + } + if_ops.push(TxnOp::put( + task_ident.to_string_key(), + not_succeeded_value.clone(), + )); + } + + let request = TxnRequest::new(conditions, if_ops).with_else(vec![TxnOp::put( + task_state_key.to_string_key(), + succeeded_value.clone(), + )]); + let reply = self.kv_api.transaction(request).await?; + + if reply.success { + ready_tasks.push(task_before_dependent.target.clone()) + } + } + Ok(Ok(ready_tasks)) + } + + #[async_backtrace::framed] + #[fastrace::trace] + pub async fn clean_task_state(&self, task_name: &str) -> Result<(), TaskApiError> { + let key = TaskStateIdent::new(&self.tenant, task_name); + let req = UpsertPB::delete(key).with(MatchSeq::GE(1)); + let _ = self.kv_api.upsert_pb(&req).await?; + Ok(()) + } + async fn create_task_inner( &self, task: Task, @@ -300,7 +466,9 @@ impl TaskMgr { } } if !task.after.is_empty() { - self.send(TaskMessage::AfterTask(task)).await?; + if let Err(err) = self.update_after(&task.task_name, &task.after).await? { + return Ok(Err(err)); + } } else if task.schedule_options.is_some() && !without_schedule { self.send(TaskMessage::ScheduleTask(task)).await?; } diff --git a/src/query/service/src/interpreters/task/private.rs b/src/query/service/src/interpreters/task/private.rs index 50ed7c43da273..9a5201d3ec217 100644 --- a/src/query/service/src/interpreters/task/private.rs +++ b/src/query/service/src/interpreters/task/private.rs @@ -18,6 +18,7 @@ use chrono::Utc; use databend_common_ast::ast::TaskSql; use databend_common_catalog::table_context::TableContext; use databend_common_cloud_control::task_utils; +use databend_common_exception::ErrorCode; use databend_common_exception::Result; use databend_common_management::task::TaskMgr; use databend_common_meta_app::principal::task::EMPTY_TASK_ID; @@ -33,6 +34,7 @@ use databend_common_users::UserApiProvider; use crate::interpreters::task::TaskInterpreter; use crate::sessions::QueryContext; +use crate::task::TaskService; pub(crate) struct PrivateTaskInterpreter; @@ -87,6 +89,14 @@ impl TaskInterpreter for PrivateTaskInterpreter { } async fn alter_task(&self, _ctx: &Arc, plan: &AlterTaskPlan) -> Result<()> { + if TaskService::instance() + .is_task_executing(&plan.task_name) + .await? + { + return Err(ErrorCode::TaskRunningWhenModifyingAfter( + "Task Running when modifying after", + )); + } UserApiProvider::instance() .task_api(&plan.tenant) .alter_task(&plan.task_name, &plan.alter_options) @@ -108,6 +118,14 @@ impl TaskInterpreter for PrivateTaskInterpreter { } async fn drop_task(&self, _ctx: &Arc, plan: &DropTaskPlan) -> Result<()> { + if TaskService::instance() + .is_task_executing(&plan.task_name) + .await? + { + return Err(ErrorCode::TaskRunningWhenModifyingAfter( + "Task Running when modifying after", + )); + } UserApiProvider::instance() .task_api(&plan.tenant) .drop_task(&plan.task_name) diff --git a/src/query/service/src/task/service.rs b/src/query/service/src/task/service.rs index af9cb60e5201b..a7f42f4275624 100644 --- a/src/query/service/src/task/service.rs +++ b/src/query/service/src/task/service.rs @@ -21,7 +21,6 @@ use std::sync::atomic::Ordering; use std::sync::Arc; use std::time::Duration; -use async_stream::stream; use chrono::DateTime; use chrono::Utc; use chrono_tz::Tz; @@ -59,10 +58,8 @@ use databend_common_meta_types::MetaError; use databend_common_sql::Planner; use databend_common_users::UserApiProvider; use databend_common_users::BUILTIN_ROLE_ACCOUNT_ADMIN; -use futures::Stream; use futures_util::stream::BoxStream; use futures_util::TryStreamExt; -use itertools::Itertools; use log::error; use tokio::time::sleep; use tokio_stream::StreamExt; @@ -136,12 +133,6 @@ impl TaskService { );"; self.execute_sql(None, create_task_run_table).await?; - let create_task_after_table = "CREATE TABLE IF NOT EXISTS system_task.task_after(\ - task_name STRING NOT NULL,\ - next_task STRING NOT NULL\ - );"; - self.execute_sql(None, create_task_after_table).await?; - Ok(()) } @@ -401,11 +392,9 @@ impl TaskService { .update_or_create_task_run(&task_run) .await?; - let mut stream = - Box::pin(task_service.check_next_tasks(&task_name)); - - while let Some(next_task) = stream.next().await { - let next_task = next_task?; + for next_task in + task_mgr.task_succeeded(&task_name).await?? + { let next_task = task_mgr .describe_task(&next_task) .await?? @@ -460,7 +449,8 @@ impl TaskService { token.cancel(); } if task_mgr.accept(&task_key).await? { - self.clean_task_afters(&task_name).await?; + task_mgr.clean_task_state(&task_name).await?; + task_mgr.update_after(&task_name, &[]).await??; } task_mgr .accept(&TaskMessageIdent::new( @@ -469,16 +459,8 @@ impl TaskService { )) .await?; } - TaskMessage::AfterTask(task) => { - if !task_mgr.accept(&task_key).await? { - continue; - } - match task.status { - Status::Suspended => continue, - Status::Started => (), - } - self.update_task_afters(&task).await?; - } + // deprecated + TaskMessage::AfterTask(_) => (), } } Ok(()) @@ -505,6 +487,18 @@ impl TaskService { }))) } + pub async fn is_task_executing(&self, task_name: &str) -> Result { + let blocks = self.execute_sql(None, &format!("SELECT state = 'EXECUTING' from system_task.task_run WHERE task_name = '{task_name}' ORDER BY run_id DESC LIMIT 1")).await?; + Ok(blocks + .first() + .and_then(|block| { + block.columns()[0] + .index(0) + .and_then(|scalar| scalar.as_boolean().cloned()) + }) + .unwrap_or(false)) + } + fn decode(resp: WatchResponse) -> Result> { let Some((key, _, Some(value))) = resp.unpack() else { return Ok(None); @@ -673,89 +667,6 @@ impl TaskService { Ok(()) } - pub fn check_next_tasks<'a>( - &'a self, - task_name: &'a str, - ) -> impl Stream> + '_ { - stream! { - let check = format!(" - WITH latest_task_run AS ( - SELECT - task_name, - state, - completed_at - FROM ( - SELECT - task_name, - state, - completed_at, - ROW_NUMBER() OVER (PARTITION BY task_name ORDER BY completed_at DESC) AS rn - FROM system_task.task_run - ) ranked - WHERE rn = 1 -), -next_task_time AS ( - SELECT - task_name AS next_task, - completed_at - FROM latest_task_run -) -SELECT DISTINCT ta.next_task -FROM system_task.task_after ta -WHERE ta.task_name = '{task_name}' - AND NOT EXISTS ( - SELECT 1 - FROM system_task.task_after ta_dep - LEFT JOIN latest_task_run tr - ON ta_dep.task_name = tr.task_name - LEFT JOIN next_task_time nt - ON ta_dep.next_task = nt.next_task - WHERE ta_dep.next_task = ta.next_task - AND ( - tr.task_name IS NULL - OR tr.state != 'SUCCEEDED' - OR tr.completed_at IS NULL - OR (nt.completed_at IS NOT NULL AND tr.completed_at <= nt.completed_at) - ) - );"); - if let Some(next_task) = self.execute_sql(None, &check).await?.first().and_then(|block| block.columns()[0].index(0).and_then(|scalar| { scalar.as_string().map(|s| s.to_string()) })) { - yield Result::Ok(next_task); - } - } - } - - pub async fn clean_task_afters(&self, task_name: &str) -> Result<()> { - self.execute_sql( - None, - &format!( - "DELETE FROM system_task.task_after WHERE next_task = '{}'", - task_name - ), - ) - .await?; - - Ok(()) - } - - pub async fn update_task_afters(&self, task: &Task) -> Result<()> { - self.clean_task_afters(&task.task_name).await?; - let values = task - .after - .iter() - .map(|after| format!("('{}', '{}')", after, task.task_name)) - .join(", "); - self.execute_sql( - None, - &format!( - "INSERT INTO system_task.task_after (task_name, next_task) VALUES {}", - values - ), - ) - .await?; - - Ok(()) - } - fn task_run2insert(task_run: &TaskRun) -> Result { let task = &task_run.task; From 096d28030df9b1b7dd1d0f5e02a5b17005ca911a Mon Sep 17 00:00:00 2001 From: kould Date: Wed, 30 Jul 2025 11:40:39 +0800 Subject: [PATCH 06/28] chore: rebase --- src/meta/proto-conv/src/task_from_to_protobuf_impl.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs b/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs index 1d19a71c70527..b4066e38c7ce4 100644 --- a/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs +++ b/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs @@ -18,7 +18,7 @@ use databend_common_meta_app::principal as mt; use databend_common_meta_app::principal::task::Status; use databend_common_meta_app::principal::DependentType; use databend_common_protos::pb; -use databend_common_protos::pb::task_message::Message; +use databend_common_protos::pb::task_message::{DeleteTask, Message}; use crate::reader_check_msg; use crate::FromToProto; From ff767ae1929ddaf62009a6e60fec52815fbb1290 Mon Sep 17 00:00:00 2001 From: kould Date: Wed, 30 Jul 2025 11:46:06 +0800 Subject: [PATCH 07/28] chore: codefmt --- src/meta/proto-conv/src/task_from_to_protobuf_impl.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs b/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs index b4066e38c7ce4..2639f35ccd3fb 100644 --- a/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs +++ b/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs @@ -18,7 +18,8 @@ use databend_common_meta_app::principal as mt; use databend_common_meta_app::principal::task::Status; use databend_common_meta_app::principal::DependentType; use databend_common_protos::pb; -use databend_common_protos::pb::task_message::{DeleteTask, Message}; +use databend_common_protos::pb::task_message::DeleteTask; +use databend_common_protos::pb::task_message::Message; use crate::reader_check_msg; use crate::FromToProto; From b909a89fb18cf6c56c8e0635d64302bfb0153034 Mon Sep 17 00:00:00 2001 From: kould Date: Wed, 30 Jul 2025 12:08:44 +0800 Subject: [PATCH 08/28] fix: test_decode_v141_task_state & test_decode_v141_task_dependent --- src/meta/proto-conv/tests/it/v141_task_state.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/meta/proto-conv/tests/it/v141_task_state.rs b/src/meta/proto-conv/tests/it/v141_task_state.rs index c89df8f8d5060..9a08fc59c50d7 100644 --- a/src/meta/proto-conv/tests/it/v141_task_state.rs +++ b/src/meta/proto-conv/tests/it/v141_task_state.rs @@ -19,7 +19,7 @@ use crate::common; #[test] fn test_decode_v141_task_state() -> anyhow::Result<()> { - let task_state_v141 = vec![]; + let task_state_v141 = vec![8, 1, 160, 6, 141, 1, 168, 6, 24]; let want = || mt::TaskState { is_succeeded: true }; common::test_pb_from_to(func_name!(), want())?; @@ -30,7 +30,7 @@ fn test_decode_v141_task_state() -> anyhow::Result<()> { #[test] fn test_decode_v141_task_dependent() -> anyhow::Result<()> { - let task_dependent_v141 = vec![]; + let task_dependent_v141 = vec![10, 1, 97, 18, 1, 99, 160, 6, 141, 1, 168, 6, 24]; let want = || mt::TaskDependent { ty: mt::DependentType::After, From 692087789d9b0491d2602144d9fb16d752c51c4d Mon Sep 17 00:00:00 2001 From: kould Date: Mon, 4 Aug 2025 17:11:06 +0800 Subject: [PATCH 09/28] chore: `update_after` is divided into `add_afters` & `remove_afters` & `clean_task_state_and_dependents` --- src/query/management/src/task/task_mgr.rs | 118 +++++++++++++++------- src/query/service/src/task/service.rs | 3 +- 2 files changed, 80 insertions(+), 41 deletions(-) diff --git a/src/query/management/src/task/task_mgr.rs b/src/query/management/src/task/task_mgr.rs index 404fb06cbe3ae..05dfe2da9f1c7 100644 --- a/src/query/management/src/task/task_mgr.rs +++ b/src/query/management/src/task/task_mgr.rs @@ -12,7 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::collections::HashSet; use std::str::FromStr; use std::sync::Arc; @@ -168,12 +167,7 @@ impl TaskMgr { name: task_name.to_string(), })); } - for after in afters { - if task.after.contains(after) { - continue; - } - task.after.push(after.clone()); - } + return self.add_after(&task.task_name, afters).await; } AlterTaskOptions::RemoveAfter(afters) => { if task.schedule_options.is_some() { @@ -182,7 +176,7 @@ impl TaskMgr { name: task_name.to_string(), })); } - task.after.retain(|task| !afters.contains(task)); + return self.remove_after(&task.task_name, afters).await; } } if let Err(e) = self @@ -268,32 +262,14 @@ impl TaskMgr { #[async_backtrace::framed] #[fastrace::trace] - pub async fn update_after( + pub async fn add_after( &self, task_name: &str, - task_after: &[String], + new_afters: &[String], ) -> Result, TaskApiError> { - let task_after_ident = DirName::new(TaskDependentIdent::new_generic( - &self.tenant, - TaskDependent::new(DependentType::After, task_name.to_string(), "".to_string()), - )); - let mut update_ops = Vec::new(); - - let mut new_afters: HashSet<&String> = task_after.iter().collect(); - let mut remove_afters: Vec = Vec::new(); - let mut task_after_stream = self.kv_api.list_pb_values(&task_after_ident).await?; - - while let Some(after_task_dependent) = task_after_stream.next().await { - let after_task_dependent = after_task_dependent?; - - debug_assert_eq!(after_task_dependent.ty, DependentType::After); - - if !new_afters.remove(&after_task_dependent.target) { - remove_afters.push(after_task_dependent.target.clone()); - } - } let mut check_ops = Vec::with_capacity(new_afters.len()); + for after in new_afters { let after_dependent = TaskDependent::new(DependentType::After, task_name.to_string(), after.clone()); @@ -320,27 +296,91 @@ impl TaskMgr { target: Some(Target::Seq(0)), }); } - for after in remove_afters { - let before_dependent_ident = TaskDependentIdent::new_generic( - &self.tenant, - TaskDependent::new(DependentType::Before, after.clone(), task_name.to_string()), - ); - - update_ops.push(TxnOp::delete(before_dependent_ident.to_string_key())); - } let request = TxnRequest::new(check_ops, update_ops); let reply = self.kv_api.transaction(request).await?; if reply.success { return Err(TaskApiError::SimultaneousUpdateTaskAfter { task_name: task_name.to_string(), - after: task_after.join(","), + after: new_afters.join(","), }); } Ok(Ok(())) } + #[async_backtrace::framed] + #[fastrace::trace] + pub async fn remove_after( + &self, + task_name: &str, + remove_afters: &[String], + ) -> Result, TaskApiError> { + let task_after_dir_ident = DirName::new(TaskDependentIdent::new_generic( + &self.tenant, + TaskDependent::new(DependentType::After, task_name.to_string(), "".to_string()), + )); + let mut task_after_stream = self.kv_api.list_pb_values(&task_after_dir_ident).await?; + let mut update_ops = Vec::new(); + + while let Some(task_after_dependent) = task_after_stream.next().await { + let task_after_dependent = task_after_dependent?; + + debug_assert_eq!(task_after_dependent.ty, DependentType::After); + + if !remove_afters.contains(&task_after_dependent.target) { + continue; + } + let task_after_ident = + TaskDependentIdent::new_generic(&self.tenant, task_after_dependent); + update_ops.push(TxnOp::delete(task_after_ident.to_string_key())); + } + let request = TxnRequest::new(vec![], update_ops); + let _ = self.kv_api.transaction(request).await?; + + Ok(Ok(())) + } + + // Tips: Task Before only cleans up when dropping a task + #[async_backtrace::framed] + #[fastrace::trace] + pub async fn clean_task_state_and_dependents( + &self, + task_name: &str, + ) -> Result, TaskApiError> { + let task_before_dir_ident = DirName::new(TaskDependentIdent::new_generic( + &self.tenant, + TaskDependent::new(DependentType::Before, task_name.to_string(), "".to_string()), + )); + let mut task_before_stream = self.kv_api.list_pb_values(&task_before_dir_ident).await?; + let mut update_ops = Vec::new(); + + while let Some(task_before_dependent) = task_before_stream.next().await { + let task_before_dependent = task_before_dependent?; + debug_assert_eq!(task_before_dependent.ty, DependentType::Before); + + let task_after_dependent = TaskDependent::new( + DependentType::After, + task_before_dependent.target.to_string(), + task_before_dependent.source.to_string(), + ); + + let task_before_ident = + TaskDependentIdent::new_generic(&self.tenant, task_before_dependent); + update_ops.push(TxnOp::delete(task_before_ident.to_string_key())); + + let task_after_ident = + TaskDependentIdent::new_generic(&self.tenant, task_after_dependent); + update_ops.push(TxnOp::delete(task_after_ident.to_string_key())); + } + update_ops.push(TxnOp::delete(TaskStateIdent::new(&self.tenant, task_name).to_string_key())); + + let request = TxnRequest::new(vec![], update_ops); + let _ = self.kv_api.transaction(request).await?; + + Ok(Ok(())) + } + #[async_backtrace::framed] #[fastrace::trace] pub async fn task_succeeded( @@ -466,7 +506,7 @@ impl TaskMgr { } } if !task.after.is_empty() { - if let Err(err) = self.update_after(&task.task_name, &task.after).await? { + if let Err(err) = self.add_after(&task.task_name, &task.after).await? { return Ok(Err(err)); } } else if task.schedule_options.is_some() && !without_schedule { diff --git a/src/query/service/src/task/service.rs b/src/query/service/src/task/service.rs index a7f42f4275624..3af3edf3568a2 100644 --- a/src/query/service/src/task/service.rs +++ b/src/query/service/src/task/service.rs @@ -449,8 +449,7 @@ impl TaskService { token.cancel(); } if task_mgr.accept(&task_key).await? { - task_mgr.clean_task_state(&task_name).await?; - task_mgr.update_after(&task_name, &[]).await??; + task_mgr.clean_task_state_and_dependents(&task_name).await??; } task_mgr .accept(&TaskMessageIdent::new( From 9808a4b2396fe5b09f477e71b69c2afbbdd4a866 Mon Sep 17 00:00:00 2001 From: kould Date: Mon, 4 Aug 2025 17:24:32 +0800 Subject: [PATCH 10/28] chore: codefmt --- src/query/management/src/task/task_mgr.rs | 4 +++- src/query/service/src/task/service.rs | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/query/management/src/task/task_mgr.rs b/src/query/management/src/task/task_mgr.rs index 05dfe2da9f1c7..7d3af78f2df5e 100644 --- a/src/query/management/src/task/task_mgr.rs +++ b/src/query/management/src/task/task_mgr.rs @@ -373,7 +373,9 @@ impl TaskMgr { TaskDependentIdent::new_generic(&self.tenant, task_after_dependent); update_ops.push(TxnOp::delete(task_after_ident.to_string_key())); } - update_ops.push(TxnOp::delete(TaskStateIdent::new(&self.tenant, task_name).to_string_key())); + update_ops.push(TxnOp::delete( + TaskStateIdent::new(&self.tenant, task_name).to_string_key(), + )); let request = TxnRequest::new(vec![], update_ops); let _ = self.kv_api.transaction(request).await?; diff --git a/src/query/service/src/task/service.rs b/src/query/service/src/task/service.rs index 3af3edf3568a2..da8bad024157f 100644 --- a/src/query/service/src/task/service.rs +++ b/src/query/service/src/task/service.rs @@ -449,7 +449,9 @@ impl TaskService { token.cancel(); } if task_mgr.accept(&task_key).await? { - task_mgr.clean_task_state_and_dependents(&task_name).await??; + task_mgr + .clean_task_state_and_dependents(&task_name) + .await??; } task_mgr .accept(&TaskMessageIdent::new( From d1b1d9609182c36a6f8cc8d7fa52fc355663796d Mon Sep 17 00:00:00 2001 From: kould Date: Tue, 5 Aug 2025 15:57:29 +0800 Subject: [PATCH 11/28] refactor: store Task's dependents in the single key --- src/meta/app/src/principal/mod.rs | 3 +- src/meta/app/src/principal/task.rs | 21 +- .../app/src/principal/task_dependent_ident.rs | 10 +- .../src/task_from_to_protobuf_impl.rs | 31 +- .../proto-conv/tests/it/v141_task_state.rs | 36 ++- src/meta/protos/proto/task.proto | 10 +- src/query/management/src/task/task_mgr.rs | 273 ++++++++++++------ tests/task/test-private-task.sh | 58 +++- 8 files changed, 314 insertions(+), 128 deletions(-) diff --git a/src/meta/app/src/principal/mod.rs b/src/meta/app/src/principal/mod.rs index c2a3f32e4bc3f..2f711358dabe7 100644 --- a/src/meta/app/src/principal/mod.rs +++ b/src/meta/app/src/principal/mod.rs @@ -97,7 +97,8 @@ pub use task::ScheduleType; pub use task::State; pub use task::Status; pub use task::Task; -pub use task::TaskDependent; +pub use task::TaskDependentKey; +pub use task::TaskDependentValue; pub use task::TaskMessage; pub use task::TaskRun; pub use task::TaskState; diff --git a/src/meta/app/src/principal/task.rs b/src/meta/app/src/principal/task.rs index a13b52989912f..e0f4694461052 100644 --- a/src/meta/app/src/principal/task.rs +++ b/src/meta/app/src/principal/task.rs @@ -13,6 +13,7 @@ // limitations under the License. use std::collections::BTreeMap; +use std::collections::HashSet; use chrono::DateTime; use chrono::Utc; @@ -185,33 +186,34 @@ pub enum DependentType { } #[derive(Debug, Clone, PartialEq)] -pub struct TaskDependent { +pub struct TaskDependentKey { pub ty: DependentType, pub source: String, - pub target: String, } -impl TaskDependent { - pub fn new(ty: DependentType, source: String, target: String) -> Self { - Self { ty, source, target } +impl TaskDependentKey { + pub fn new(ty: DependentType, source: String) -> Self { + Self { ty, source } } } +#[derive(Debug, Clone, PartialEq)] +pub struct TaskDependentValue(pub HashSet); + mod kvapi_key_impl { use databend_common_meta_kvapi::kvapi; use databend_common_meta_kvapi::kvapi::KeyError; use crate::principal::DependentType; - use crate::principal::TaskDependent; + use crate::principal::TaskDependentKey; - impl kvapi::KeyCodec for TaskDependent { + impl kvapi::KeyCodec for TaskDependentKey { fn encode_key(&self, b: kvapi::KeyBuilder) -> kvapi::KeyBuilder { match self.ty { DependentType::After => b.push_str("After"), DependentType::Before => b.push_str("Before"), } .push_str(self.source.as_str()) - .push_str(self.target.as_str()) } fn decode_key(parser: &mut kvapi::KeyParser) -> Result { @@ -226,9 +228,8 @@ mod kvapi_key_impl { } }; let source = parser.next_str()?; - let target = parser.next_str()?; - Ok(Self { ty, source, target }) + Ok(Self { ty, source }) } } } diff --git a/src/meta/app/src/principal/task_dependent_ident.rs b/src/meta/app/src/principal/task_dependent_ident.rs index f5e60a4dcb938..910ba6f4c0a61 100644 --- a/src/meta/app/src/principal/task_dependent_ident.rs +++ b/src/meta/app/src/principal/task_dependent_ident.rs @@ -14,16 +14,16 @@ use crate::tenant_key::ident::TIdent; -pub type TaskDependentIdent = TIdent; +pub type TaskDependentIdent = TIdent; pub use kvapi_impl::TaskDependentResource; -use crate::principal::TaskDependent; +use crate::principal::TaskDependentKey; mod kvapi_impl { use databend_common_meta_kvapi::kvapi; - use crate::principal::task::TaskDependent; + use crate::principal::task::TaskDependentValue; use crate::principal::task_dependent_ident::TaskDependentIdent; use crate::tenant_key::resource::TenantResource; @@ -32,10 +32,10 @@ mod kvapi_impl { const PREFIX: &'static str = "__fd_task_dependents"; const TYPE: &'static str = "TaskDependentIdent"; const HAS_TENANT: bool = true; - type ValueType = TaskDependent; + type ValueType = TaskDependentValue; } - impl kvapi::Value for TaskDependent { + impl kvapi::Value for TaskDependentValue { type KeyType = TaskDependentIdent; fn dependency_keys(&self, _key: &Self::KeyType) -> impl IntoIterator { diff --git a/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs b/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs index 2639f35ccd3fb..2d69e1e9d75ca 100644 --- a/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs +++ b/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::collections::HashSet; + use chrono::DateTime; use chrono::Utc; use databend_common_meta_app::principal as mt; @@ -197,8 +199,8 @@ impl FromToProto for mt::TaskMessage { } } -impl FromToProto for mt::TaskDependent { - type PB = pb::TaskDependent; +impl FromToProto for mt::TaskDependentKey { + type PB = pb::TaskDependentKey; fn get_pb_ver(p: &Self::PB) -> u64 { p.ver @@ -213,22 +215,41 @@ impl FromToProto for mt::TaskDependent { _ => return Err(Incompatible::new(format!("invalid task type {}", p.ty))), }, source: p.source, - target: p.target, }) } fn to_pb(&self) -> Result { - Ok(pb::TaskDependent { + Ok(pb::TaskDependentKey { ver: VER, min_reader_ver: MIN_READER_VER, source: self.source.clone(), - target: self.target.clone(), ty: self.ty as i32, }) } } +impl FromToProto for mt::TaskDependentValue { + type PB = pb::TaskDependentValue; + + fn get_pb_ver(p: &Self::PB) -> u64 { + p.ver + } + + fn from_pb(p: Self::PB) -> Result + where Self: Sized { + Ok(Self(HashSet::from_iter(p.names))) + } + + fn to_pb(&self) -> Result { + Ok(pb::TaskDependentValue { + ver: VER, + min_reader_ver: MIN_READER_VER, + names: Vec::from_iter(self.0.iter().cloned()), + }) + } +} + impl FromToProto for mt::TaskState { type PB = pb::TaskState; diff --git a/src/meta/proto-conv/tests/it/v141_task_state.rs b/src/meta/proto-conv/tests/it/v141_task_state.rs index 9a08fc59c50d7..6467e43229998 100644 --- a/src/meta/proto-conv/tests/it/v141_task_state.rs +++ b/src/meta/proto-conv/tests/it/v141_task_state.rs @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::collections::HashSet; + use databend_common_meta_app::principal as mt; use fastrace::func_name; @@ -30,15 +32,31 @@ fn test_decode_v141_task_state() -> anyhow::Result<()> { #[test] fn test_decode_v141_task_dependent() -> anyhow::Result<()> { - let task_dependent_v141 = vec![10, 1, 97, 18, 1, 99, 160, 6, 141, 1, 168, 6, 24]; - - let want = || mt::TaskDependent { - ty: mt::DependentType::After, - source: s("a"), - target: s("c"), - }; - common::test_pb_from_to(func_name!(), want())?; - common::test_load_old(func_name!(), task_dependent_v141.as_slice(), 141, want())?; + { + let task_dependent_key_v141 = vec![10, 1, 97, 160, 6, 141, 1, 168, 6, 24]; + let want = || mt::TaskDependentKey { + ty: mt::DependentType::After, + source: s("a"), + }; + common::test_pb_from_to(func_name!(), want())?; + common::test_load_old( + func_name!(), + task_dependent_key_v141.as_slice(), + 141, + want(), + )?; + } + { + let task_dependent_value_v141 = vec![10, 1, 97, 10, 1, 98, 160, 6, 141, 1, 168, 6, 24]; + let want = || mt::TaskDependentValue(HashSet::from([s("a"), s("b")])); + common::test_pb_from_to(func_name!(), want())?; + common::test_load_old( + func_name!(), + task_dependent_value_v141.as_slice(), + 141, + want(), + )?; + } Ok(()) } diff --git a/src/meta/protos/proto/task.proto b/src/meta/protos/proto/task.proto index 38a5e90dbe38a..41c9cf44bef34 100644 --- a/src/meta/protos/proto/task.proto +++ b/src/meta/protos/proto/task.proto @@ -86,7 +86,7 @@ message TaskState { bool is_succeeded = 1; } -message TaskDependent { +message TaskDependentKey { uint64 ver = 100; uint64 min_reader_ver = 101; @@ -95,6 +95,12 @@ message TaskDependent { Before = 1; } string source = 1; - string target = 2; DependentType ty = 3; } + +message TaskDependentValue { + uint64 ver = 100; + uint64 min_reader_ver = 101; + + repeated string names = 1; +} diff --git a/src/query/management/src/task/task_mgr.rs b/src/query/management/src/task/task_mgr.rs index 7d3af78f2df5e..52898cd3dffad 100644 --- a/src/query/management/src/task/task_mgr.rs +++ b/src/query/management/src/task/task_mgr.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::collections::HashSet; use std::str::FromStr; use std::sync::Arc; @@ -22,20 +23,24 @@ use databend_common_ast::ast::AlterTaskOptions; use databend_common_ast::ast::ScheduleOptions; use databend_common_meta_api::kv_pb_api::KVPbApi; use databend_common_meta_api::kv_pb_api::UpsertPB; +use databend_common_meta_api::SchemaApi; use databend_common_meta_app::principal::task; use databend_common_meta_app::principal::task::TaskMessage; use databend_common_meta_app::principal::task::TaskState; use databend_common_meta_app::principal::task_dependent_ident::TaskDependentIdent; +use databend_common_meta_app::principal::task_dependent_ident::TaskDependentResource; use databend_common_meta_app::principal::task_message_ident::TaskMessageIdent; use databend_common_meta_app::principal::task_state_ident::TaskStateIdent; use databend_common_meta_app::principal::DependentType; use databend_common_meta_app::principal::ScheduleType; use databend_common_meta_app::principal::Status; use databend_common_meta_app::principal::Task; -use databend_common_meta_app::principal::TaskDependent; +use databend_common_meta_app::principal::TaskDependentKey; +use databend_common_meta_app::principal::TaskDependentValue; use databend_common_meta_app::principal::TaskIdent; use databend_common_meta_app::schema::CreateOption; use databend_common_meta_app::tenant::Tenant; +use databend_common_meta_app::tenant_key::ident::TIdent; use databend_common_meta_kvapi::kvapi; use databend_common_meta_kvapi::kvapi::DirName; use databend_common_meta_kvapi::kvapi::Key; @@ -48,7 +53,6 @@ use databend_common_meta_types::TxnOp; use databend_common_meta_types::TxnRequest; use databend_common_meta_types::With; use databend_common_proto_conv::FromToProto; -use futures::StreamExt; use futures::TryStreamExt; use prost::Message; @@ -270,36 +274,37 @@ impl TaskMgr { let mut update_ops = Vec::new(); let mut check_ops = Vec::with_capacity(new_afters.len()); - for after in new_afters { - let after_dependent = - TaskDependent::new(DependentType::After, task_name.to_string(), after.clone()); - let after_dependent_ident = - TaskDependentIdent::new_generic(&self.tenant, after_dependent.clone()); + let after_dependent = TaskDependentKey::new(DependentType::After, task_name.to_string()); + let after_dependent_ident = + TaskDependentIdent::new_generic(&self.tenant, after_dependent.clone()); - let before_dependent = - TaskDependent::new(DependentType::Before, after.clone(), task_name.to_string()); - let before_dependent_ident = - TaskDependentIdent::new_generic(&self.tenant, before_dependent.clone()); + self.check_and_set( + &mut check_ops, + &mut update_ops, + new_afters, + &after_dependent_ident, + true, + ) + .await?; - update_ops.push(TxnOp::put( - after_dependent_ident.to_string_key(), - after_dependent.to_pb()?.encode_to_vec(), - )); - update_ops.push(TxnOp::put( - before_dependent_ident.to_string_key(), - before_dependent.to_pb()?.encode_to_vec(), - )); - // Ensure that the related tasks still exist - check_ops.push(TxnCondition { - key: TaskStateIdent::new(&self.tenant, after).to_string_key(), - expected: ConditionResult::Ge as i32, - target: Some(Target::Seq(0)), - }); + for after in new_afters { + let before_dependent = TaskDependentKey::new(DependentType::Before, after.clone()); + let before_dependent_ident = + TaskDependentIdent::new_generic(&self.tenant, before_dependent); + + self.check_and_set( + &mut check_ops, + &mut update_ops, + &[task_name.to_string()], + &before_dependent_ident, + true, + ) + .await?; } let request = TxnRequest::new(check_ops, update_ops); let reply = self.kv_api.transaction(request).await?; - if reply.success { + if !reply.success { return Err(TaskApiError::SimultaneousUpdateTaskAfter { task_name: task_name.to_string(), after: new_afters.join(","), @@ -316,27 +321,45 @@ impl TaskMgr { task_name: &str, remove_afters: &[String], ) -> Result, TaskApiError> { - let task_after_dir_ident = DirName::new(TaskDependentIdent::new_generic( - &self.tenant, - TaskDependent::new(DependentType::After, task_name.to_string(), "".to_string()), - )); - let mut task_after_stream = self.kv_api.list_pb_values(&task_after_dir_ident).await?; let mut update_ops = Vec::new(); + let mut check_ops = Vec::with_capacity(remove_afters.len()); + + let after_dependent = TaskDependentKey::new(DependentType::After, task_name.to_string()); + let after_dependent_ident = + TaskDependentIdent::new_generic(&self.tenant, after_dependent.clone()); + + self.check_and_set( + &mut check_ops, + &mut update_ops, + remove_afters, + &after_dependent_ident, + false, + ) + .await?; + + for after in remove_afters { + let before_dependent = TaskDependentKey::new(DependentType::Before, after.clone()); + let before_dependent_ident = + TaskDependentIdent::new_generic(&self.tenant, before_dependent.clone()); - while let Some(task_after_dependent) = task_after_stream.next().await { - let task_after_dependent = task_after_dependent?; - - debug_assert_eq!(task_after_dependent.ty, DependentType::After); + self.check_and_set( + &mut check_ops, + &mut update_ops, + &[task_name.to_string()], + &before_dependent_ident, + false, + ) + .await?; + } + let request = TxnRequest::new(check_ops, update_ops); + let reply = self.kv_api.transaction(request).await?; - if !remove_afters.contains(&task_after_dependent.target) { - continue; - } - let task_after_ident = - TaskDependentIdent::new_generic(&self.tenant, task_after_dependent); - update_ops.push(TxnOp::delete(task_after_ident.to_string_key())); + if !reply.success { + return Err(TaskApiError::SimultaneousUpdateTaskAfter { + task_name: task_name.to_string(), + after: remove_afters.join(","), + }); } - let request = TxnRequest::new(vec![], update_ops); - let _ = self.kv_api.transaction(request).await?; Ok(Ok(())) } @@ -348,31 +371,28 @@ impl TaskMgr { &self, task_name: &str, ) -> Result, TaskApiError> { - let task_before_dir_ident = DirName::new(TaskDependentIdent::new_generic( - &self.tenant, - TaskDependent::new(DependentType::Before, task_name.to_string(), "".to_string()), - )); - let mut task_before_stream = self.kv_api.list_pb_values(&task_before_dir_ident).await?; + let mut check_ops = Vec::new(); let mut update_ops = Vec::new(); - while let Some(task_before_dependent) = task_before_stream.next().await { - let task_before_dependent = task_before_dependent?; - debug_assert_eq!(task_before_dependent.ty, DependentType::Before); - - let task_after_dependent = TaskDependent::new( - DependentType::After, - task_before_dependent.target.to_string(), - task_before_dependent.source.to_string(), - ); - - let task_before_ident = - TaskDependentIdent::new_generic(&self.tenant, task_before_dependent); - update_ops.push(TxnOp::delete(task_before_ident.to_string_key())); + self.clean_related_dependent(&task_name, &mut check_ops, &mut update_ops, true) + .await?; + self.clean_related_dependent(&task_name, &mut check_ops, &mut update_ops, false) + .await?; - let task_after_ident = - TaskDependentIdent::new_generic(&self.tenant, task_after_dependent); - update_ops.push(TxnOp::delete(task_after_ident.to_string_key())); - } + update_ops.push(TxnOp::delete( + TaskDependentIdent::new_generic( + &self.tenant, + TaskDependentKey::new(DependentType::Before, task_name.to_string()), + ) + .to_string_key(), + )); + update_ops.push(TxnOp::delete( + TaskDependentIdent::new_generic( + &self.tenant, + TaskDependentKey::new(DependentType::After, task_name.to_string()), + ) + .to_string_key(), + )); update_ops.push(TxnOp::delete( TaskStateIdent::new(&self.tenant, task_name).to_string_key(), )); @@ -389,10 +409,10 @@ impl TaskMgr { &self, task_name: &str, ) -> Result, TaskError>, TaskApiError> { - let task_before_ident = DirName::new(TaskDependentIdent::new_generic( + let task_before_ident = TaskDependentIdent::new_generic( &self.tenant, - TaskDependent::new(DependentType::Before, task_name.to_string(), "".to_string()), - )); + TaskDependentKey::new(DependentType::Before, task_name.to_string()), + ); let task_state_key = TaskStateIdent::new(&self.tenant, task_name); let succeeded_value = TaskState { is_succeeded: true }.to_pb()?.encode_to_vec(); let not_succeeded_value = TaskState { @@ -401,31 +421,27 @@ impl TaskMgr { .to_pb()? .encode_to_vec(); + let Some(task_before_dependent) = self.kv_api.get_pb(&task_before_ident).await? else { + return Ok(Ok(Vec::new())); + }; let mut ready_tasks = Vec::new(); - let mut task_before_stream = self.kv_api.list_pb_values(&task_before_ident).await?; - - while let Some(task_before_dependent) = task_before_stream.next().await { - let task_before_dependent = task_before_dependent?; - let before_task_after_ident = DirName::new(TaskDependentIdent::new_generic( + for task_before_target in task_before_dependent.0.iter() { + let target_after_ident = TaskDependentIdent::new_generic( &self.tenant, - TaskDependent::new( - DependentType::After, - task_before_dependent.target.to_string(), - "".to_string(), - ), - )); - let mut before_task_after_stream = - self.kv_api.list_pb_values(&before_task_after_ident).await?; + TaskDependentKey::new(DependentType::After, task_before_target.to_string()), + ); + let Some(target_after_dependent) = self.kv_api.get(&target_after_ident).await? else { + continue; + }; let mut conditions = Vec::new(); let mut if_ops = Vec::new(); - while let Some(before_task_after_dependent) = before_task_after_stream.next().await { - let before_task_after_dependent = before_task_after_dependent?; - let task_ident = - TaskStateIdent::new(&self.tenant, &before_task_after_dependent.target); + + for target_after_task in target_after_dependent.0.iter() { + let task_ident = TaskStateIdent::new(&self.tenant, target_after_task); // Only care about the predecessors of this task's successor tasks, excluding this task itself. - if before_task_after_dependent.target != task_name { + if target_after_task != task_name { conditions.push(TxnCondition::eq_value( task_ident.to_string_key(), succeeded_value.clone(), @@ -436,7 +452,6 @@ impl TaskMgr { not_succeeded_value.clone(), )); } - let request = TxnRequest::new(conditions, if_ops).with_else(vec![TxnOp::put( task_state_key.to_string_key(), succeeded_value.clone(), @@ -444,7 +459,7 @@ impl TaskMgr { let reply = self.kv_api.transaction(request).await?; if reply.success { - ready_tasks.push(task_before_dependent.target.clone()) + ready_tasks.push(task_before_target.clone()) } } Ok(Ok(ready_tasks)) @@ -518,6 +533,88 @@ impl TaskMgr { Ok(Ok(())) } + async fn clean_related_dependent( + &self, + task_name: &&str, + mut check_ops: &mut Vec, + mut update_ops: &mut Vec, + is_after: bool, + ) -> Result<(), TaskApiError> { + let (self_dependent, other_dependent) = if is_after { + (DependentType::After, DependentType::Before) + } else { + (DependentType::Before, DependentType::After) + }; + + let task_ident = TaskDependentIdent::new_generic( + &self.tenant, + TaskDependentKey::new(self_dependent, task_name.to_string()), + ); + if let Some(task_dependent) = self.kv_api.get(&task_ident).await? { + for dependent_target in task_dependent.0.iter() { + let target_key = TaskDependentKey::new(other_dependent, dependent_target.clone()); + let target_ident = + TaskDependentIdent::new_generic(&self.tenant, target_key.clone()); + + self.check_and_set( + check_ops, + update_ops, + &[task_name.to_string()], + &target_ident, + false, + ) + .await?; + } + } + Ok(()) + } + + async fn check_and_set( + &self, + check_ops: &mut Vec, + update_ops: &mut Vec, + task_names: &[String], + dependent_ident: &TIdent, + is_add: bool, + ) -> Result<(), TaskApiError> { + match self.kv_api.get(dependent_ident).await? { + None => { + check_ops.push(TxnCondition { + key: dependent_ident.to_string_key(), + expected: ConditionResult::Eq as i32, + target: Some(Target::Seq(0)), + }); + + if is_add { + update_ops.push(TxnOp::put( + dependent_ident.to_string_key(), + TaskDependentValue(HashSet::from_iter(task_names.iter().cloned())) + .to_pb()? + .encode_to_vec(), + )); + } + } + Some(mut old_dependent) => { + check_ops.push(TxnCondition::eq_value( + dependent_ident.to_string_key(), + old_dependent.to_pb()?.encode_to_vec(), + )); + + if is_add { + old_dependent.0.extend(task_names.iter().cloned()); + } else { + old_dependent.0.retain(|name| !task_names.contains(name)); + } + + update_ops.push(TxnOp::put( + dependent_ident.to_string_key(), + old_dependent.to_pb()?.encode_to_vec(), + )); + } + } + Ok(()) + } + pub fn make_schedule_options(opt: ScheduleOptions) -> task::ScheduleOptions { match opt { ScheduleOptions::IntervalSecs(secs, ms) => { diff --git a/tests/task/test-private-task.sh b/tests/task/test-private-task.sh index 0c31108312969..8e66447699e5b 100644 --- a/tests/task/test-private-task.sh +++ b/tests/task/test-private-task.sh @@ -14,13 +14,13 @@ rm -rf .databend echo "Starting Databend Query cluster with 2 nodes enable private task" -for node in 1 2; do - CONFIG_FILE="./scripts/ci/deploy/config/databend-query-node-${node}.toml" - - echo "Appending history table config to node-${node}" - cat ./tests/task/private_task.toml >> "$CONFIG_FILE" - sed -i '/^cloud_control_grpc_server_address/d' $CONFIG_FILE -done +#for node in 1 2; do +# CONFIG_FILE="./scripts/ci/deploy/config/databend-query-node-${node}.toml" +# +# echo "Appending history table config to node-${node}" +# cat ./tests/task/private_task.toml >> "$CONFIG_FILE" +# sed -i '/^cloud_control_grpc_server_address/d' $CONFIG_FILE +#done # Start meta cluster (3 nodes - needed for HA) echo 'Start Meta service HA cluster(3 nodes)...' @@ -149,6 +149,48 @@ else exit 1 fi +response=$(curl -s -u root: -XPOST "http://localhost:8000/v1/query" -H 'Content-Type: application/json' -d "{\"sql\": \"ALTER TASK my_task_3 REMOVE AFTER 'my_task_2'\"}") +alter_task_query_id=$(echo $response | jq -r '.id') +echo "ALTER Task 3 ID: $alter_task_query_id" + +sleep 5 + +response=$(curl -s -u root: -XPOST "http://localhost:8000/v1/query" -H 'Content-Type: application/json' -d "{\"sql\": \"EXECUTE TASK my_task_1\"}") +execute_task_1_query_id=$(echo $response | jq -r '.id') +echo "Execute Task 1 ID: $execute_task_1_query_id" + +sleep 15 + +response=$(curl -s -u root: -XPOST "http://localhost:8000/v1/query" -H 'Content-Type: application/json' -d "{\"sql\": \"SELECT c1 FROM t1 ORDER BY c1\"}") + +actual=$(echo "$response" | jq -c '.data') +expected='[["0"],["0"],["0"],["1"],["1"],["1"],["2"],["2"],["2"]]' + +if [ "$actual" = "$expected" ]; then + echo "✅ Query result matches expected" +else + echo "❌ Mismatch" + echo "Expected: $expected" + echo "Actual : $actual" + exit 1 +fi + +sleep 30 + +response=$(curl -s -u root: -XPOST "http://localhost:8000/v1/query" -H 'Content-Type: application/json' -d "{\"sql\": \"SELECT c1 FROM t1 ORDER BY c1\"}") + +actual=$(echo "$response" | jq -c '.data') +expected='[["0"],["0"],["0"],["1"],["1"],["1"],["1"],["2"],["2"],["2"]]' + +if [ "$actual" = "$expected" ]; then + echo "✅ Query result matches expected" +else + echo "❌ Mismatch" + echo "Expected: $expected" + echo "Actual : $actual" + exit 1 +fi + # Test whether the schedule can be restored after restart killall -9 databend-query || true @@ -172,7 +214,7 @@ sleep 45 response9=$(curl -s -u root: -XPOST "http://localhost:8000/v1/query" -H 'Content-Type: application/json' -d "{\"sql\": \"SELECT c1 FROM t1 ORDER BY c1\"}") actual=$(echo "$response9" | jq -c '.data') -expected='[["0"],["0"],["1"],["1"],["1"],["2"],["2"]]' +expected='[["0"],["0"],["0"],["1"],["1"],["1"],["1"],["1"],["2"],["2"],["2"]]' if [ "$actual" = "$expected" ]; then echo "✅ Query result matches expected" From 309b5f4a58ee8966e48d8cc1d7a13fc769552f56 Mon Sep 17 00:00:00 2001 From: kould Date: Tue, 5 Aug 2025 16:03:54 +0800 Subject: [PATCH 12/28] chore: codefmt --- src/query/management/src/task/task_mgr.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/query/management/src/task/task_mgr.rs b/src/query/management/src/task/task_mgr.rs index 52898cd3dffad..18edbe9f2b52e 100644 --- a/src/query/management/src/task/task_mgr.rs +++ b/src/query/management/src/task/task_mgr.rs @@ -536,8 +536,8 @@ impl TaskMgr { async fn clean_related_dependent( &self, task_name: &&str, - mut check_ops: &mut Vec, - mut update_ops: &mut Vec, + check_ops: &mut Vec, + update_ops: &mut Vec, is_after: bool, ) -> Result<(), TaskApiError> { let (self_dependent, other_dependent) = if is_after { From 5a9de35c740c96af18ea3f3e977e7d35842ba004 Mon Sep 17 00:00:00 2001 From: kould Date: Tue, 5 Aug 2025 16:33:29 +0800 Subject: [PATCH 13/28] chore: codefmt --- tests/task/test-private-task.sh | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/task/test-private-task.sh b/tests/task/test-private-task.sh index 8e66447699e5b..4deaeeb00d187 100644 --- a/tests/task/test-private-task.sh +++ b/tests/task/test-private-task.sh @@ -14,13 +14,13 @@ rm -rf .databend echo "Starting Databend Query cluster with 2 nodes enable private task" -#for node in 1 2; do -# CONFIG_FILE="./scripts/ci/deploy/config/databend-query-node-${node}.toml" -# -# echo "Appending history table config to node-${node}" -# cat ./tests/task/private_task.toml >> "$CONFIG_FILE" -# sed -i '/^cloud_control_grpc_server_address/d' $CONFIG_FILE -#done +for node in 1 2; do + CONFIG_FILE="./scripts/ci/deploy/config/databend-query-node-${node}.toml" + + echo "Appending history table config to node-${node}" + cat ./tests/task/private_task.toml >> "$CONFIG_FILE" + sed -i '/^cloud_control_grpc_server_address/d' $CONFIG_FILE +done # Start meta cluster (3 nodes - needed for HA) echo 'Start Meta service HA cluster(3 nodes)...' From 7ef31317d1658b2d1a82f4f764ff3960d2a82ed7 Mon Sep 17 00:00:00 2001 From: kould Date: Wed, 6 Aug 2025 11:00:14 +0800 Subject: [PATCH 14/28] chore: codefmt --- src/meta/app/src/principal/task.rs | 4 +- .../src/task_from_to_protobuf_impl.rs | 35 +------- src/meta/protos/proto/task.proto | 4 +- src/query/management/src/task/task_mgr.rs | 80 ++++++++++++------- 4 files changed, 55 insertions(+), 68 deletions(-) diff --git a/src/meta/app/src/principal/task.rs b/src/meta/app/src/principal/task.rs index e0f4694461052..0f9a754199e93 100644 --- a/src/meta/app/src/principal/task.rs +++ b/src/meta/app/src/principal/task.rs @@ -13,7 +13,7 @@ // limitations under the License. use std::collections::BTreeMap; -use std::collections::HashSet; +use std::collections::BTreeSet; use chrono::DateTime; use chrono::Utc; @@ -198,7 +198,7 @@ impl TaskDependentKey { } #[derive(Debug, Clone, PartialEq)] -pub struct TaskDependentValue(pub HashSet); +pub struct TaskDependentValue(pub BTreeSet); mod kvapi_key_impl { use databend_common_meta_kvapi::kvapi; diff --git a/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs b/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs index 2d69e1e9d75ca..3cc62a1b9b1c2 100644 --- a/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs +++ b/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs @@ -12,13 +12,12 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::collections::HashSet; +use std::collections::BTreeSet; use chrono::DateTime; use chrono::Utc; use databend_common_meta_app::principal as mt; use databend_common_meta_app::principal::task::Status; -use databend_common_meta_app::principal::DependentType; use databend_common_protos::pb; use databend_common_protos::pb::task_message::DeleteTask; use databend_common_protos::pb::task_message::Message; @@ -199,36 +198,6 @@ impl FromToProto for mt::TaskMessage { } } -impl FromToProto for mt::TaskDependentKey { - type PB = pb::TaskDependentKey; - - fn get_pb_ver(p: &Self::PB) -> u64 { - p.ver - } - - fn from_pb(p: Self::PB) -> Result - where Self: Sized { - Ok(Self { - ty: match p.ty { - 0 => DependentType::After, - 1 => DependentType::Before, - _ => return Err(Incompatible::new(format!("invalid task type {}", p.ty))), - }, - source: p.source, - }) - } - - fn to_pb(&self) -> Result { - Ok(pb::TaskDependentKey { - ver: VER, - min_reader_ver: MIN_READER_VER, - - source: self.source.clone(), - ty: self.ty as i32, - }) - } -} - impl FromToProto for mt::TaskDependentValue { type PB = pb::TaskDependentValue; @@ -238,7 +207,7 @@ impl FromToProto for mt::TaskDependentValue { fn from_pb(p: Self::PB) -> Result where Self: Sized { - Ok(Self(HashSet::from_iter(p.names))) + Ok(Self(BTreeSet::from_iter(p.names))) } fn to_pb(&self) -> Result { diff --git a/src/meta/protos/proto/task.proto b/src/meta/protos/proto/task.proto index 41c9cf44bef34..638636644f441 100644 --- a/src/meta/protos/proto/task.proto +++ b/src/meta/protos/proto/task.proto @@ -94,8 +94,8 @@ message TaskDependentKey { After = 0; Before = 1; } - string source = 1; - DependentType ty = 3; + DependentType ty = 1; + string source = 2; } message TaskDependentValue { diff --git a/src/query/management/src/task/task_mgr.rs b/src/query/management/src/task/task_mgr.rs index 18edbe9f2b52e..99c8a70c45a7f 100644 --- a/src/query/management/src/task/task_mgr.rs +++ b/src/query/management/src/task/task_mgr.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::collections::HashSet; +use std::collections::BTreeSet; use std::str::FromStr; use std::sync::Arc; @@ -28,7 +28,6 @@ use databend_common_meta_app::principal::task; use databend_common_meta_app::principal::task::TaskMessage; use databend_common_meta_app::principal::task::TaskState; use databend_common_meta_app::principal::task_dependent_ident::TaskDependentIdent; -use databend_common_meta_app::principal::task_dependent_ident::TaskDependentResource; use databend_common_meta_app::principal::task_message_ident::TaskMessageIdent; use databend_common_meta_app::principal::task_state_ident::TaskStateIdent; use databend_common_meta_app::principal::DependentType; @@ -40,7 +39,6 @@ use databend_common_meta_app::principal::TaskDependentValue; use databend_common_meta_app::principal::TaskIdent; use databend_common_meta_app::schema::CreateOption; use databend_common_meta_app::tenant::Tenant; -use databend_common_meta_app::tenant_key::ident::TIdent; use databend_common_meta_kvapi::kvapi; use databend_common_meta_kvapi::kvapi::DirName; use databend_common_meta_kvapi::kvapi::Key; @@ -278,12 +276,11 @@ impl TaskMgr { let after_dependent_ident = TaskDependentIdent::new_generic(&self.tenant, after_dependent.clone()); - self.check_and_set( + self.check_and_add( &mut check_ops, &mut update_ops, new_afters, &after_dependent_ident, - true, ) .await?; @@ -292,12 +289,11 @@ impl TaskMgr { let before_dependent_ident = TaskDependentIdent::new_generic(&self.tenant, before_dependent); - self.check_and_set( + self.check_and_add( &mut check_ops, &mut update_ops, &[task_name.to_string()], &before_dependent_ident, - true, ) .await?; } @@ -328,12 +324,11 @@ impl TaskMgr { let after_dependent_ident = TaskDependentIdent::new_generic(&self.tenant, after_dependent.clone()); - self.check_and_set( + self.check_and_remove( &mut check_ops, &mut update_ops, remove_afters, &after_dependent_ident, - false, ) .await?; @@ -342,12 +337,11 @@ impl TaskMgr { let before_dependent_ident = TaskDependentIdent::new_generic(&self.tenant, before_dependent.clone()); - self.check_and_set( + self.check_and_remove( &mut check_ops, &mut update_ops, &[task_name.to_string()], &before_dependent_ident, - false, ) .await?; } @@ -556,12 +550,11 @@ impl TaskMgr { let target_ident = TaskDependentIdent::new_generic(&self.tenant, target_key.clone()); - self.check_and_set( + self.check_and_remove( check_ops, update_ops, &[task_name.to_string()], &target_ident, - false, ) .await?; } @@ -569,15 +562,14 @@ impl TaskMgr { Ok(()) } - async fn check_and_set( + async fn check_and_add( &self, check_ops: &mut Vec, update_ops: &mut Vec, task_names: &[String], - dependent_ident: &TIdent, - is_add: bool, + dependent_ident: &TaskDependentIdent, ) -> Result<(), TaskApiError> { - match self.kv_api.get(dependent_ident).await? { + match self.kv_api.get_pb(dependent_ident).await? { None => { check_ops.push(TxnCondition { key: dependent_ident.to_string_key(), @@ -585,26 +577,52 @@ impl TaskMgr { target: Some(Target::Seq(0)), }); - if is_add { - update_ops.push(TxnOp::put( - dependent_ident.to_string_key(), - TaskDependentValue(HashSet::from_iter(task_names.iter().cloned())) - .to_pb()? - .encode_to_vec(), - )); - } + update_ops.push(TxnOp::put( + dependent_ident.to_string_key(), + TaskDependentValue(BTreeSet::from_iter(task_names.iter().cloned())) + .to_pb()? + .encode_to_vec(), + )); } Some(mut old_dependent) => { - check_ops.push(TxnCondition::eq_value( + check_ops.push(TxnCondition::eq_seq( + dependent_ident.to_string_key(), + old_dependent.seq, + )); + + old_dependent.0.extend(task_names.iter().cloned()); + + update_ops.push(TxnOp::put( dependent_ident.to_string_key(), old_dependent.to_pb()?.encode_to_vec(), )); + } + } + Ok(()) + } - if is_add { - old_dependent.0.extend(task_names.iter().cloned()); - } else { - old_dependent.0.retain(|name| !task_names.contains(name)); - } + async fn check_and_remove( + &self, + check_ops: &mut Vec, + update_ops: &mut Vec, + task_names: &[String], + dependent_ident: &TaskDependentIdent, + ) -> Result<(), TaskApiError> { + match self.kv_api.get_pb(dependent_ident).await? { + None => { + check_ops.push(TxnCondition { + key: dependent_ident.to_string_key(), + expected: ConditionResult::Eq as i32, + target: Some(Target::Seq(0)), + }); + } + Some(mut old_dependent) => { + check_ops.push(TxnCondition::eq_seq( + dependent_ident.to_string_key(), + old_dependent.seq, + )); + + old_dependent.0.retain(|name| !task_names.contains(name)); update_ops.push(TxnOp::put( dependent_ident.to_string_key(), From ea3f1bf3013d8e43d5af5cbd4c9d2fd6e5dfce86 Mon Sep 17 00:00:00 2001 From: kould Date: Wed, 6 Aug 2025 11:25:02 +0800 Subject: [PATCH 15/28] chore: codefmt --- .../proto-conv/tests/it/v141_task_state.rs | 36 ++++++------------- 1 file changed, 10 insertions(+), 26 deletions(-) diff --git a/src/meta/proto-conv/tests/it/v141_task_state.rs b/src/meta/proto-conv/tests/it/v141_task_state.rs index 6467e43229998..df6f0d8458fd6 100644 --- a/src/meta/proto-conv/tests/it/v141_task_state.rs +++ b/src/meta/proto-conv/tests/it/v141_task_state.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::collections::HashSet; +use std::collections::BTreeSet; use databend_common_meta_app::principal as mt; use fastrace::func_name; @@ -32,31 +32,15 @@ fn test_decode_v141_task_state() -> anyhow::Result<()> { #[test] fn test_decode_v141_task_dependent() -> anyhow::Result<()> { - { - let task_dependent_key_v141 = vec![10, 1, 97, 160, 6, 141, 1, 168, 6, 24]; - let want = || mt::TaskDependentKey { - ty: mt::DependentType::After, - source: s("a"), - }; - common::test_pb_from_to(func_name!(), want())?; - common::test_load_old( - func_name!(), - task_dependent_key_v141.as_slice(), - 141, - want(), - )?; - } - { - let task_dependent_value_v141 = vec![10, 1, 97, 10, 1, 98, 160, 6, 141, 1, 168, 6, 24]; - let want = || mt::TaskDependentValue(HashSet::from([s("a"), s("b")])); - common::test_pb_from_to(func_name!(), want())?; - common::test_load_old( - func_name!(), - task_dependent_value_v141.as_slice(), - 141, - want(), - )?; - } + let task_dependent_value_v141 = vec![10, 1, 97, 10, 1, 98, 160, 6, 141, 1, 168, 6, 24]; + let want = || mt::TaskDependentValue(BTreeSet::from([s("a"), s("b")])); + common::test_pb_from_to(func_name!(), want())?; + common::test_load_old( + func_name!(), + task_dependent_value_v141.as_slice(), + 141, + want(), + )?; Ok(()) } From b1ea18ea2d999e5c77ce42151f4bb00504ef50a5 Mon Sep 17 00:00:00 2001 From: kould Date: Wed, 6 Aug 2025 15:37:36 +0800 Subject: [PATCH 16/28] chore: codefmt --- src/query/management/src/task/task_mgr.rs | 237 ++++++++++++---------- 1 file changed, 127 insertions(+), 110 deletions(-) diff --git a/src/query/management/src/task/task_mgr.rs b/src/query/management/src/task/task_mgr.rs index 99c8a70c45a7f..d07d38549bb8e 100644 --- a/src/query/management/src/task/task_mgr.rs +++ b/src/query/management/src/task/task_mgr.rs @@ -275,27 +275,67 @@ impl TaskMgr { let after_dependent = TaskDependentKey::new(DependentType::After, task_name.to_string()); let after_dependent_ident = TaskDependentIdent::new_generic(&self.tenant, after_dependent.clone()); + let after_key = after_dependent_ident.to_string_key(); - self.check_and_add( - &mut check_ops, - &mut update_ops, - new_afters, - &after_dependent_ident, - ) - .await?; + match self.kv_api.get_pb(&after_dependent_ident).await? { + None => { + check_ops.push(TxnCondition { + key: after_key.clone(), + expected: ConditionResult::Eq as i32, + target: Some(Target::Seq(0)), + }); + + update_ops.push(TxnOp::put( + after_key, + TaskDependentValue(BTreeSet::from_iter(new_afters.iter().cloned())) + .to_pb()? + .encode_to_vec(), + )); + } + Some(mut old_dependent) => { + check_ops.push(TxnCondition::eq_seq(after_key.clone(), old_dependent.seq)); + + old_dependent.0.extend(new_afters.iter().cloned()); + + update_ops.push(TxnOp::put( + after_key, + old_dependent.to_pb()?.encode_to_vec(), + )); + } + } for after in new_afters { let before_dependent = TaskDependentKey::new(DependentType::Before, after.clone()); let before_dependent_ident = TaskDependentIdent::new_generic(&self.tenant, before_dependent); + let before_key = before_dependent_ident.to_string_key(); + + match self.kv_api.get_pb(&before_dependent_ident).await? { + None => { + check_ops.push(TxnCondition { + key: before_key.clone(), + expected: ConditionResult::Eq as i32, + target: Some(Target::Seq(0)), + }); + + update_ops.push(TxnOp::put( + before_key, + TaskDependentValue(BTreeSet::from_iter([task_name.to_string()])) + .to_pb()? + .encode_to_vec(), + )); + } + Some(mut old_dependent) => { + check_ops.push(TxnCondition::eq_seq(before_key.clone(), old_dependent.seq)); - self.check_and_add( - &mut check_ops, - &mut update_ops, - &[task_name.to_string()], - &before_dependent_ident, - ) - .await?; + old_dependent.0.insert(task_name.to_string()); + + update_ops.push(TxnOp::put( + before_key, + old_dependent.to_pb()?.encode_to_vec(), + )); + } + } } let request = TxnRequest::new(check_ops, update_ops); let reply = self.kv_api.transaction(request).await?; @@ -323,27 +363,53 @@ impl TaskMgr { let after_dependent = TaskDependentKey::new(DependentType::After, task_name.to_string()); let after_dependent_ident = TaskDependentIdent::new_generic(&self.tenant, after_dependent.clone()); + let after_key = after_dependent_ident.to_string_key(); - self.check_and_remove( - &mut check_ops, - &mut update_ops, - remove_afters, - &after_dependent_ident, - ) - .await?; + match self.kv_api.get_pb(&after_dependent_ident).await? { + None => { + check_ops.push(TxnCondition { + key: after_key.clone(), + expected: ConditionResult::Eq as i32, + target: Some(Target::Seq(0)), + }); + } + Some(mut old_dependent) => { + check_ops.push(TxnCondition::eq_seq(after_key.clone(), old_dependent.seq)); + + old_dependent.0.retain(|name| !remove_afters.contains(name)); + + update_ops.push(TxnOp::put( + after_key, + old_dependent.to_pb()?.encode_to_vec(), + )); + } + } for after in remove_afters { let before_dependent = TaskDependentKey::new(DependentType::Before, after.clone()); let before_dependent_ident = - TaskDependentIdent::new_generic(&self.tenant, before_dependent.clone()); + TaskDependentIdent::new_generic(&self.tenant, before_dependent); + let before_key = before_dependent_ident.to_string_key(); + + match self.kv_api.get_pb(&before_dependent_ident).await? { + None => { + check_ops.push(TxnCondition { + key: before_key.clone(), + expected: ConditionResult::Eq as i32, + target: Some(Target::Seq(0)), + }); + } + Some(mut old_dependent) => { + check_ops.push(TxnCondition::eq_seq(before_key.clone(), old_dependent.seq)); - self.check_and_remove( - &mut check_ops, - &mut update_ops, - &[task_name.to_string()], - &before_dependent_ident, - ) - .await?; + old_dependent.0.remove(task_name); + + update_ops.push(TxnOp::put( + before_key, + old_dependent.to_pb()?.encode_to_vec(), + )); + } + } } let request = TxnRequest::new(check_ops, update_ops); let reply = self.kv_api.transaction(request).await?; @@ -368,9 +434,9 @@ impl TaskMgr { let mut check_ops = Vec::new(); let mut update_ops = Vec::new(); - self.clean_related_dependent(&task_name, &mut check_ops, &mut update_ops, true) + self.clean_related_dependent(task_name, &mut check_ops, &mut update_ops, true) .await?; - self.clean_related_dependent(&task_name, &mut check_ops, &mut update_ops, false) + self.clean_related_dependent(task_name, &mut check_ops, &mut update_ops, false) .await?; update_ops.push(TxnOp::delete( @@ -529,7 +595,7 @@ impl TaskMgr { async fn clean_related_dependent( &self, - task_name: &&str, + task_name: &str, check_ops: &mut Vec, update_ops: &mut Vec, is_after: bool, @@ -550,84 +616,35 @@ impl TaskMgr { let target_ident = TaskDependentIdent::new_generic(&self.tenant, target_key.clone()); - self.check_and_remove( - check_ops, - update_ops, - &[task_name.to_string()], - &target_ident, - ) - .await?; - } - } - Ok(()) - } - - async fn check_and_add( - &self, - check_ops: &mut Vec, - update_ops: &mut Vec, - task_names: &[String], - dependent_ident: &TaskDependentIdent, - ) -> Result<(), TaskApiError> { - match self.kv_api.get_pb(dependent_ident).await? { - None => { - check_ops.push(TxnCondition { - key: dependent_ident.to_string_key(), - expected: ConditionResult::Eq as i32, - target: Some(Target::Seq(0)), - }); - - update_ops.push(TxnOp::put( - dependent_ident.to_string_key(), - TaskDependentValue(BTreeSet::from_iter(task_names.iter().cloned())) - .to_pb()? - .encode_to_vec(), - )); - } - Some(mut old_dependent) => { - check_ops.push(TxnCondition::eq_seq( - dependent_ident.to_string_key(), - old_dependent.seq, - )); - - old_dependent.0.extend(task_names.iter().cloned()); - - update_ops.push(TxnOp::put( - dependent_ident.to_string_key(), - old_dependent.to_pb()?.encode_to_vec(), - )); - } - } - Ok(()) - } - - async fn check_and_remove( - &self, - check_ops: &mut Vec, - update_ops: &mut Vec, - task_names: &[String], - dependent_ident: &TaskDependentIdent, - ) -> Result<(), TaskApiError> { - match self.kv_api.get_pb(dependent_ident).await? { - None => { - check_ops.push(TxnCondition { - key: dependent_ident.to_string_key(), - expected: ConditionResult::Eq as i32, - target: Some(Target::Seq(0)), - }); - } - Some(mut old_dependent) => { - check_ops.push(TxnCondition::eq_seq( - dependent_ident.to_string_key(), - old_dependent.seq, - )); - - old_dependent.0.retain(|name| !task_names.contains(name)); - - update_ops.push(TxnOp::put( - dependent_ident.to_string_key(), - old_dependent.to_pb()?.encode_to_vec(), - )); + match self.kv_api.get_pb(&target_ident).await? { + None => { + check_ops.push(TxnCondition { + key: target_ident.to_string_key(), + expected: ConditionResult::Eq as i32, + target: Some(Target::Seq(0)), + }); + + update_ops.push(TxnOp::put( + target_ident.to_string_key(), + TaskDependentValue(BTreeSet::from_iter([task_name.to_string()])) + .to_pb()? + .encode_to_vec(), + )); + } + Some(mut old_dependent) => { + check_ops.push(TxnCondition::eq_seq( + target_ident.to_string_key(), + old_dependent.seq, + )); + + old_dependent.0.remove(task_name); + + update_ops.push(TxnOp::put( + target_ident.to_string_key(), + old_dependent.to_pb()?.encode_to_vec(), + )); + } + } } } Ok(()) From 8b34211aba478ac0c364de3d5f0c7110df4798cc Mon Sep 17 00:00:00 2001 From: kould Date: Wed, 6 Aug 2025 15:53:32 +0800 Subject: [PATCH 17/28] chore: codefmt --- src/query/management/src/task/task_mgr.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/query/management/src/task/task_mgr.rs b/src/query/management/src/task/task_mgr.rs index d07d38549bb8e..51ea912150605 100644 --- a/src/query/management/src/task/task_mgr.rs +++ b/src/query/management/src/task/task_mgr.rs @@ -376,8 +376,9 @@ impl TaskMgr { Some(mut old_dependent) => { check_ops.push(TxnCondition::eq_seq(after_key.clone(), old_dependent.seq)); - old_dependent.0.retain(|name| !remove_afters.contains(name)); - + for remove_after in remove_afters { + old_dependent.0.remove(remove_after); + } update_ops.push(TxnOp::put( after_key, old_dependent.to_pb()?.encode_to_vec(), From c6c7373721ac634ef88f5b403e7ad68046f18f72 Mon Sep 17 00:00:00 2001 From: kould Date: Wed, 6 Aug 2025 16:29:57 +0800 Subject: [PATCH 18/28] chore: codefmt --- src/query/management/src/task/task_mgr.rs | 100 ++++++++-------------- 1 file changed, 35 insertions(+), 65 deletions(-) diff --git a/src/query/management/src/task/task_mgr.rs b/src/query/management/src/task/task_mgr.rs index 51ea912150605..2c10b86865bb6 100644 --- a/src/query/management/src/task/task_mgr.rs +++ b/src/query/management/src/task/task_mgr.rs @@ -42,8 +42,6 @@ use databend_common_meta_app::tenant::Tenant; use databend_common_meta_kvapi::kvapi; use databend_common_meta_kvapi::kvapi::DirName; use databend_common_meta_kvapi::kvapi::Key; -use databend_common_meta_types::txn_condition::Target; -use databend_common_meta_types::ConditionResult; use databend_common_meta_types::MatchSeq; use databend_common_meta_types::MetaError; use databend_common_meta_types::TxnCondition; @@ -277,32 +275,27 @@ impl TaskMgr { TaskDependentIdent::new_generic(&self.tenant, after_dependent.clone()); let after_key = after_dependent_ident.to_string_key(); - match self.kv_api.get_pb(&after_dependent_ident).await? { + let seq = match self.kv_api.get_pb(&after_dependent_ident).await? { None => { - check_ops.push(TxnCondition { - key: after_key.clone(), - expected: ConditionResult::Eq as i32, - target: Some(Target::Seq(0)), - }); - update_ops.push(TxnOp::put( - after_key, + after_key.clone(), TaskDependentValue(BTreeSet::from_iter(new_afters.iter().cloned())) .to_pb()? .encode_to_vec(), )); + 0 } Some(mut old_dependent) => { - check_ops.push(TxnCondition::eq_seq(after_key.clone(), old_dependent.seq)); - old_dependent.0.extend(new_afters.iter().cloned()); update_ops.push(TxnOp::put( - after_key, + after_key.clone(), old_dependent.to_pb()?.encode_to_vec(), )); + old_dependent.seq } - } + }; + check_ops.push(TxnCondition::eq_seq(after_key, seq)); for after in new_afters { let before_dependent = TaskDependentKey::new(DependentType::Before, after.clone()); @@ -310,32 +303,27 @@ impl TaskMgr { TaskDependentIdent::new_generic(&self.tenant, before_dependent); let before_key = before_dependent_ident.to_string_key(); - match self.kv_api.get_pb(&before_dependent_ident).await? { + let seq = match self.kv_api.get_pb(&before_dependent_ident).await? { None => { - check_ops.push(TxnCondition { - key: before_key.clone(), - expected: ConditionResult::Eq as i32, - target: Some(Target::Seq(0)), - }); - update_ops.push(TxnOp::put( - before_key, + before_key.clone(), TaskDependentValue(BTreeSet::from_iter([task_name.to_string()])) .to_pb()? .encode_to_vec(), )); + 0 } Some(mut old_dependent) => { - check_ops.push(TxnCondition::eq_seq(before_key.clone(), old_dependent.seq)); - old_dependent.0.insert(task_name.to_string()); update_ops.push(TxnOp::put( - before_key, + before_key.clone(), old_dependent.to_pb()?.encode_to_vec(), )); + old_dependent.seq } - } + }; + check_ops.push(TxnCondition::eq_seq(before_key, seq)); } let request = TxnRequest::new(check_ops, update_ops); let reply = self.kv_api.transaction(request).await?; @@ -365,26 +353,21 @@ impl TaskMgr { TaskDependentIdent::new_generic(&self.tenant, after_dependent.clone()); let after_key = after_dependent_ident.to_string_key(); - match self.kv_api.get_pb(&after_dependent_ident).await? { - None => { - check_ops.push(TxnCondition { - key: after_key.clone(), - expected: ConditionResult::Eq as i32, - target: Some(Target::Seq(0)), - }); - } + let seq = match self.kv_api.get_pb(&after_dependent_ident).await? { + None => 0, Some(mut old_dependent) => { - check_ops.push(TxnCondition::eq_seq(after_key.clone(), old_dependent.seq)); - for remove_after in remove_afters { old_dependent.0.remove(remove_after); } update_ops.push(TxnOp::put( - after_key, + after_key.clone(), old_dependent.to_pb()?.encode_to_vec(), )); + + old_dependent.seq } - } + }; + check_ops.push(TxnCondition::eq_seq(after_key, seq)); for after in remove_afters { let before_dependent = TaskDependentKey::new(DependentType::Before, after.clone()); @@ -392,25 +375,19 @@ impl TaskMgr { TaskDependentIdent::new_generic(&self.tenant, before_dependent); let before_key = before_dependent_ident.to_string_key(); - match self.kv_api.get_pb(&before_dependent_ident).await? { - None => { - check_ops.push(TxnCondition { - key: before_key.clone(), - expected: ConditionResult::Eq as i32, - target: Some(Target::Seq(0)), - }); - } + let seq = match self.kv_api.get_pb(&before_dependent_ident).await? { + None => 0, Some(mut old_dependent) => { - check_ops.push(TxnCondition::eq_seq(before_key.clone(), old_dependent.seq)); - old_dependent.0.remove(task_name); update_ops.push(TxnOp::put( - before_key, + before_key.clone(), old_dependent.to_pb()?.encode_to_vec(), )); + old_dependent.seq } - } + }; + check_ops.push(TxnCondition::eq_seq(before_key, seq)); } let request = TxnRequest::new(check_ops, update_ops); let reply = self.kv_api.transaction(request).await?; @@ -616,36 +593,29 @@ impl TaskMgr { let target_key = TaskDependentKey::new(other_dependent, dependent_target.clone()); let target_ident = TaskDependentIdent::new_generic(&self.tenant, target_key.clone()); + let target_key = target_ident.to_string_key(); - match self.kv_api.get_pb(&target_ident).await? { + let seq = match self.kv_api.get_pb(&target_ident).await? { None => { - check_ops.push(TxnCondition { - key: target_ident.to_string_key(), - expected: ConditionResult::Eq as i32, - target: Some(Target::Seq(0)), - }); - update_ops.push(TxnOp::put( - target_ident.to_string_key(), + target_key.clone(), TaskDependentValue(BTreeSet::from_iter([task_name.to_string()])) .to_pb()? .encode_to_vec(), )); + 0 } Some(mut old_dependent) => { - check_ops.push(TxnCondition::eq_seq( - target_ident.to_string_key(), - old_dependent.seq, - )); - old_dependent.0.remove(task_name); update_ops.push(TxnOp::put( - target_ident.to_string_key(), + target_key.clone(), old_dependent.to_pb()?.encode_to_vec(), )); + old_dependent.seq } - } + }; + check_ops.push(TxnCondition::eq_seq(target_key, seq)); } } Ok(()) From 804fcd6c3e5de7f1b850d0afbad49e02a7ddb80d Mon Sep 17 00:00:00 2001 From: kould Date: Wed, 6 Aug 2025 17:58:51 +0800 Subject: [PATCH 19/28] chore: codefmt --- src/meta/app/src/principal/task.rs | 2 +- src/query/management/src/task/errors.rs | 11 ++ src/query/management/src/task/task_mgr.rs | 141 ++++++---------------- 3 files changed, 51 insertions(+), 103 deletions(-) diff --git a/src/meta/app/src/principal/task.rs b/src/meta/app/src/principal/task.rs index 0f9a754199e93..8c67ad465f07e 100644 --- a/src/meta/app/src/principal/task.rs +++ b/src/meta/app/src/principal/task.rs @@ -197,7 +197,7 @@ impl TaskDependentKey { } } -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq, Default)] pub struct TaskDependentValue(pub BTreeSet); mod kvapi_key_impl { diff --git a/src/query/management/src/task/errors.rs b/src/query/management/src/task/errors.rs index bc74fe880acf9..d76b8c3be5f8b 100644 --- a/src/query/management/src/task/errors.rs +++ b/src/query/management/src/task/errors.rs @@ -13,6 +13,7 @@ // limitations under the License. use databend_common_exception::ErrorCode; +use databend_common_meta_types::InvalidArgument; use databend_common_meta_types::MetaError; use databend_common_proto_conv::Incompatible; @@ -89,6 +90,9 @@ pub enum TaskApiError { #[error("There are simultaneous update to task: {task_name} afters: {after}")] SimultaneousUpdateTaskAfter { task_name: String, after: String }, + + #[error("InvalidArgument error: {inner}")] + InvalidArgument { inner: InvalidArgument }, } impl From for TaskApiError { @@ -100,6 +104,12 @@ impl From for TaskApiError { } } +impl From for TaskApiError { + fn from(err: InvalidArgument) -> Self { + TaskApiError::InvalidArgument { inner: err } + } +} + impl From for TaskApiError { fn from(value: Incompatible) -> Self { TaskApiError::Incompatible { inner: value } @@ -117,6 +127,7 @@ impl From for ErrorCode { TaskApiError::SimultaneousUpdateTaskAfter { .. } => { ErrorCode::from_string(value.to_string()) } + TaskApiError::InvalidArgument { inner } => ErrorCode::from_std_error(inner), } } } diff --git a/src/query/management/src/task/task_mgr.rs b/src/query/management/src/task/task_mgr.rs index 2c10b86865bb6..ecc44d4c4769d 100644 --- a/src/query/management/src/task/task_mgr.rs +++ b/src/query/management/src/task/task_mgr.rs @@ -12,7 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::collections::BTreeSet; use std::str::FromStr; use std::sync::Arc; @@ -23,6 +22,7 @@ use databend_common_ast::ast::AlterTaskOptions; use databend_common_ast::ast::ScheduleOptions; use databend_common_meta_api::kv_pb_api::KVPbApi; use databend_common_meta_api::kv_pb_api::UpsertPB; +use databend_common_meta_api::util::txn_put_pb; use databend_common_meta_api::SchemaApi; use databend_common_meta_app::principal::task; use databend_common_meta_app::principal::task::TaskMessage; @@ -35,7 +35,6 @@ use databend_common_meta_app::principal::ScheduleType; use databend_common_meta_app::principal::Status; use databend_common_meta_app::principal::Task; use databend_common_meta_app::principal::TaskDependentKey; -use databend_common_meta_app::principal::TaskDependentValue; use databend_common_meta_app::principal::TaskIdent; use databend_common_meta_app::schema::CreateOption; use databend_common_meta_app::tenant::Tenant; @@ -51,6 +50,7 @@ use databend_common_meta_types::With; use databend_common_proto_conv::FromToProto; use futures::TryStreamExt; use prost::Message; +use seq_marked::SeqValue; use crate::task::errors::TaskApiError; use crate::task::errors::TaskError; @@ -275,27 +275,12 @@ impl TaskMgr { TaskDependentIdent::new_generic(&self.tenant, after_dependent.clone()); let after_key = after_dependent_ident.to_string_key(); - let seq = match self.kv_api.get_pb(&after_dependent_ident).await? { - None => { - update_ops.push(TxnOp::put( - after_key.clone(), - TaskDependentValue(BTreeSet::from_iter(new_afters.iter().cloned())) - .to_pb()? - .encode_to_vec(), - )); - 0 - } - Some(mut old_dependent) => { - old_dependent.0.extend(new_afters.iter().cloned()); - - update_ops.push(TxnOp::put( - after_key.clone(), - old_dependent.to_pb()?.encode_to_vec(), - )); - old_dependent.seq - } - }; - check_ops.push(TxnCondition::eq_seq(after_key, seq)); + let after_seq_deps = self.kv_api.get_pb(&after_dependent_ident).await?; + check_ops.push(TxnCondition::eq_seq(after_key, after_seq_deps.seq())); + + let mut after_deps = after_seq_deps.unwrap_or_default(); + after_deps.0.extend(new_afters.iter().cloned()); + update_ops.push(txn_put_pb(&after_dependent_ident, &after_deps)?); for after in new_afters { let before_dependent = TaskDependentKey::new(DependentType::Before, after.clone()); @@ -303,27 +288,12 @@ impl TaskMgr { TaskDependentIdent::new_generic(&self.tenant, before_dependent); let before_key = before_dependent_ident.to_string_key(); - let seq = match self.kv_api.get_pb(&before_dependent_ident).await? { - None => { - update_ops.push(TxnOp::put( - before_key.clone(), - TaskDependentValue(BTreeSet::from_iter([task_name.to_string()])) - .to_pb()? - .encode_to_vec(), - )); - 0 - } - Some(mut old_dependent) => { - old_dependent.0.insert(task_name.to_string()); + let before_seq_deps = self.kv_api.get_pb(&before_dependent_ident).await?; + check_ops.push(TxnCondition::eq_seq(before_key, before_seq_deps.seq())); - update_ops.push(TxnOp::put( - before_key.clone(), - old_dependent.to_pb()?.encode_to_vec(), - )); - old_dependent.seq - } - }; - check_ops.push(TxnCondition::eq_seq(before_key, seq)); + let mut deps = before_seq_deps.unwrap_or_default(); + deps.0.insert(task_name.to_string()); + update_ops.push(txn_put_pb(&before_dependent_ident, &deps)?); } let request = TxnRequest::new(check_ops, update_ops); let reply = self.kv_api.transaction(request).await?; @@ -353,21 +323,15 @@ impl TaskMgr { TaskDependentIdent::new_generic(&self.tenant, after_dependent.clone()); let after_key = after_dependent_ident.to_string_key(); - let seq = match self.kv_api.get_pb(&after_dependent_ident).await? { - None => 0, - Some(mut old_dependent) => { - for remove_after in remove_afters { - old_dependent.0.remove(remove_after); - } - update_ops.push(TxnOp::put( - after_key.clone(), - old_dependent.to_pb()?.encode_to_vec(), - )); + let after_seq_deps = self.kv_api.get_pb(&after_dependent_ident).await?; + check_ops.push(TxnCondition::eq_seq(after_key, after_seq_deps.seq())); - old_dependent.seq + if let Some(mut deps) = after_seq_deps { + for remove_after in remove_afters { + deps.0.remove(remove_after); } - }; - check_ops.push(TxnCondition::eq_seq(after_key, seq)); + update_ops.push(txn_put_pb(&after_dependent_ident, &deps)?); + } for after in remove_afters { let before_dependent = TaskDependentKey::new(DependentType::Before, after.clone()); @@ -375,19 +339,13 @@ impl TaskMgr { TaskDependentIdent::new_generic(&self.tenant, before_dependent); let before_key = before_dependent_ident.to_string_key(); - let seq = match self.kv_api.get_pb(&before_dependent_ident).await? { - None => 0, - Some(mut old_dependent) => { - old_dependent.0.remove(task_name); + let before_seq_deps = self.kv_api.get_pb(&before_dependent_ident).await?; + check_ops.push(TxnCondition::eq_seq(before_key, before_seq_deps.seq())); - update_ops.push(TxnOp::put( - before_key.clone(), - old_dependent.to_pb()?.encode_to_vec(), - )); - old_dependent.seq - } - }; - check_ops.push(TxnCondition::eq_seq(before_key, seq)); + if let Some(mut deps) = before_seq_deps { + deps.0.remove(task_name); + update_ops.push(txn_put_pb(&before_dependent_ident, &deps)?); + } } let request = TxnRequest::new(check_ops, update_ops); let reply = self.kv_api.transaction(request).await?; @@ -452,12 +410,10 @@ impl TaskMgr { TaskDependentKey::new(DependentType::Before, task_name.to_string()), ); let task_state_key = TaskStateIdent::new(&self.tenant, task_name); - let succeeded_value = TaskState { is_succeeded: true }.to_pb()?.encode_to_vec(); + let succeeded_value = TaskState { is_succeeded: true }; let not_succeeded_value = TaskState { is_succeeded: false, - } - .to_pb()? - .encode_to_vec(); + }; let Some(task_before_dependent) = self.kv_api.get_pb(&task_before_ident).await? else { return Ok(Ok(Vec::new())); @@ -482,18 +438,13 @@ impl TaskMgr { if target_after_task != task_name { conditions.push(TxnCondition::eq_value( task_ident.to_string_key(), - succeeded_value.clone(), + succeeded_value.to_pb()?.encode_to_vec(), )); } - if_ops.push(TxnOp::put( - task_ident.to_string_key(), - not_succeeded_value.clone(), - )); + if_ops.push(txn_put_pb(&task_ident, ¬_succeeded_value)?); } - let request = TxnRequest::new(conditions, if_ops).with_else(vec![TxnOp::put( - task_state_key.to_string_key(), - succeeded_value.clone(), - )]); + let request = TxnRequest::new(conditions, if_ops) + .with_else(vec![txn_put_pb(&task_state_key, &succeeded_value)?]); let reply = self.kv_api.transaction(request).await?; if reply.success { @@ -595,27 +546,13 @@ impl TaskMgr { TaskDependentIdent::new_generic(&self.tenant, target_key.clone()); let target_key = target_ident.to_string_key(); - let seq = match self.kv_api.get_pb(&target_ident).await? { - None => { - update_ops.push(TxnOp::put( - target_key.clone(), - TaskDependentValue(BTreeSet::from_iter([task_name.to_string()])) - .to_pb()? - .encode_to_vec(), - )); - 0 - } - Some(mut old_dependent) => { - old_dependent.0.remove(task_name); - - update_ops.push(TxnOp::put( - target_key.clone(), - old_dependent.to_pb()?.encode_to_vec(), - )); - old_dependent.seq - } - }; - check_ops.push(TxnCondition::eq_seq(target_key, seq)); + let seq_dep = self.kv_api.get_pb(&target_ident).await?; + check_ops.push(TxnCondition::eq_seq(target_key, seq_dep.seq())); + + if let Some(mut deps) = seq_dep { + deps.0.remove(task_name); + update_ops.push(txn_put_pb(&target_ident, &deps)?); + } } } Ok(()) From 60da8630ce1e1b4fa5a807020fb815d78e217d9a Mon Sep 17 00:00:00 2001 From: kould Date: Thu, 7 Aug 2025 10:19:35 +0800 Subject: [PATCH 20/28] chore: codefmt --- src/query/management/src/task/task_mgr.rs | 73 ++++++++++++++--------- 1 file changed, 44 insertions(+), 29 deletions(-) diff --git a/src/query/management/src/task/task_mgr.rs b/src/query/management/src/task/task_mgr.rs index ecc44d4c4769d..e24350c47efab 100644 --- a/src/query/management/src/task/task_mgr.rs +++ b/src/query/management/src/task/task_mgr.rs @@ -22,6 +22,7 @@ use databend_common_ast::ast::AlterTaskOptions; use databend_common_ast::ast::ScheduleOptions; use databend_common_meta_api::kv_pb_api::KVPbApi; use databend_common_meta_api::kv_pb_api::UpsertPB; +use databend_common_meta_api::txn_cond_eq_seq; use databend_common_meta_api::util::txn_put_pb; use databend_common_meta_api::SchemaApi; use databend_common_meta_app::principal::task; @@ -273,23 +274,29 @@ impl TaskMgr { let after_dependent = TaskDependentKey::new(DependentType::After, task_name.to_string()); let after_dependent_ident = TaskDependentIdent::new_generic(&self.tenant, after_dependent.clone()); - let after_key = after_dependent_ident.to_string_key(); let after_seq_deps = self.kv_api.get_pb(&after_dependent_ident).await?; - check_ops.push(TxnCondition::eq_seq(after_key, after_seq_deps.seq())); + check_ops.push(txn_cond_eq_seq( + &after_dependent_ident, + after_seq_deps.seq(), + )); let mut after_deps = after_seq_deps.unwrap_or_default(); after_deps.0.extend(new_afters.iter().cloned()); update_ops.push(txn_put_pb(&after_dependent_ident, &after_deps)?); - for after in new_afters { - let before_dependent = TaskDependentKey::new(DependentType::Before, after.clone()); - let before_dependent_ident = - TaskDependentIdent::new_generic(&self.tenant, before_dependent); - let before_key = before_dependent_ident.to_string_key(); - - let before_seq_deps = self.kv_api.get_pb(&before_dependent_ident).await?; - check_ops.push(TxnCondition::eq_seq(before_key, before_seq_deps.seq())); + for (before_dependent_ident, before_seq_deps) in self + .kv_api + .get_pb_vec(new_afters.iter().map(|after| { + let before_dependent = TaskDependentKey::new(DependentType::Before, after.clone()); + TaskDependentIdent::new_generic(&self.tenant, before_dependent) + })) + .await? + { + check_ops.push(txn_cond_eq_seq( + &before_dependent_ident, + before_seq_deps.seq(), + )); let mut deps = before_seq_deps.unwrap_or_default(); deps.0.insert(task_name.to_string()); @@ -321,10 +328,12 @@ impl TaskMgr { let after_dependent = TaskDependentKey::new(DependentType::After, task_name.to_string()); let after_dependent_ident = TaskDependentIdent::new_generic(&self.tenant, after_dependent.clone()); - let after_key = after_dependent_ident.to_string_key(); let after_seq_deps = self.kv_api.get_pb(&after_dependent_ident).await?; - check_ops.push(TxnCondition::eq_seq(after_key, after_seq_deps.seq())); + check_ops.push(txn_cond_eq_seq( + &after_dependent_ident, + after_seq_deps.seq(), + )); if let Some(mut deps) = after_seq_deps { for remove_after in remove_afters { @@ -333,14 +342,18 @@ impl TaskMgr { update_ops.push(txn_put_pb(&after_dependent_ident, &deps)?); } - for after in remove_afters { - let before_dependent = TaskDependentKey::new(DependentType::Before, after.clone()); - let before_dependent_ident = - TaskDependentIdent::new_generic(&self.tenant, before_dependent); - let before_key = before_dependent_ident.to_string_key(); - - let before_seq_deps = self.kv_api.get_pb(&before_dependent_ident).await?; - check_ops.push(TxnCondition::eq_seq(before_key, before_seq_deps.seq())); + for (before_dependent_ident, before_seq_deps) in self + .kv_api + .get_pb_vec(remove_afters.iter().map(|after| { + let before_dependent = TaskDependentKey::new(DependentType::Before, after.clone()); + TaskDependentIdent::new_generic(&self.tenant, before_dependent) + })) + .await? + { + check_ops.push(txn_cond_eq_seq( + &before_dependent_ident, + before_seq_deps.seq(), + )); if let Some(mut deps) = before_seq_deps { deps.0.remove(task_name); @@ -420,12 +433,15 @@ impl TaskMgr { }; let mut ready_tasks = Vec::new(); - for task_before_target in task_before_dependent.0.iter() { - let target_after_ident = TaskDependentIdent::new_generic( - &self.tenant, - TaskDependentKey::new(DependentType::After, task_before_target.to_string()), - ); - let Some(target_after_dependent) = self.kv_api.get(&target_after_ident).await? else { + for (target_after_ident, target_after_dependent) in self + .kv_api + .get_pb_vec(task_before_dependent.0.iter().map(|before| { + let after_dependent = TaskDependentKey::new(DependentType::After, before.clone()); + TaskDependentIdent::new_generic(&self.tenant, after_dependent) + })) + .await? + { + let Some(target_after_dependent) = target_after_dependent else { continue; }; @@ -448,7 +464,7 @@ impl TaskMgr { let reply = self.kv_api.transaction(request).await?; if reply.success { - ready_tasks.push(task_before_target.clone()) + ready_tasks.push(target_after_ident.name().source.clone()); } } Ok(Ok(ready_tasks)) @@ -544,10 +560,9 @@ impl TaskMgr { let target_key = TaskDependentKey::new(other_dependent, dependent_target.clone()); let target_ident = TaskDependentIdent::new_generic(&self.tenant, target_key.clone()); - let target_key = target_ident.to_string_key(); let seq_dep = self.kv_api.get_pb(&target_ident).await?; - check_ops.push(TxnCondition::eq_seq(target_key, seq_dep.seq())); + check_ops.push(txn_cond_eq_seq(&target_ident, seq_dep.seq())); if let Some(mut deps) = seq_dep { deps.0.remove(task_name); From ee04c4a9954ae3804222ffdc2d6dcf17fdbdffc3 Mon Sep 17 00:00:00 2001 From: kould Date: Thu, 7 Aug 2025 12:08:23 +0800 Subject: [PATCH 21/28] chore: codefmt --- src/query/management/src/task/task_mgr.rs | 136 +++++++++++----------- 1 file changed, 67 insertions(+), 69 deletions(-) diff --git a/src/query/management/src/task/task_mgr.rs b/src/query/management/src/task/task_mgr.rs index e24350c47efab..8d981db4854e5 100644 --- a/src/query/management/src/task/task_mgr.rs +++ b/src/query/management/src/task/task_mgr.rs @@ -285,13 +285,12 @@ impl TaskMgr { after_deps.0.extend(new_afters.iter().cloned()); update_ops.push(txn_put_pb(&after_dependent_ident, &after_deps)?); - for (before_dependent_ident, before_seq_deps) in self - .kv_api - .get_pb_vec(new_afters.iter().map(|after| { - let before_dependent = TaskDependentKey::new(DependentType::Before, after.clone()); - TaskDependentIdent::new_generic(&self.tenant, before_dependent) - })) - .await? + let before_dependent_idents = new_afters.iter().map(|after| { + let before_dependent = TaskDependentKey::new(DependentType::Before, after.clone()); + TaskDependentIdent::new_generic(&self.tenant, before_dependent) + }); + for (before_dependent_ident, before_seq_deps) in + self.kv_api.get_pb_vec(before_dependent_idents).await? { check_ops.push(txn_cond_eq_seq( &before_dependent_ident, @@ -342,13 +341,12 @@ impl TaskMgr { update_ops.push(txn_put_pb(&after_dependent_ident, &deps)?); } - for (before_dependent_ident, before_seq_deps) in self - .kv_api - .get_pb_vec(remove_afters.iter().map(|after| { - let before_dependent = TaskDependentKey::new(DependentType::Before, after.clone()); - TaskDependentIdent::new_generic(&self.tenant, before_dependent) - })) - .await? + let before_dependent_idents = remove_afters.iter().map(|after| { + let before_dependent = TaskDependentKey::new(DependentType::Before, after.clone()); + TaskDependentIdent::new_generic(&self.tenant, before_dependent) + }); + for (before_dependent_ident, before_seq_deps) in + self.kv_api.get_pb_vec(before_dependent_idents).await? { check_ops.push(txn_cond_eq_seq( &before_dependent_ident, @@ -383,11 +381,44 @@ impl TaskMgr { let mut check_ops = Vec::new(); let mut update_ops = Vec::new(); - self.clean_related_dependent(task_name, &mut check_ops, &mut update_ops, true) - .await?; - self.clean_related_dependent(task_name, &mut check_ops, &mut update_ops, false) - .await?; + let task_after_ident = TaskDependentIdent::new_generic( + &self.tenant, + TaskDependentKey::new(DependentType::After, task_name.to_string()), + ); + if let Some(task_after_dependent) = self.kv_api.get(&task_after_ident).await? { + let target_idents = task_after_dependent.0.into_iter().map(|dependent_target| { + let target_key = + TaskDependentKey::new(DependentType::Before, dependent_target.clone()); + TaskDependentIdent::new_generic(&self.tenant, target_key.clone()) + }); + for (target_ident, seq_dep) in self.kv_api.get_pb_vec(target_idents).await? { + check_ops.push(txn_cond_eq_seq(&target_ident, seq_dep.seq())); + + if let Some(mut deps) = seq_dep { + deps.0.remove(task_name); + update_ops.push(txn_put_pb(&target_ident, &deps)?); + } + } + } + let task_before_ident = TaskDependentIdent::new_generic( + &self.tenant, + TaskDependentKey::new(DependentType::Before, task_name.to_string()), + ); + if let Some(task_before_dependent) = self.kv_api.get(&task_before_ident).await? { + let target_idents = task_before_dependent.0.into_iter().map(|dependent_target| { + let target_key = + TaskDependentKey::new(DependentType::After, dependent_target.clone()); + TaskDependentIdent::new_generic(&self.tenant, target_key.clone()) + }); + for (target_ident, seq_dep) in self.kv_api.get_pb_vec(target_idents).await? { + check_ops.push(txn_cond_eq_seq(&target_ident, seq_dep.seq())); + if let Some(mut deps) = seq_dep { + deps.0.remove(task_name); + update_ops.push(txn_put_pb(&target_ident, &deps)?); + } + } + } update_ops.push(TxnOp::delete( TaskDependentIdent::new_generic( &self.tenant, @@ -406,7 +437,7 @@ impl TaskMgr { TaskStateIdent::new(&self.tenant, task_name).to_string_key(), )); - let request = TxnRequest::new(vec![], update_ops); + let request = TxnRequest::new(check_ops, update_ops); let _ = self.kv_api.transaction(request).await?; Ok(Ok(())) @@ -433,34 +464,36 @@ impl TaskMgr { }; let mut ready_tasks = Vec::new(); - for (target_after_ident, target_after_dependent) in self - .kv_api - .get_pb_vec(task_before_dependent.0.iter().map(|before| { - let after_dependent = TaskDependentKey::new(DependentType::After, before.clone()); - TaskDependentIdent::new_generic(&self.tenant, after_dependent) - })) - .await? + let target_after_idents = task_before_dependent.0.iter().map(|before| { + let after_dependent = TaskDependentKey::new(DependentType::After, before.clone()); + TaskDependentIdent::new_generic(&self.tenant, after_dependent) + }); + for (target_after_ident, target_after_dependent) in + self.kv_api.get_pb_vec(target_after_idents).await? { let Some(target_after_dependent) = target_after_dependent else { continue; }; - let mut conditions = Vec::new(); - let mut if_ops = Vec::new(); + let mut request = TxnRequest::new(vec![], vec![]) + .with_else(vec![txn_put_pb(&task_state_key, &succeeded_value)?]); for target_after_task in target_after_dependent.0.iter() { let task_ident = TaskStateIdent::new(&self.tenant, target_after_task); // Only care about the predecessors of this task's successor tasks, excluding this task itself. if target_after_task != task_name { - conditions.push(TxnCondition::eq_value( + request = + request.push_if_then([], [txn_put_pb(&task_ident, ¬_succeeded_value)?]); + continue; + } + request = request.push_if_then( + [TxnCondition::eq_value( task_ident.to_string_key(), succeeded_value.to_pb()?.encode_to_vec(), - )); - } - if_ops.push(txn_put_pb(&task_ident, ¬_succeeded_value)?); + )], + [txn_put_pb(&task_ident, ¬_succeeded_value)?], + ); } - let request = TxnRequest::new(conditions, if_ops) - .with_else(vec![txn_put_pb(&task_state_key, &succeeded_value)?]); let reply = self.kv_api.transaction(request).await?; if reply.success { @@ -538,41 +571,6 @@ impl TaskMgr { Ok(Ok(())) } - async fn clean_related_dependent( - &self, - task_name: &str, - check_ops: &mut Vec, - update_ops: &mut Vec, - is_after: bool, - ) -> Result<(), TaskApiError> { - let (self_dependent, other_dependent) = if is_after { - (DependentType::After, DependentType::Before) - } else { - (DependentType::Before, DependentType::After) - }; - - let task_ident = TaskDependentIdent::new_generic( - &self.tenant, - TaskDependentKey::new(self_dependent, task_name.to_string()), - ); - if let Some(task_dependent) = self.kv_api.get(&task_ident).await? { - for dependent_target in task_dependent.0.iter() { - let target_key = TaskDependentKey::new(other_dependent, dependent_target.clone()); - let target_ident = - TaskDependentIdent::new_generic(&self.tenant, target_key.clone()); - - let seq_dep = self.kv_api.get_pb(&target_ident).await?; - check_ops.push(txn_cond_eq_seq(&target_ident, seq_dep.seq())); - - if let Some(mut deps) = seq_dep { - deps.0.remove(task_name); - update_ops.push(txn_put_pb(&target_ident, &deps)?); - } - } - } - Ok(()) - } - pub fn make_schedule_options(opt: ScheduleOptions) -> task::ScheduleOptions { match opt { ScheduleOptions::IntervalSecs(secs, ms) => { From 6132b93e080a25830f0411b075d56b7596aa85cf Mon Sep 17 00:00:00 2001 From: kould Date: Thu, 7 Aug 2025 13:49:48 +0800 Subject: [PATCH 22/28] chore: codefmt --- .../app/src/principal/task_dependent_ident.rs | 18 +++ src/query/management/src/task/task_mgr.rs | 139 ++++++++---------- src/query/service/src/task/service.rs | 2 +- 3 files changed, 80 insertions(+), 79 deletions(-) diff --git a/src/meta/app/src/principal/task_dependent_ident.rs b/src/meta/app/src/principal/task_dependent_ident.rs index 910ba6f4c0a61..010123fd6e467 100644 --- a/src/meta/app/src/principal/task_dependent_ident.rs +++ b/src/meta/app/src/principal/task_dependent_ident.rs @@ -16,9 +16,27 @@ use crate::tenant_key::ident::TIdent; pub type TaskDependentIdent = TIdent; +impl TaskDependentIdent { + pub fn new_before(tenant: impl ToTenant, task_name: impl ToString) -> Self { + TaskDependentIdent::new_generic( + tenant, + TaskDependentKey::new(DependentType::Before, task_name.to_string()), + ) + } + + pub fn new_after(tenant: impl ToTenant, task_name: impl ToString) -> Self { + TaskDependentIdent::new_generic( + tenant, + TaskDependentKey::new(DependentType::After, task_name.to_string()), + ) + } +} + pub use kvapi_impl::TaskDependentResource; +use crate::principal::DependentType; use crate::principal::TaskDependentKey; +use crate::tenant::ToTenant; mod kvapi_impl { use databend_common_meta_kvapi::kvapi; diff --git a/src/query/management/src/task/task_mgr.rs b/src/query/management/src/task/task_mgr.rs index 8d981db4854e5..9f75d5d12c81c 100644 --- a/src/query/management/src/task/task_mgr.rs +++ b/src/query/management/src/task/task_mgr.rs @@ -31,11 +31,9 @@ use databend_common_meta_app::principal::task::TaskState; use databend_common_meta_app::principal::task_dependent_ident::TaskDependentIdent; use databend_common_meta_app::principal::task_message_ident::TaskMessageIdent; use databend_common_meta_app::principal::task_state_ident::TaskStateIdent; -use databend_common_meta_app::principal::DependentType; use databend_common_meta_app::principal::ScheduleType; use databend_common_meta_app::principal::Status; use databend_common_meta_app::principal::Task; -use databend_common_meta_app::principal::TaskDependentKey; use databend_common_meta_app::principal::TaskIdent; use databend_common_meta_app::schema::CreateOption; use databend_common_meta_app::tenant::Tenant; @@ -271,9 +269,7 @@ impl TaskMgr { let mut update_ops = Vec::new(); let mut check_ops = Vec::with_capacity(new_afters.len()); - let after_dependent = TaskDependentKey::new(DependentType::After, task_name.to_string()); - let after_dependent_ident = - TaskDependentIdent::new_generic(&self.tenant, after_dependent.clone()); + let after_dependent_ident = TaskDependentIdent::new_after(&self.tenant, task_name); let after_seq_deps = self.kv_api.get_pb(&after_dependent_ident).await?; check_ops.push(txn_cond_eq_seq( @@ -285,10 +281,9 @@ impl TaskMgr { after_deps.0.extend(new_afters.iter().cloned()); update_ops.push(txn_put_pb(&after_dependent_ident, &after_deps)?); - let before_dependent_idents = new_afters.iter().map(|after| { - let before_dependent = TaskDependentKey::new(DependentType::Before, after.clone()); - TaskDependentIdent::new_generic(&self.tenant, before_dependent) - }); + let before_dependent_idents = new_afters + .iter() + .map(|after| TaskDependentIdent::new_before(&self.tenant, after)); for (before_dependent_ident, before_seq_deps) in self.kv_api.get_pb_vec(before_dependent_idents).await? { @@ -324,10 +319,7 @@ impl TaskMgr { let mut update_ops = Vec::new(); let mut check_ops = Vec::with_capacity(remove_afters.len()); - let after_dependent = TaskDependentKey::new(DependentType::After, task_name.to_string()); - let after_dependent_ident = - TaskDependentIdent::new_generic(&self.tenant, after_dependent.clone()); - + let after_dependent_ident = TaskDependentIdent::new_after(&self.tenant, task_name); let after_seq_deps = self.kv_api.get_pb(&after_dependent_ident).await?; check_ops.push(txn_cond_eq_seq( &after_dependent_ident, @@ -341,10 +333,9 @@ impl TaskMgr { update_ops.push(txn_put_pb(&after_dependent_ident, &deps)?); } - let before_dependent_idents = remove_afters.iter().map(|after| { - let before_dependent = TaskDependentKey::new(DependentType::Before, after.clone()); - TaskDependentIdent::new_generic(&self.tenant, before_dependent) - }); + let before_dependent_idents = remove_afters + .iter() + .map(|after| TaskDependentIdent::new_before(&self.tenant, after)); for (before_dependent_ident, before_seq_deps) in self.kv_api.get_pb_vec(before_dependent_idents).await? { @@ -381,57 +372,33 @@ impl TaskMgr { let mut check_ops = Vec::new(); let mut update_ops = Vec::new(); - let task_after_ident = TaskDependentIdent::new_generic( - &self.tenant, - TaskDependentKey::new(DependentType::After, task_name.to_string()), - ); - if let Some(task_after_dependent) = self.kv_api.get(&task_after_ident).await? { - let target_idents = task_after_dependent.0.into_iter().map(|dependent_target| { - let target_key = - TaskDependentKey::new(DependentType::Before, dependent_target.clone()); - TaskDependentIdent::new_generic(&self.tenant, target_key.clone()) - }); - for (target_ident, seq_dep) in self.kv_api.get_pb_vec(target_idents).await? { - check_ops.push(txn_cond_eq_seq(&target_ident, seq_dep.seq())); + let task_after_ident = TaskDependentIdent::new_after(&self.tenant, task_name); + let task_before_ident = TaskDependentIdent::new_before(&self.tenant, task_name); - if let Some(mut deps) = seq_dep { - deps.0.remove(task_name); - update_ops.push(txn_put_pb(&target_ident, &deps)?); - } - } + let mut target_idents = Vec::new(); + if let Some(task_after_dependent) = self.kv_api.get(&task_after_ident).await? { + target_idents.extend(task_after_dependent.0.into_iter().map(|dependent_target| { + TaskDependentIdent::new_before(&self.tenant, dependent_target) + })); } - let task_before_ident = TaskDependentIdent::new_generic( - &self.tenant, - TaskDependentKey::new(DependentType::Before, task_name.to_string()), - ); if let Some(task_before_dependent) = self.kv_api.get(&task_before_ident).await? { - let target_idents = task_before_dependent.0.into_iter().map(|dependent_target| { - let target_key = - TaskDependentKey::new(DependentType::After, dependent_target.clone()); - TaskDependentIdent::new_generic(&self.tenant, target_key.clone()) - }); - for (target_ident, seq_dep) in self.kv_api.get_pb_vec(target_idents).await? { - check_ops.push(txn_cond_eq_seq(&target_ident, seq_dep.seq())); + target_idents.extend(task_before_dependent.0.into_iter().map(|dependent_target| { + TaskDependentIdent::new_after(&self.tenant, dependent_target) + })); + } + for (target_ident, seq_dep) in self.kv_api.get_pb_vec(target_idents).await? { + check_ops.push(txn_cond_eq_seq(&target_ident, seq_dep.seq())); - if let Some(mut deps) = seq_dep { - deps.0.remove(task_name); - update_ops.push(txn_put_pb(&target_ident, &deps)?); - } + if let Some(mut deps) = seq_dep { + deps.0.remove(task_name); + update_ops.push(txn_put_pb(&target_ident, &deps)?); } } update_ops.push(TxnOp::delete( - TaskDependentIdent::new_generic( - &self.tenant, - TaskDependentKey::new(DependentType::Before, task_name.to_string()), - ) - .to_string_key(), + TaskDependentIdent::new_before(&self.tenant, task_name).to_string_key(), )); update_ops.push(TxnOp::delete( - TaskDependentIdent::new_generic( - &self.tenant, - TaskDependentKey::new(DependentType::After, task_name.to_string()), - ) - .to_string_key(), + TaskDependentIdent::new_after(&self.tenant, task_name).to_string_key(), )); update_ops.push(TxnOp::delete( TaskStateIdent::new(&self.tenant, task_name).to_string_key(), @@ -443,16 +410,36 @@ impl TaskMgr { Ok(Ok(())) } + /// Marks the given task as succeeded, and checks all tasks that depend on it. + /// + /// For each task that depends on the completed task (`task_name`), we check if all its + /// predecessor tasks are also succeeded. If so, we mark the dependent task as *not succeeded* + /// to prevent premature execution. Otherwise, we record the dependent task as *ready* + /// for further processing. + /// + /// # Arguments + /// - `task_name`: The name of the task that has just completed successfully. + /// + /// # Returns + /// - `Vec`: A list of dependent task names that are ready to proceed. + /// + /// # Behavior + /// 1. Retrieves all tasks that must be executed *before* the given `task_name`. + /// 2. For each such task, find the tasks that depend on it (`after` tasks). + /// 3. For each `after` task: + /// - If all its dependencies (excluding the current task) are succeeded: + /// - Mark that task as **not succeeded**. + /// - Also mark the current task as succeeded. + /// - Record it as ready for further processing. + /// - Otherwise: + /// - Still mark the current task as succeeded. #[async_backtrace::framed] #[fastrace::trace] - pub async fn task_succeeded( + pub async fn get_next_ready_tasks( &self, task_name: &str, ) -> Result, TaskError>, TaskApiError> { - let task_before_ident = TaskDependentIdent::new_generic( - &self.tenant, - TaskDependentKey::new(DependentType::Before, task_name.to_string()), - ); + let task_before_ident = TaskDependentIdent::new_before(&self.tenant, task_name); let task_state_key = TaskStateIdent::new(&self.tenant, task_name); let succeeded_value = TaskState { is_succeeded: true }; let not_succeeded_value = TaskState { @@ -464,17 +451,16 @@ impl TaskMgr { }; let mut ready_tasks = Vec::new(); - let target_after_idents = task_before_dependent.0.iter().map(|before| { - let after_dependent = TaskDependentKey::new(DependentType::After, before.clone()); - TaskDependentIdent::new_generic(&self.tenant, after_dependent) - }); + let target_after_idents = task_before_dependent + .0 + .iter() + .map(|before| TaskDependentIdent::new_after(&self.tenant, before)); for (target_after_ident, target_after_dependent) in self.kv_api.get_pb_vec(target_after_idents).await? { let Some(target_after_dependent) = target_after_dependent else { continue; }; - let mut request = TxnRequest::new(vec![], vec![]) .with_else(vec![txn_put_pb(&task_state_key, &succeeded_value)?]); @@ -482,17 +468,14 @@ impl TaskMgr { let task_ident = TaskStateIdent::new(&self.tenant, target_after_task); // Only care about the predecessors of this task's successor tasks, excluding this task itself. if target_after_task != task_name { - request = - request.push_if_then([], [txn_put_pb(&task_ident, ¬_succeeded_value)?]); - continue; - } - request = request.push_if_then( - [TxnCondition::eq_value( + request.condition.push(TxnCondition::eq_value( task_ident.to_string_key(), succeeded_value.to_pb()?.encode_to_vec(), - )], - [txn_put_pb(&task_ident, ¬_succeeded_value)?], - ); + )); + } + request + .if_then + .push(txn_put_pb(&task_ident, ¬_succeeded_value)?); } let reply = self.kv_api.transaction(request).await?; diff --git a/src/query/service/src/task/service.rs b/src/query/service/src/task/service.rs index da8bad024157f..1f42442d19e60 100644 --- a/src/query/service/src/task/service.rs +++ b/src/query/service/src/task/service.rs @@ -393,7 +393,7 @@ impl TaskService { .await?; for next_task in - task_mgr.task_succeeded(&task_name).await?? + task_mgr.get_next_ready_tasks(&task_name).await?? { let next_task = task_mgr .describe_task(&next_task) From 066290744909783a9baa0e026934f16460ef7b37 Mon Sep 17 00:00:00 2001 From: kould Date: Thu, 7 Aug 2025 17:39:16 +0800 Subject: [PATCH 23/28] chore: codefmt --- src/query/management/src/task/task_mgr.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/query/management/src/task/task_mgr.rs b/src/query/management/src/task/task_mgr.rs index 9f75d5d12c81c..16c6eef064fe9 100644 --- a/src/query/management/src/task/task_mgr.rs +++ b/src/query/management/src/task/task_mgr.rs @@ -424,15 +424,15 @@ impl TaskMgr { /// - `Vec`: A list of dependent task names that are ready to proceed. /// /// # Behavior - /// 1. Retrieves all tasks that must be executed *before* the given `task_name`. - /// 2. For each such task, find the tasks that depend on it (`after` tasks). - /// 3. For each `after` task: - /// - If all its dependencies (excluding the current task) are succeeded: - /// - Mark that task as **not succeeded**. - /// - Also mark the current task as succeeded. - /// - Record it as ready for further processing. + /// 1. Retrieves all tasks that the given `task_name` is a dependency of (i.e., tasks that depend on `task_name`). + /// 2. For each such dependent task: + /// - Check whether all its dependencies (excluding `task_name`) have succeeded. + /// - If so: + /// - Mark that task as **not succeeded** to trigger reevaluation or blocking. + /// - Mark `task_name` as succeeded. + /// - Add the task to the ready list for further processing. /// - Otherwise: - /// - Still mark the current task as succeeded. + /// - Still mark `task_name` as succeeded. #[async_backtrace::framed] #[fastrace::trace] pub async fn get_next_ready_tasks( From 1b6d0637c4dfbd62ab3e34715eb4e52f208867bf Mon Sep 17 00:00:00 2001 From: kould Date: Thu, 7 Aug 2025 17:59:47 +0800 Subject: [PATCH 24/28] chore: codefmt --- src/query/management/src/task/task_mgr.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/query/management/src/task/task_mgr.rs b/src/query/management/src/task/task_mgr.rs index 16c6eef064fe9..743e93f2ebef3 100644 --- a/src/query/management/src/task/task_mgr.rs +++ b/src/query/management/src/task/task_mgr.rs @@ -424,15 +424,14 @@ impl TaskMgr { /// - `Vec`: A list of dependent task names that are ready to proceed. /// /// # Behavior - /// 1. Retrieves all tasks that the given `task_name` is a dependency of (i.e., tasks that depend on `task_name`). - /// 2. For each such dependent task: - /// - Check whether all its dependencies (excluding `task_name`) have succeeded. - /// - If so: - /// - Mark that task as **not succeeded** to trigger reevaluation or blocking. - /// - Mark `task_name` as succeeded. - /// - Add the task to the ready list for further processing. - /// - Otherwise: - /// - Still mark `task_name` as succeeded. + /// 1. Retrieves all tasks that depend on the given `task_name`. + /// 2. For each dependent task: + /// - Check whether all its dependencies have succeeded. + /// - If all dependencies are complete: + /// - Add the dependent task to the ready list. + /// - Set the status of its dependencies to `not succeeded`. + /// - If not all dependencies of the dependent task are complete: + /// - Only mark `task_name` as succeeded. #[async_backtrace::framed] #[fastrace::trace] pub async fn get_next_ready_tasks( From f10608f2b2163e9e1b9e0fa55955bc0a0c2b584f Mon Sep 17 00:00:00 2001 From: kould Date: Fri, 8 Aug 2025 00:27:30 +0800 Subject: [PATCH 25/28] fix: task state will be affected by `get_next_ready_tasks` --- src/meta/app/src/principal/mod.rs | 2 +- src/meta/app/src/principal/task.rs | 32 ++++++- .../app/src/principal/task_state_ident.rs | 22 +++-- .../src/task_from_to_protobuf_impl.rs | 6 +- .../proto-conv/tests/it/v141_task_state.rs | 2 +- src/meta/protos/proto/task.proto | 14 +-- src/query/management/src/task/task_mgr.rs | 96 ++++++++++--------- 7 files changed, 105 insertions(+), 69 deletions(-) diff --git a/src/meta/app/src/principal/mod.rs b/src/meta/app/src/principal/mod.rs index 2f711358dabe7..4808d141c8a12 100644 --- a/src/meta/app/src/principal/mod.rs +++ b/src/meta/app/src/principal/mod.rs @@ -101,7 +101,7 @@ pub use task::TaskDependentKey; pub use task::TaskDependentValue; pub use task::TaskMessage; pub use task::TaskRun; -pub use task::TaskState; +pub use task::TaskStateValue; pub use task::WarehouseOptions; pub use task_ident::TaskIdent; pub use task_ident::TaskIdentRaw; diff --git a/src/meta/app/src/principal/task.rs b/src/meta/app/src/principal/task.rs index 8c67ad465f07e..cf72b918804d4 100644 --- a/src/meta/app/src/principal/task.rs +++ b/src/meta/app/src/principal/task.rs @@ -175,7 +175,19 @@ impl TaskMessage { } #[derive(Debug, Clone, PartialEq)] -pub struct TaskState { +pub struct TaskStateKey { + pub current: String, + pub next: String, +} + +impl TaskStateKey { + pub fn new(current: String, next: String) -> Self { + Self { current, next } + } +} + +#[derive(Debug, Clone, PartialEq)] +pub struct TaskStateValue { pub is_succeeded: bool, } @@ -202,11 +214,29 @@ pub struct TaskDependentValue(pub BTreeSet); mod kvapi_key_impl { use databend_common_meta_kvapi::kvapi; + use databend_common_meta_kvapi::kvapi::KeyBuilder; use databend_common_meta_kvapi::kvapi::KeyError; + use databend_common_meta_kvapi::kvapi::KeyParser; + use crate::principal::task::TaskStateKey; use crate::principal::DependentType; use crate::principal::TaskDependentKey; + impl kvapi::KeyCodec for TaskStateKey { + fn encode_key(&self, b: KeyBuilder) -> KeyBuilder { + b.push_str(self.current.as_str()) + .push_str(self.next.as_str()) + } + + fn decode_key(parser: &mut KeyParser) -> Result + where Self: Sized { + let current = parser.next_str()?; + let next = parser.next_str()?; + + Ok(Self { current, next }) + } + } + impl kvapi::KeyCodec for TaskDependentKey { fn encode_key(&self, b: kvapi::KeyBuilder) -> kvapi::KeyBuilder { match self.ty { diff --git a/src/meta/app/src/principal/task_state_ident.rs b/src/meta/app/src/principal/task_state_ident.rs index 7b973e2b50fb4..3f41242a49ff5 100644 --- a/src/meta/app/src/principal/task_state_ident.rs +++ b/src/meta/app/src/principal/task_state_ident.rs @@ -14,16 +14,26 @@ use crate::tenant_key::ident::TIdent; -pub type TaskStateIdent = TIdent; - -pub type TaskStateIdentRaw = TIdent; +pub type TaskStateIdent = TIdent; + +impl TaskStateIdent { + pub fn new(tenant: impl ToTenant, current: impl ToString, next: impl ToString) -> Self { + TaskStateIdent::new_generic( + tenant, + TaskStateKey::new(current.to_string(), next.to_string()), + ) + } +} pub use kvapi_impl::Resource; +use crate::principal::task::TaskStateKey; +use crate::tenant::ToTenant; + mod kvapi_impl { use databend_common_meta_kvapi::kvapi; - use crate::principal::task::TaskState; + use crate::principal::task::TaskStateValue; use crate::principal::task_state_ident::TaskStateIdent; use crate::tenant_key::resource::TenantResource; @@ -32,10 +42,10 @@ mod kvapi_impl { const PREFIX: &'static str = "__fd_task_states"; const TYPE: &'static str = "TaskStateIdent"; const HAS_TENANT: bool = true; - type ValueType = TaskState; + type ValueType = TaskStateValue; } - impl kvapi::Value for TaskState { + impl kvapi::Value for TaskStateValue { type KeyType = TaskStateIdent; fn dependency_keys(&self, _key: &Self::KeyType) -> impl IntoIterator { diff --git a/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs b/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs index 3cc62a1b9b1c2..bfea58da721ee 100644 --- a/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs +++ b/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs @@ -219,8 +219,8 @@ impl FromToProto for mt::TaskDependentValue { } } -impl FromToProto for mt::TaskState { - type PB = pb::TaskState; +impl FromToProto for mt::TaskStateValue { + type PB = pb::TaskStateValue; fn get_pb_ver(p: &Self::PB) -> u64 { p.ver @@ -234,7 +234,7 @@ impl FromToProto for mt::TaskState { } fn to_pb(&self) -> Result { - Ok(pb::TaskState { + Ok(pb::TaskStateValue { ver: VER, min_reader_ver: MIN_READER_VER, is_succeeded: self.is_succeeded, diff --git a/src/meta/proto-conv/tests/it/v141_task_state.rs b/src/meta/proto-conv/tests/it/v141_task_state.rs index df6f0d8458fd6..e92d403d23234 100644 --- a/src/meta/proto-conv/tests/it/v141_task_state.rs +++ b/src/meta/proto-conv/tests/it/v141_task_state.rs @@ -23,7 +23,7 @@ use crate::common; fn test_decode_v141_task_state() -> anyhow::Result<()> { let task_state_v141 = vec![8, 1, 160, 6, 141, 1, 168, 6, 24]; - let want = || mt::TaskState { is_succeeded: true }; + let want = || mt::TaskStateValue { is_succeeded: true }; common::test_pb_from_to(func_name!(), want())?; common::test_load_old(func_name!(), task_state_v141.as_slice(), 141, want())?; diff --git a/src/meta/protos/proto/task.proto b/src/meta/protos/proto/task.proto index 638636644f441..0beb00cd54c7e 100644 --- a/src/meta/protos/proto/task.proto +++ b/src/meta/protos/proto/task.proto @@ -79,25 +79,13 @@ message Task { string owner_user = 21; } -message TaskState { +message TaskStateValue { uint64 ver = 100; uint64 min_reader_ver = 101; bool is_succeeded = 1; } -message TaskDependentKey { - uint64 ver = 100; - uint64 min_reader_ver = 101; - - enum DependentType { - After = 0; - Before = 1; - } - DependentType ty = 1; - string source = 2; -} - message TaskDependentValue { uint64 ver = 100; uint64 min_reader_ver = 101; diff --git a/src/query/management/src/task/task_mgr.rs b/src/query/management/src/task/task_mgr.rs index 743e93f2ebef3..e4a1cfea56703 100644 --- a/src/query/management/src/task/task_mgr.rs +++ b/src/query/management/src/task/task_mgr.rs @@ -27,7 +27,7 @@ use databend_common_meta_api::util::txn_put_pb; use databend_common_meta_api::SchemaApi; use databend_common_meta_app::principal::task; use databend_common_meta_app::principal::task::TaskMessage; -use databend_common_meta_app::principal::task::TaskState; +use databend_common_meta_app::principal::task::TaskStateValue; use databend_common_meta_app::principal::task_dependent_ident::TaskDependentIdent; use databend_common_meta_app::principal::task_message_ident::TaskMessageIdent; use databend_common_meta_app::principal::task_state_ident::TaskStateIdent; @@ -47,6 +47,7 @@ use databend_common_meta_types::TxnOp; use databend_common_meta_types::TxnRequest; use databend_common_meta_types::With; use databend_common_proto_conv::FromToProto; +use futures::StreamExt; use futures::TryStreamExt; use prost::Message; use seq_marked::SeqValue; @@ -266,20 +267,20 @@ impl TaskMgr { task_name: &str, new_afters: &[String], ) -> Result, TaskApiError> { - let mut update_ops = Vec::new(); - let mut check_ops = Vec::with_capacity(new_afters.len()); - + let mut request = TxnRequest::new(vec![], vec![]); let after_dependent_ident = TaskDependentIdent::new_after(&self.tenant, task_name); let after_seq_deps = self.kv_api.get_pb(&after_dependent_ident).await?; - check_ops.push(txn_cond_eq_seq( + request.condition.push(txn_cond_eq_seq( &after_dependent_ident, after_seq_deps.seq(), )); let mut after_deps = after_seq_deps.unwrap_or_default(); after_deps.0.extend(new_afters.iter().cloned()); - update_ops.push(txn_put_pb(&after_dependent_ident, &after_deps)?); + request + .if_then + .push(txn_put_pb(&after_dependent_ident, &after_deps)?); let before_dependent_idents = new_afters .iter() @@ -287,16 +288,17 @@ impl TaskMgr { for (before_dependent_ident, before_seq_deps) in self.kv_api.get_pb_vec(before_dependent_idents).await? { - check_ops.push(txn_cond_eq_seq( + request.condition.push(txn_cond_eq_seq( &before_dependent_ident, before_seq_deps.seq(), )); let mut deps = before_seq_deps.unwrap_or_default(); deps.0.insert(task_name.to_string()); - update_ops.push(txn_put_pb(&before_dependent_ident, &deps)?); + request + .if_then + .push(txn_put_pb(&before_dependent_ident, &deps)?); } - let request = TxnRequest::new(check_ops, update_ops); let reply = self.kv_api.transaction(request).await?; if !reply.success { @@ -316,12 +318,11 @@ impl TaskMgr { task_name: &str, remove_afters: &[String], ) -> Result, TaskApiError> { - let mut update_ops = Vec::new(); - let mut check_ops = Vec::with_capacity(remove_afters.len()); + let mut request = TxnRequest::new(vec![], vec![]); let after_dependent_ident = TaskDependentIdent::new_after(&self.tenant, task_name); let after_seq_deps = self.kv_api.get_pb(&after_dependent_ident).await?; - check_ops.push(txn_cond_eq_seq( + request.condition.push(txn_cond_eq_seq( &after_dependent_ident, after_seq_deps.seq(), )); @@ -330,7 +331,9 @@ impl TaskMgr { for remove_after in remove_afters { deps.0.remove(remove_after); } - update_ops.push(txn_put_pb(&after_dependent_ident, &deps)?); + request + .if_then + .push(txn_put_pb(&after_dependent_ident, &deps)?); } let before_dependent_idents = remove_afters @@ -339,17 +342,18 @@ impl TaskMgr { for (before_dependent_ident, before_seq_deps) in self.kv_api.get_pb_vec(before_dependent_idents).await? { - check_ops.push(txn_cond_eq_seq( + request.condition.push(txn_cond_eq_seq( &before_dependent_ident, before_seq_deps.seq(), )); if let Some(mut deps) = before_seq_deps { deps.0.remove(task_name); - update_ops.push(txn_put_pb(&before_dependent_ident, &deps)?); + request + .if_then + .push(txn_put_pb(&before_dependent_ident, &deps)?); } } - let request = TxnRequest::new(check_ops, update_ops); let reply = self.kv_api.transaction(request).await?; if !reply.success { @@ -369,8 +373,7 @@ impl TaskMgr { &self, task_name: &str, ) -> Result, TaskApiError> { - let mut check_ops = Vec::new(); - let mut update_ops = Vec::new(); + let mut request = TxnRequest::new(vec![], vec![]); let task_after_ident = TaskDependentIdent::new_after(&self.tenant, task_name); let task_before_ident = TaskDependentIdent::new_before(&self.tenant, task_name); @@ -387,24 +390,33 @@ impl TaskMgr { })); } for (target_ident, seq_dep) in self.kv_api.get_pb_vec(target_idents).await? { - check_ops.push(txn_cond_eq_seq(&target_ident, seq_dep.seq())); + request + .condition + .push(txn_cond_eq_seq(&target_ident, seq_dep.seq())); if let Some(mut deps) = seq_dep { deps.0.remove(task_name); - update_ops.push(txn_put_pb(&target_ident, &deps)?); + request.if_then.push(txn_put_pb(&target_ident, &deps)?); } } - update_ops.push(TxnOp::delete( + request.if_then.push(TxnOp::delete( TaskDependentIdent::new_before(&self.tenant, task_name).to_string_key(), )); - update_ops.push(TxnOp::delete( + request.if_then.push(TxnOp::delete( TaskDependentIdent::new_after(&self.tenant, task_name).to_string_key(), )); - update_ops.push(TxnOp::delete( - TaskStateIdent::new(&self.tenant, task_name).to_string_key(), - )); + let mut stream = self + .kv_api + .list_pb_keys(&DirName::new(TaskStateIdent::new( + &self.tenant, + task_name, + "", + ))) + .await?; - let request = TxnRequest::new(check_ops, update_ops); + while let Some(result) = stream.next().await { + request.if_then.push(TxnOp::delete(result?.to_string_key())) + } let _ = self.kv_api.transaction(request).await?; Ok(Ok(())) @@ -439,9 +451,8 @@ impl TaskMgr { task_name: &str, ) -> Result, TaskError>, TaskApiError> { let task_before_ident = TaskDependentIdent::new_before(&self.tenant, task_name); - let task_state_key = TaskStateIdent::new(&self.tenant, task_name); - let succeeded_value = TaskState { is_succeeded: true }; - let not_succeeded_value = TaskState { + let succeeded_value = TaskStateValue { is_succeeded: true }; + let not_succeeded_value = TaskStateValue { is_succeeded: false, }; @@ -460,13 +471,19 @@ impl TaskMgr { let Some(target_after_dependent) = target_after_dependent else { continue; }; - let mut request = TxnRequest::new(vec![], vec![]) - .with_else(vec![txn_put_pb(&task_state_key, &succeeded_value)?]); - - for target_after_task in target_after_dependent.0.iter() { - let task_ident = TaskStateIdent::new(&self.tenant, target_after_task); + let target_after = &target_after_ident.name().source; + let this_task_to_target_state = + TaskStateIdent::new(&self.tenant, task_name, target_after); + let mut request = TxnRequest::new(vec![], vec![]).with_else(vec![txn_put_pb( + &this_task_to_target_state, + &succeeded_value, + )?]); + + for before_target_after in target_after_dependent.0.iter() { + let task_ident = + TaskStateIdent::new(&self.tenant, before_target_after, target_after); // Only care about the predecessors of this task's successor tasks, excluding this task itself. - if target_after_task != task_name { + if before_target_after != task_name { request.condition.push(TxnCondition::eq_value( task_ident.to_string_key(), succeeded_value.to_pb()?.encode_to_vec(), @@ -485,15 +502,6 @@ impl TaskMgr { Ok(Ok(ready_tasks)) } - #[async_backtrace::framed] - #[fastrace::trace] - pub async fn clean_task_state(&self, task_name: &str) -> Result<(), TaskApiError> { - let key = TaskStateIdent::new(&self.tenant, task_name); - let req = UpsertPB::delete(key).with(MatchSeq::GE(1)); - let _ = self.kv_api.upsert_pb(&req).await?; - Ok(()) - } - async fn create_task_inner( &self, task: Task, From 6a37789ce5f8f2c9fd3e66ca4643ec1f070a1f2f Mon Sep 17 00:00:00 2001 From: kould Date: Fri, 8 Aug 2025 11:07:30 +0800 Subject: [PATCH 26/28] chore: codefmt --- src/query/management/src/task/task_mgr.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/query/management/src/task/task_mgr.rs b/src/query/management/src/task/task_mgr.rs index e4a1cfea56703..acdc3278721fe 100644 --- a/src/query/management/src/task/task_mgr.rs +++ b/src/query/management/src/task/task_mgr.rs @@ -436,14 +436,19 @@ impl TaskMgr { /// - `Vec`: A list of dependent task names that are ready to proceed. /// /// # Behavior - /// 1. Retrieves all tasks that depend on the given `task_name`. - /// 2. For each dependent task: - /// - Check whether all its dependencies have succeeded. + /// Assume: + /// - `a` = given `task_name` (the completed task) + /// - `b` = a task that has `a` in its `AFTER` list (i.e., depends on `a`) + /// - `c` = other tasks in `b`'s `AFTER` list (other dependencies of `b`) + /// + /// 1. Retrieves all tasks (`b`) that have `a` in their `AFTER` list. + /// 2. For each `b`: + /// - Check whether all its dependencies (`a` + `c`) have succeeded. /// - If all dependencies are complete: - /// - Add the dependent task to the ready list. - /// - Set the status of its dependencies to `not succeeded`. - /// - If not all dependencies of the dependent task are complete: - /// - Only mark `task_name` as succeeded. + /// - Add `b` to the ready list. + /// - Set the status of its dependencies (`a` + `c`) to `not succeeded`. + /// - If not all dependencies of `b` are complete: + /// - Only mark `a` as succeeded. #[async_backtrace::framed] #[fastrace::trace] pub async fn get_next_ready_tasks( From be933c36920ef28991c69ab6bf08d76361aef066 Mon Sep 17 00:00:00 2001 From: kould Date: Fri, 8 Aug 2025 13:54:11 +0800 Subject: [PATCH 27/28] chore: codefmt --- src/meta/app/src/principal/mod.rs | 2 +- src/meta/app/src/principal/task.rs | 36 ++++++++++--------- .../app/src/principal/task_state_ident.rs | 30 +++++++++------- .../src/task_from_to_protobuf_impl.rs | 9 ++--- .../proto-conv/tests/it/v141_task_state.rs | 4 +-- src/meta/protos/proto/task.proto | 2 -- src/query/management/src/task/task_mgr.rs | 34 ++++++++---------- 7 files changed, 58 insertions(+), 59 deletions(-) diff --git a/src/meta/app/src/principal/mod.rs b/src/meta/app/src/principal/mod.rs index 4808d141c8a12..d349b99190c8b 100644 --- a/src/meta/app/src/principal/mod.rs +++ b/src/meta/app/src/principal/mod.rs @@ -101,7 +101,7 @@ pub use task::TaskDependentKey; pub use task::TaskDependentValue; pub use task::TaskMessage; pub use task::TaskRun; -pub use task::TaskStateValue; +pub use task::TaskSucceededStateValue; pub use task::WarehouseOptions; pub use task_ident::TaskIdent; pub use task_ident::TaskIdentRaw; diff --git a/src/meta/app/src/principal/task.rs b/src/meta/app/src/principal/task.rs index cf72b918804d4..43e358d4a1387 100644 --- a/src/meta/app/src/principal/task.rs +++ b/src/meta/app/src/principal/task.rs @@ -175,21 +175,22 @@ impl TaskMessage { } #[derive(Debug, Clone, PartialEq)] -pub struct TaskStateKey { - pub current: String, - pub next: String, +pub struct TaskSucceededStateKey { + pub before_task: String, + pub after_task: String, } -impl TaskStateKey { - pub fn new(current: String, next: String) -> Self { - Self { current, next } +impl TaskSucceededStateKey { + pub fn new(before_task: String, after_task: String) -> Self { + Self { + before_task, + after_task, + } } } #[derive(Debug, Clone, PartialEq)] -pub struct TaskStateValue { - pub is_succeeded: bool, -} +pub struct TaskSucceededStateValue; #[derive(Debug, Clone, Copy, PartialEq)] pub enum DependentType { @@ -218,22 +219,25 @@ mod kvapi_key_impl { use databend_common_meta_kvapi::kvapi::KeyError; use databend_common_meta_kvapi::kvapi::KeyParser; - use crate::principal::task::TaskStateKey; + use crate::principal::task::TaskSucceededStateKey; use crate::principal::DependentType; use crate::principal::TaskDependentKey; - impl kvapi::KeyCodec for TaskStateKey { + impl kvapi::KeyCodec for TaskSucceededStateKey { fn encode_key(&self, b: KeyBuilder) -> KeyBuilder { - b.push_str(self.current.as_str()) - .push_str(self.next.as_str()) + b.push_str(self.before_task.as_str()) + .push_str(self.after_task.as_str()) } fn decode_key(parser: &mut KeyParser) -> Result where Self: Sized { - let current = parser.next_str()?; - let next = parser.next_str()?; + let before_task = parser.next_str()?; + let after_task = parser.next_str()?; - Ok(Self { current, next }) + Ok(Self { + before_task, + after_task, + }) } } diff --git a/src/meta/app/src/principal/task_state_ident.rs b/src/meta/app/src/principal/task_state_ident.rs index 3f41242a49ff5..b4b0abf75367c 100644 --- a/src/meta/app/src/principal/task_state_ident.rs +++ b/src/meta/app/src/principal/task_state_ident.rs @@ -14,39 +14,43 @@ use crate::tenant_key::ident::TIdent; -pub type TaskStateIdent = TIdent; - -impl TaskStateIdent { - pub fn new(tenant: impl ToTenant, current: impl ToString, next: impl ToString) -> Self { - TaskStateIdent::new_generic( +pub type TaskSucceededStateIdent = TIdent; + +impl TaskSucceededStateIdent { + pub fn new( + tenant: impl ToTenant, + before_task: impl ToString, + after_task: impl ToString, + ) -> Self { + TaskSucceededStateIdent::new_generic( tenant, - TaskStateKey::new(current.to_string(), next.to_string()), + TaskSucceededStateKey::new(before_task.to_string(), after_task.to_string()), ) } } pub use kvapi_impl::Resource; -use crate::principal::task::TaskStateKey; +use crate::principal::task::TaskSucceededStateKey; use crate::tenant::ToTenant; mod kvapi_impl { use databend_common_meta_kvapi::kvapi; - use crate::principal::task::TaskStateValue; - use crate::principal::task_state_ident::TaskStateIdent; + use crate::principal::task_state_ident::TaskSucceededStateIdent; + use crate::principal::TaskSucceededStateValue; use crate::tenant_key::resource::TenantResource; pub struct Resource; impl TenantResource for Resource { const PREFIX: &'static str = "__fd_task_states"; - const TYPE: &'static str = "TaskStateIdent"; + const TYPE: &'static str = "TaskSucceededStateIdent"; const HAS_TENANT: bool = true; - type ValueType = TaskStateValue; + type ValueType = TaskSucceededStateValue; } - impl kvapi::Value for TaskStateValue { - type KeyType = TaskStateIdent; + impl kvapi::Value for TaskSucceededStateValue { + type KeyType = TaskSucceededStateIdent; fn dependency_keys(&self, _key: &Self::KeyType) -> impl IntoIterator { [] diff --git a/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs b/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs index bfea58da721ee..a98a97e19371d 100644 --- a/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs +++ b/src/meta/proto-conv/src/task_from_to_protobuf_impl.rs @@ -219,25 +219,22 @@ impl FromToProto for mt::TaskDependentValue { } } -impl FromToProto for mt::TaskStateValue { +impl FromToProto for mt::TaskSucceededStateValue { type PB = pb::TaskStateValue; fn get_pb_ver(p: &Self::PB) -> u64 { p.ver } - fn from_pb(p: Self::PB) -> Result + fn from_pb(_p: Self::PB) -> Result where Self: Sized { - Ok(Self { - is_succeeded: p.is_succeeded, - }) + Ok(Self) } fn to_pb(&self) -> Result { Ok(pb::TaskStateValue { ver: VER, min_reader_ver: MIN_READER_VER, - is_succeeded: self.is_succeeded, }) } } diff --git a/src/meta/proto-conv/tests/it/v141_task_state.rs b/src/meta/proto-conv/tests/it/v141_task_state.rs index e92d403d23234..a4f8db05cd091 100644 --- a/src/meta/proto-conv/tests/it/v141_task_state.rs +++ b/src/meta/proto-conv/tests/it/v141_task_state.rs @@ -21,9 +21,9 @@ use crate::common; #[test] fn test_decode_v141_task_state() -> anyhow::Result<()> { - let task_state_v141 = vec![8, 1, 160, 6, 141, 1, 168, 6, 24]; + let task_state_v141 = vec![160, 6, 141, 1, 168, 6, 24]; - let want = || mt::TaskStateValue { is_succeeded: true }; + let want = || mt::TaskSucceededStateValue; common::test_pb_from_to(func_name!(), want())?; common::test_load_old(func_name!(), task_state_v141.as_slice(), 141, want())?; diff --git a/src/meta/protos/proto/task.proto b/src/meta/protos/proto/task.proto index 0beb00cd54c7e..c6caa8bfe0090 100644 --- a/src/meta/protos/proto/task.proto +++ b/src/meta/protos/proto/task.proto @@ -82,8 +82,6 @@ message Task { message TaskStateValue { uint64 ver = 100; uint64 min_reader_ver = 101; - - bool is_succeeded = 1; } message TaskDependentValue { diff --git a/src/query/management/src/task/task_mgr.rs b/src/query/management/src/task/task_mgr.rs index acdc3278721fe..b74e6969a7521 100644 --- a/src/query/management/src/task/task_mgr.rs +++ b/src/query/management/src/task/task_mgr.rs @@ -23,14 +23,15 @@ use databend_common_ast::ast::ScheduleOptions; use databend_common_meta_api::kv_pb_api::KVPbApi; use databend_common_meta_api::kv_pb_api::UpsertPB; use databend_common_meta_api::txn_cond_eq_seq; +use databend_common_meta_api::txn_op_del; use databend_common_meta_api::util::txn_put_pb; use databend_common_meta_api::SchemaApi; use databend_common_meta_app::principal::task; use databend_common_meta_app::principal::task::TaskMessage; -use databend_common_meta_app::principal::task::TaskStateValue; +use databend_common_meta_app::principal::task::TaskSucceededStateValue; use databend_common_meta_app::principal::task_dependent_ident::TaskDependentIdent; use databend_common_meta_app::principal::task_message_ident::TaskMessageIdent; -use databend_common_meta_app::principal::task_state_ident::TaskStateIdent; +use databend_common_meta_app::principal::task_state_ident::TaskSucceededStateIdent; use databend_common_meta_app::principal::ScheduleType; use databend_common_meta_app::principal::Status; use databend_common_meta_app::principal::Task; @@ -40,16 +41,16 @@ use databend_common_meta_app::tenant::Tenant; use databend_common_meta_kvapi::kvapi; use databend_common_meta_kvapi::kvapi::DirName; use databend_common_meta_kvapi::kvapi::Key; +use databend_common_meta_types::txn_condition::Target; +use databend_common_meta_types::ConditionResult; use databend_common_meta_types::MatchSeq; use databend_common_meta_types::MetaError; use databend_common_meta_types::TxnCondition; use databend_common_meta_types::TxnOp; use databend_common_meta_types::TxnRequest; use databend_common_meta_types::With; -use databend_common_proto_conv::FromToProto; use futures::StreamExt; use futures::TryStreamExt; -use prost::Message; use seq_marked::SeqValue; use crate::task::errors::TaskApiError; @@ -407,7 +408,7 @@ impl TaskMgr { )); let mut stream = self .kv_api - .list_pb_keys(&DirName::new(TaskStateIdent::new( + .list_pb_keys(&DirName::new(TaskSucceededStateIdent::new( &self.tenant, task_name, "", @@ -456,10 +457,6 @@ impl TaskMgr { task_name: &str, ) -> Result, TaskError>, TaskApiError> { let task_before_ident = TaskDependentIdent::new_before(&self.tenant, task_name); - let succeeded_value = TaskStateValue { is_succeeded: true }; - let not_succeeded_value = TaskStateValue { - is_succeeded: false, - }; let Some(task_before_dependent) = self.kv_api.get_pb(&task_before_ident).await? else { return Ok(Ok(Vec::new())); @@ -478,25 +475,24 @@ impl TaskMgr { }; let target_after = &target_after_ident.name().source; let this_task_to_target_state = - TaskStateIdent::new(&self.tenant, task_name, target_after); + TaskSucceededStateIdent::new(&self.tenant, task_name, target_after); let mut request = TxnRequest::new(vec![], vec![]).with_else(vec![txn_put_pb( &this_task_to_target_state, - &succeeded_value, + &TaskSucceededStateValue, )?]); for before_target_after in target_after_dependent.0.iter() { let task_ident = - TaskStateIdent::new(&self.tenant, before_target_after, target_after); + TaskSucceededStateIdent::new(&self.tenant, before_target_after, target_after); // Only care about the predecessors of this task's successor tasks, excluding this task itself. if before_target_after != task_name { - request.condition.push(TxnCondition::eq_value( - task_ident.to_string_key(), - succeeded_value.to_pb()?.encode_to_vec(), - )); + request.condition.push(TxnCondition { + key: task_ident.to_string_key(), + expected: ConditionResult::Gt as i32, + target: Some(Target::Seq(0)), + }); } - request - .if_then - .push(txn_put_pb(&task_ident, ¬_succeeded_value)?); + request.if_then.push(txn_op_del(&task_ident)); } let reply = self.kv_api.transaction(request).await?; From 14d3cadc9ba9eb7406fab314e60054a20f26081a Mon Sep 17 00:00:00 2001 From: kould Date: Fri, 8 Aug 2025 14:09:55 +0800 Subject: [PATCH 28/28] chore: codefmt --- src/query/management/src/task/task_mgr.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/query/management/src/task/task_mgr.rs b/src/query/management/src/task/task_mgr.rs index b74e6969a7521..d7872a885ec4a 100644 --- a/src/query/management/src/task/task_mgr.rs +++ b/src/query/management/src/task/task_mgr.rs @@ -444,12 +444,12 @@ impl TaskMgr { /// /// 1. Retrieves all tasks (`b`) that have `a` in their `AFTER` list. /// 2. For each `b`: - /// - Check whether all its dependencies (`a` + `c`) have succeeded. + /// - Check whether all its dependencies (`a/b` + `c/b`) have succeeded. /// - If all dependencies are complete: /// - Add `b` to the ready list. - /// - Set the status of its dependencies (`a` + `c`) to `not succeeded`. + /// - Set the status of its dependencies (`a/b` + `c/b`) to `not succeeded`. /// - If not all dependencies of `b` are complete: - /// - Only mark `a` as succeeded. + /// - Only mark `a/b` as succeeded. #[async_backtrace::framed] #[fastrace::trace] pub async fn get_next_ready_tasks(