From 6f5017d5715fcb5b8230d3cff7ad4100c28c9298 Mon Sep 17 00:00:00 2001 From: obdev Date: Mon, 12 Dec 2022 16:11:36 +0000 Subject: [PATCH] [CP] fix offline ddl and drop index concurrency. --- .../ddl_task/ob_drop_index_task.cpp | 8 ++--- .../ddl_task/ob_index_build_task.cpp | 3 +- src/rootserver/ob_ddl_operator.cpp | 4 +++ src/rootserver/ob_ddl_operator.h | 1 + src/rootserver/ob_ddl_service.cpp | 36 ++++++++++--------- src/rootserver/ob_ddl_service.h | 4 +-- src/rootserver/ob_index_builder.cpp | 7 ++-- src/share/ob_ddl_common.h | 2 +- src/share/ob_debug_sync_point.h | 1 - 9 files changed, 36 insertions(+), 30 deletions(-) diff --git a/src/rootserver/ddl_task/ob_drop_index_task.cpp b/src/rootserver/ddl_task/ob_drop_index_task.cpp index 70865d3f4..002a9b129 100644 --- a/src/rootserver/ddl_task/ob_drop_index_task.cpp +++ b/src/rootserver/ddl_task/ob_drop_index_task.cpp @@ -156,15 +156,13 @@ int ObDropIndexTask::prepare(const ObDDLTaskStatus new_status) return ret; } -int ObDropIndexTask::set_write_only(const ObDDLTaskStatus new_status) +// Disused stage, just for compatibility. +int ObDropIndexTask::set_write_only(const share::ObDDLTaskStatus new_status) { int ret = OB_SUCCESS; - DEBUG_SYNC(DROP_INDEX_SET_WRITE_ONLY); if (OB_UNLIKELY(!is_inited_)) { ret = OB_NOT_INIT; LOG_WARN("ObDropIndexTask has not been inited", K(ret)); - } else if (OB_FAIL(update_index_status(INDEX_STATUS_UNAVAILABLE))) { - LOG_WARN("update index status failed", K(ret)); } else if (OB_FAIL(switch_status(new_status, ret))) { LOG_WARN("switch status failed", K(ret)); } @@ -311,7 +309,7 @@ int ObDropIndexTask::process() break; case ObDDLTaskStatus::SET_WRITE_ONLY: if (OB_FAIL(set_write_only(WAIT_TRANS_END_FOR_WRITE_ONLY))) { - LOG_WARN("prepare failed", K(ret)); + LOG_WARN("set write only failed", K(ret)); } break; case ObDDLTaskStatus::WAIT_TRANS_END_FOR_WRITE_ONLY: diff --git a/src/rootserver/ddl_task/ob_index_build_task.cpp b/src/rootserver/ddl_task/ob_index_build_task.cpp index 8e2f7dd35..7838d9935 100644 --- a/src/rootserver/ddl_task/ob_index_build_task.cpp +++ b/src/rootserver/ddl_task/ob_index_build_task.cpp @@ -1140,7 +1140,8 @@ int ObIndexBuildTask::clean_on_failed() index_name = "__fake"; } else if (OB_FAIL(index_schema->get_index_name(index_name))) { LOG_WARN("get index name failed", K(ret)); - } else { + } else if (0 == parent_task_id_) { + // generate ddl_stmt if it is not a child task. if (is_oracle_mode) { if (OB_FAIL(drop_index_sql.append_fmt("drop index \"%.*s\"", index_name.length(), index_name.ptr()))) { LOG_WARN("generate drop index sql failed", K(ret)); diff --git a/src/rootserver/ob_ddl_operator.cpp b/src/rootserver/ob_ddl_operator.cpp index a390dfe5d..51e22801d 100644 --- a/src/rootserver/ob_ddl_operator.cpp +++ b/src/rootserver/ob_ddl_operator.cpp @@ -3192,6 +3192,7 @@ int ObDDLOperator::alter_table_rename_index( const uint64_t data_table_id, const uint64_t database_id, const obrpc::ObRenameIndexArg &rename_index_arg, + const ObIndexStatus *new_index_status, common::ObMySQLTransaction &trans, schema::ObTableSchema &new_index_table_schema) { @@ -3244,6 +3245,9 @@ int ObDDLOperator::alter_table_rename_index( LOG_WARN("fail to assign schema", K(ret)); } else { new_index_table_schema.set_schema_version(new_schema_version); + if (nullptr != new_index_status) { + new_index_table_schema.set_index_status(*new_index_status); + } } if (OB_FAIL(ret)) { } else if (OB_FAIL(new_index_table_schema.set_table_name(new_index_table_name))) { diff --git a/src/rootserver/ob_ddl_operator.h b/src/rootserver/ob_ddl_operator.h index cd18835f0..542deb452 100644 --- a/src/rootserver/ob_ddl_operator.h +++ b/src/rootserver/ob_ddl_operator.h @@ -344,6 +344,7 @@ public: const uint64_t data_table_id, const uint64_t database_id, const obrpc::ObRenameIndexArg &rename_index_arg, + const ObIndexStatus *new_index_status, common::ObMySQLTransaction &trans, share::schema::ObTableSchema &new_index_table_schema); virtual int alter_index_table_parallel(const uint64_t tenant_id, diff --git a/src/rootserver/ob_ddl_service.cpp b/src/rootserver/ob_ddl_service.cpp index 1816520f9..b3f573f5e 100644 --- a/src/rootserver/ob_ddl_service.cpp +++ b/src/rootserver/ob_ddl_service.cpp @@ -5151,6 +5151,7 @@ int ObDDLService::alter_table_index(const obrpc::ObAlterTableArg &alter_table_ar origin_table_schema.get_table_id(), origin_table_schema.get_database_id(), *rename_index_arg, + nullptr /* new_index_status */, trans, new_index_schema))) { LOG_WARN("failed to rename index", K(*rename_index_arg), K(ret)); @@ -5816,22 +5817,24 @@ int ObDDLService::rename_dropping_index_name( if (OB_FAIL(get_index_schema_by_name(data_table_id, database_id, drop_index_arg, schema_guard, index_table_schema))) { LOG_WARN("get index schema by name", K(ret), K(data_table_id), K(database_id)); - } else if ((nwrite = snprintf(buf, buf_size, "%s_%s_%lu", - index_name.ptr(), "DELETING", ObTimeUtility::current_time())) >= buf_size || nwrite < 0) { + } else if ((nwrite = snprintf(buf, buf_size, "%s_%lu", + "DELETING", ObTimeUtility::current_time())) >= buf_size || nwrite < 0) { ret = common::OB_BUF_NOT_ENOUGH; LOG_WARN("buf is not large enough", K(ret), K(buf_size)); } else { + const ObIndexStatus new_index_status = INDEX_STATUS_UNAVAILABLE; ObString new_index_name = ObString::make_string(buf); obrpc::ObRenameIndexArg rename_index_arg; rename_index_arg.tenant_id_ = index_table_schema->get_tenant_id(); rename_index_arg.origin_index_name_ = index_name; rename_index_arg.new_index_name_ = new_index_name; if (OB_FAIL(ddl_operator.alter_table_rename_index(index_table_schema->get_tenant_id(), - index_table_schema->get_data_table_id(), - index_table_schema->get_database_id(), - rename_index_arg, - trans, - new_index_schema))) { + index_table_schema->get_data_table_id(), + index_table_schema->get_database_id(), + rename_index_arg, + &new_index_status, + trans, + new_index_schema))) { LOG_WARN("rename index failed", K(ret)); } } @@ -10439,17 +10442,17 @@ int ObDDLService::check_is_offline_ddl(ObAlterTableArg &alter_table_arg, } } if (OB_SUCC(ret) && is_double_table_long_running_ddl(ddl_type)) { - bool is_building_index = false; + bool has_index_operation = false; bool is_adding_constraint = false; uint64_t table_id = alter_table_arg.alter_table_schema_.get_table_id(); - if (OB_FAIL(check_is_building_index(schema_guard, + if (OB_FAIL(check_has_index_operation(schema_guard, tenant_id, table_id, - is_building_index))) { - LOG_WARN("failed to call check_is_building_index", K(ret)); + has_index_operation))) { + LOG_WARN("check has index operation failed", K(ret)); } else if (OB_FAIL(check_is_adding_constraint(table_id, is_adding_constraint))) { LOG_WARN("failed to call check_is_adding_constraint", K(ret)); - } else if (is_building_index) { + } else if (has_index_operation) { ret = OB_NOT_SUPPORTED; LOG_USER_ERROR(OB_NOT_SUPPORTED, "The DDL cannot be run concurrently with creating index."); } else if (is_adding_constraint) { @@ -10461,13 +10464,14 @@ int ObDDLService::check_is_offline_ddl(ObAlterTableArg &alter_table_arg, return ret; } -int ObDDLService::check_is_building_index(ObSchemaGetterGuard &schema_guard, +// check whether there is index operation, including add index and drop index. +int ObDDLService::check_has_index_operation(ObSchemaGetterGuard &schema_guard, const uint64_t tenant_id, const uint64_t table_id, - bool &is_building_index) + bool &has_index_operation) { int ret = OB_SUCCESS; - is_building_index = false; + has_index_operation = false; // 1. get table schema const ObTableSchema *orig_table = nullptr; ObSEArray index_infos; @@ -10495,7 +10499,7 @@ int ObDDLService::check_is_building_index(ObSchemaGetterGuard &schema_guard, LOG_WARN("invalid index table id", "index_table_id", index_infos.at(i).table_id_); } else if (index_schema->is_unavailable_index()) { // 3. check if index is still constructing - is_building_index = true; + has_index_operation = true; break; } } diff --git a/src/rootserver/ob_ddl_service.h b/src/rootserver/ob_ddl_service.h index f8841acd9..b0613adda 100644 --- a/src/rootserver/ob_ddl_service.h +++ b/src/rootserver/ob_ddl_service.h @@ -997,11 +997,11 @@ private: LOCALITY_NOT_CHANGED, ALTER_LOCALITY_INVALID, }; - int check_is_building_index( + int check_has_index_operation( ObSchemaGetterGuard &schema_guard, const uint64_t teannt_id, const uint64_t table_id, - bool &is_building_index); + bool &has_index_operation); int check_is_adding_constraint(const uint64_t table_id, bool &is_building); int modify_tenant_inner_phase(const obrpc::ObModifyTenantArg &arg, const ObTenantSchema *orig_tenant_schema, diff --git a/src/rootserver/ob_index_builder.cpp b/src/rootserver/ob_index_builder.cpp index c1d5198f4..d20e1e558 100644 --- a/src/rootserver/ob_index_builder.cpp +++ b/src/rootserver/ob_index_builder.cpp @@ -138,9 +138,8 @@ int ObIndexBuilder::drop_index(const ObDropIndexArg &arg, obrpc::ObDropIndexRes } if (OB_SUCC(ret)) { - const uint64_t table_id = table_schema->get_table_id(); + const uint64_t data_table_id = table_schema->get_table_id(); const ObTableSchema *index_table_schema = NULL; - if (OB_INVALID_ID != arg.index_table_id_) { LOG_DEBUG("drop index with index_table_id", K(arg.index_table_id_)); if (OB_FAIL(schema_guard.get_table_schema(tenant_id, arg.index_table_id_, index_table_schema))) { @@ -149,8 +148,8 @@ int ObIndexBuilder::drop_index(const ObDropIndexArg &arg, obrpc::ObDropIndexRes } else { ObString index_table_name; if (OB_FAIL(ObTableSchema::build_index_table_name( - allocator, table_id, arg.index_name_, index_table_name))) { - LOG_WARN("build_index_table_name failed", K(arg), K(table_id), K(ret)); + allocator, data_table_id, arg.index_name_, index_table_name))) { + LOG_WARN("build_index_table_name failed", K(arg), K(data_table_id), K(ret)); } else if (OB_FAIL(schema_guard.get_table_schema(tenant_id, table_schema->get_database_id(), index_table_name, diff --git a/src/share/ob_ddl_common.h b/src/share/ob_ddl_common.h index 98ee3cbda..fb2425d45 100644 --- a/src/share/ob_ddl_common.h +++ b/src/share/ob_ddl_common.h @@ -111,7 +111,7 @@ enum ObDDLTaskStatus { CHECK_CONSTRAINT_VALID = 7, SET_CONSTRAINT_VALIDATE = 8, MODIFY_AUTOINC = 9, - SET_WRITE_ONLY = 10, + SET_WRITE_ONLY = 10, // disused, just for compatibility. WAIT_TRANS_END_FOR_WRITE_ONLY = 11, SET_UNUSABLE = 12, WAIT_TRANS_END_FOR_UNUSABLE = 13, diff --git a/src/share/ob_debug_sync_point.h b/src/share/ob_debug_sync_point.h index 03dac3766..e255c8201 100644 --- a/src/share/ob_debug_sync_point.h +++ b/src/share/ob_debug_sync_point.h @@ -365,7 +365,6 @@ class ObString; ACT(BEFORE_MIGRATE_FETCH_SSTABLE_MACRO_INFO,)\ ACT(BEFORE_MIGRATE_FETCH_MACRO_BLOCK,)\ ACT(BEFORE_ADD_BACKUP_TASK_INTO_SCHEDULER,)\ - ACT(DROP_INDEX_SET_WRITE_ONLY,)\ ACT(BEFORE_MIGRATION_BUILD_TABLET_SSTABLE_INFO,)\ ACT(BEFORE_MIGRATION_TABLET_COPY_SSTABLE,)\ ACT(AFTER_MIGRATION_LOAD_LS_INNER_TABLET,)\