dataroaring commented on code in PR #19063:
URL: https://github.com/apache/doris/pull/19063#discussion_r1202422628
##########
be/src/olap/tablet_schema.cpp:
##########
@@ -719,6 +724,17 @@ void TabletSchema::copy_from(const TabletSchema&
tablet_schema) {
init_from_pb(tablet_schema_pb);
}
+void TabletSchema::update_tablet_columns(const TabletSchema& tablet_schema,
+ const std::vector<TColumn>& t_columns) {
+ copy_from(tablet_schema);
+ if (!t_columns.empty() && t_columns[0].col_unique_id >= 0) {
+ clear_columns();
+ for (const auto& column : t_columns) {
+ append_column(TabletColumn(column));
+ }
+ }
+}
+
Review Comment:
We should do some checking here.
##########
be/src/agent/task_worker_pool.h:
##########
@@ -208,10 +208,6 @@ class TaskWorkerPool {
void _push_cooldown_conf_worker_thread_callback();
void _push_storage_policy_worker_thread_callback();
- void _alter_inverted_index(const TAgentTaskRequest&
alter_inverted_index_request,
- int64_t signature, const TTaskType::type
task_type,
- TFinishTaskRequest* finish_task_request);
-
Review Comment:
refer to https://github.com/apache/doris/pull/19734.
##########
be/src/olap/olap_server.cpp:
##########
@@ -738,6 +740,50 @@ Status
StorageEngine::submit_seg_compaction_task(BetaRowsetWriter* writer,
std::bind<void>(&StorageEngine::_handle_seg_compaction, this,
writer, segments));
}
+Status StorageEngine::process_index_change_task(const TAlterInvertedIndexReq&
request) {
+ auto tablet_id = request.tablet_id;
+ TabletSharedPtr tablet = _tablet_manager->get_tablet(tablet_id);
+ if (tablet == nullptr) {
+ LOG(WARNING) << "tablet: " << tablet_id << " not exist";
+ return Status::InternalError(
+ "tablet not exist, tablet_id={}.", tablet_id);
+ }
+ bool tablet_busy = _tablet_busy(tablet);
+ if (tablet_busy) {
+ // can not do task this time
+ LOG(WARNING) << "tablet: " << tablet_id << " can not do build inverted
index this time";
+ return Status::InternalError(
+ "tablet is busy, can not do build inverted index this time,
tablet_id={}.", tablet_id);
+ }
+
Review Comment:
We do not hold lock, so after this line, tablets may be busy immediately.
##########
be/src/olap/task/engine_index_change_task.cpp:
##########
@@ -0,0 +1,55 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "olap/task/engine_index_change_task.h"
+
+#include "runtime/memory/mem_tracker.h"
+#include "runtime/thread_context.h"
+
+namespace doris {
+
+EngineIndexChangeTask::EngineIndexChangeTask(
+ const TAlterInvertedIndexReq& alter_inverted_index_request)
+ : _alter_inverted_index_req(alter_inverted_index_request) {
+ _mem_tracker = std::make_shared<MemTrackerLimiter>(
+ MemTrackerLimiter::Type::SCHEMA_CHANGE,
+ fmt::format("EngineIndexChangeTask#tabletId={}",
+ std::to_string(_alter_inverted_index_req.tablet_id)),
+ config::memory_limitation_per_thread_for_schema_change_bytes);
+}
+
+Status EngineIndexChangeTask::execute() {
+ SCOPED_ATTACH_TASK(_mem_tracker);
+
DorisMetrics::instance()->alter_inverted_index_requests_total->increment(1);
+
+ Status res =
StorageEngine::instance()->process_index_change_task(_alter_inverted_index_req);
+
+ if (!res.ok()) {
+ LOG(WARNING) << "failed to do index change task. res=" << res
+ << " tablet_id=" << _alter_inverted_index_req.tablet_id
+ << ", schema_hash=" <<
_alter_inverted_index_req.schema_hash;
+
DorisMetrics::instance()->alter_inverted_index_requests_failed->increment(1);
+ return res;
+ }
+
+ LOG(INFO) << "success to execute index change task. res=" << res
+ << " tablet_id=" << _alter_inverted_index_req.tablet_id
+ << ", schema_hash=" << _alter_inverted_index_req.schema_hash;
Review Comment:
Time in seconds is welcome in log.
##########
be/src/olap/olap_server.cpp:
##########
@@ -738,6 +740,50 @@ Status
StorageEngine::submit_seg_compaction_task(BetaRowsetWriter* writer,
std::bind<void>(&StorageEngine::_handle_seg_compaction, this,
writer, segments));
}
+Status StorageEngine::process_index_change_task(const TAlterInvertedIndexReq&
request) {
+ auto tablet_id = request.tablet_id;
+ TabletSharedPtr tablet = _tablet_manager->get_tablet(tablet_id);
+ if (tablet == nullptr) {
+ LOG(WARNING) << "tablet: " << tablet_id << " not exist";
+ return Status::InternalError(
+ "tablet not exist, tablet_id={}.", tablet_id);
+ }
+ bool tablet_busy = _tablet_busy(tablet);
+ if (tablet_busy) {
+ // can not do task this time
+ LOG(WARNING) << "tablet: " << tablet_id << " can not do build inverted
index this time";
+ return Status::InternalError(
+ "tablet is busy, can not do build inverted index this time,
tablet_id={}.", tablet_id);
+ }
+
+ IndexBuilderSharedPtr index_builder =
+ std::make_shared<IndexBuilder>(tablet, request.columns,
request.indexes_desc,
+
request.alter_inverted_indexes, request.is_drop_op);
+ RETURN_IF_ERROR(_handle_index_change(index_builder));
+ return Status::OK();
+}
+
+Status StorageEngine::_handle_index_change(IndexBuilderSharedPtr
index_builder) {
+ RETURN_IF_ERROR(index_builder->init());
+ RETURN_IF_ERROR(index_builder->do_build_inverted_index());
+ return Status::OK();
+}
+
+bool StorageEngine::_tablet_busy(TabletSharedPtr tablet) {
+ std::shared_lock meta_rlock(tablet->get_header_lock());
+ if (SchemaChangeHandler::base_tablet_ids_in_converting(tablet->tablet_id())
+ || (tablet->tablet_state() == TABLET_NOTREADY
+ &&
SchemaChangeHandler::tablet_in_converting(tablet->tablet_id()))) {
Review Comment:
Is it enough to put base tabletid and tabletid in tablet_in_coverting?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
