You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "HHoflittlefish777 (via GitHub)" <gi...@apache.org> on 2023/04/20 12:44:14 UTC
[GitHub] [doris] HHoflittlefish777 opened a new pull request, #18874: [WIP][enhancement](storage) lazy-open necessary partitions when load
HHoflittlefish777 opened a new pull request, #18874:
URL: https://github.com/apache/doris/pull/18874
# Proposed changes
Issue Number: close #18092
## Problem summary
Describe your changes.
## Checklist(Required)
* [ ] Does it affect the original behavior
* [ ] Has unit tests been added
* [ ] Has document been added or modified
* [ ] Does it need to update dependencies
* [ ] Is this PR support rollback (If NO, please explain WHY)
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1536204790
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1537776866
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on a diff in pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1187261849
##########
be/src/util/open_partition_closure.h:
##########
@@ -0,0 +1,62 @@
+// 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.
+
+#pragma once
+
+#include <google/protobuf/stubs/common.h>
+
+#include <atomic>
+
+#include "runtime/exec_env.h"
+#include "runtime/thread_context.h"
+#include "service/brpc.h"
+#include "vec/sink/vtablet_sink.h"
+
+namespace doris {
+
+using namespace stream_load;
+
+template <typename T>
+class OpenPartitionClosure : public google::protobuf::Closure {
+public:
+ OpenPartitionClosure(VNodeChannel* vnode_channel, IndexChannel* index_channel)
+ : vnode_channel(vnode_channel), index_channel(index_channel) {}
+ ~OpenPartitionClosure() = default;
Review Comment:
warning: annotate this function with 'override' or (rarely) 'final' [modernize-use-override]
```suggestion
~OpenPartitionClosure() override = default;
```
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on a diff in pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1187486734
##########
be/src/util/open_partition_closure.h:
##########
@@ -0,0 +1,62 @@
+// 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.
+
+#pragma once
+
+#include <google/protobuf/stubs/common.h>
+
+#include <atomic>
+
+#include "runtime/exec_env.h"
+#include "runtime/thread_context.h"
+#include "service/brpc.h"
+#include "vec/sink/vtablet_sink.h"
+
+namespace doris {
+
+using namespace stream_load;
+
+template <typename T>
+class OpenPartitionClosure : public google::protobuf::Closure {
+public:
+ OpenPartitionClosure(VNodeChannel* vnode_channel, IndexChannel* index_channel)
+ : vnode_channel(vnode_channel), index_channel(index_channel) {}
+ ~OpenPartitionClosure() = default;
+
+ void Run() override {
+ SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(ExecEnv::GetInstance()->orphan_mem_tracker());
+ if (cntl.Failed()) {
+ std::stringstream ss;
+ ss << "failed to open partition, error=" << berror(this->cntl.ErrorCode())
+ << ", error_text=" << this->cntl.ErrorText();
+ LOG(WARNING) << ss.str() << " " << vnode_channel->channel_info();
+ vnode_channel->cancel("Open partition error");
+ index_channel->mark_as_failed(vnode_channel->node_id(), vnode_channel->host(),
Review Comment:
@dataroaring
1. first question,if timeout，the load task will be cancel and data will be rollback.
2. second,if a open partition is not returned and adding block goes to destination,I don't think this will have a significant impact.
- RPC is asynchronous, and if it executes too long, it won't block.
- At the same time, the RPC is in a opening state after sending , it won't cause unnecessary RPC.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1543293870
clang-tidy review says "All clean, LGTM! :+1:"
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on a diff in pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1188372124
##########
be/src/vec/sink/vtablet_sink.cpp:
##########
@@ -1062,6 +1102,22 @@ Status VOlapTableSink::open(RuntimeState* state) {
return Status::OK();
}
+void VOlapTableSink::_open_partition(const VOlapTablePartition* partition) {
+ const auto& id = partition->id;
+ auto it = _opened_partitions.find(id);
+ if (it == _opened_partitions.end()) {
+ _opened_partitions.insert(id);
+ for (int j = 0; j < partition->indexes.size(); ++j) {
Review Comment:
we need index(1,2,3) not only index.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1536956974
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1545811283
run p0
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1545090361
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1545138667
clang-tidy review says "All clean, LGTM! :+1:"
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] platoneko commented on a diff in pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "platoneko (via GitHub)" <gi...@apache.org>.
platoneko commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1188303741
##########
be/src/runtime/tablets_channel.cpp:
##########
@@ -281,9 +292,7 @@ Status TabletsChannel::_open_all_writers(const PTabletWriterOpenRequest& request
}
}
if (index_slots == nullptr) {
- std::stringstream ss;
- ss << "unknown index id, key=" << _key;
- return Status::InternalError(ss.str());
+ return Status::InternalError("unknown index id, key=" + _key.to_string());
Review Comment:
Recommend to use `Status::InternalError("unknown index id, key={}", _key.to_string());`
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1538171812
run arm
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] kaijchen commented on a diff in pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1286556381
##########
be/src/runtime/tablets_channel.cpp:
##########
@@ -388,23 +508,36 @@ Status TabletsChannel::add_batch(const PTabletWriterAddBlockRequest& request,
std::function<Status(DeltaWriter * writer)> write_func) {
google::protobuf::RepeatedPtrField<PTabletError>* tablet_errors =
response->mutable_tablet_errors();
- auto tablet_writer_it = _tablet_writers.find(tablet_id);
- if (tablet_writer_it == _tablet_writers.end()) {
- return Status::InternalError("unknown tablet to append data, tablet={}", tablet_id);
- }
- Status st = write_func(tablet_writer_it->second);
- if (!st.ok()) {
- auto err_msg =
- fmt::format("tablet writer write failed, tablet_id={}, txn_id={}, err={}",
- tablet_id, _txn_id, st.to_string());
- LOG(WARNING) << err_msg;
- PTabletError* error = tablet_errors->Add();
- error->set_tablet_id(tablet_id);
- error->set_msg(err_msg);
- tablet_writer_it->second->cancel_with_status(st);
- _add_broken_tablet(tablet_id);
- // continue write to other tablet.
- // the error will return back to sender.
+ {
+ std::lock_guard<SpinLock> l(_tablet_writers_lock);
Review Comment:
The scope of this lock may be too large. Changing it in #22703.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on a diff in pull request #18874: [enhancement](storage) lazy-open necessary partitions when load
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1174739826
##########
be/src/util/partition_open_closure.h:
##########
@@ -0,0 +1,63 @@
+// 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.
+
+#pragma once
+
+#include <google/protobuf/stubs/common.h>
+
+#include <atomic>
+
+#include "runtime/exec_env.h"
+#include "runtime/thread_context.h"
+#include "service/brpc.h"
+#include "vec/sink/vtablet_sink.h"
+
+namespace doris {
+
+using namespace stream_load;
+
+template <typename T>
+class PartitionOpenClosure : public google::protobuf::Closure {
+public:
+ PartitionOpenClosure(VNodeChannel* channel) : _channel(channel), _refs(0) {}
+ ~PartitionOpenClosure() = default;
Review Comment:
warning: annotate this function with 'override' or (rarely) 'final' [modernize-use-override]
```suggestion
~PartitionOpenClosure() override = default;
```
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](storage) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1532395999
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] freemandealer commented on a diff in pull request #18874: [enhancement](storage) lazy-open necessary partitions when load
Posted by "freemandealer (via GitHub)" <gi...@apache.org>.
freemandealer commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1177821712
##########
be/src/vec/sink/vtablet_sink.cpp:
##########
@@ -1014,6 +1071,35 @@
return Status::OK();
}
+void VOlapTableSink::_open_partition(const VOlapTablePartition* partition, uint32_t tablet_index) {
+ SCOPED_CONSUME_MEM_TRACKER(_node_channel_tracker.get());
Review Comment:
delete this
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1543930219
run compile
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1545134912
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1545485735
run p0
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] dataroaring merged pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "dataroaring (via GitHub)" <gi...@apache.org>.
dataroaring merged PR #18874:
URL: https://github.com/apache/doris/pull/18874
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] platoneko commented on a diff in pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "platoneko (via GitHub)" <gi...@apache.org>.
platoneko commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1191362520
##########
be/src/util/open_partition_closure.h:
##########
Review Comment:
Strongly recommend to move these code in vtablet_sink.cpp
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1544046688
clang-tidy review says "All clean, LGTM! :+1:"
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](storage) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1525702712
run build all
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](storage) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1526840053
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1536974654
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] freemandealer commented on a diff in pull request #18874: [WIP][enhancement](storage) lazy-open necessary partitions when load
Posted by "freemandealer (via GitHub)" <gi...@apache.org>.
freemandealer commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1173251722
##########
gensrc/proto/internal_service.proto:
##########
@@ -89,6 +89,14 @@ message PTabletWriterOpenResult {
required PStatus status = 1;
};
+message DeltaWriterOpenRequest{
Review Comment:
better rename it to "PartitionOpenRequest" since we do not want to expose DeltaWriter details to upstream BE
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] freemandealer commented on a diff in pull request #18874: [enhancement](storage) lazy-open necessary partitions when load
Posted by "freemandealer (via GitHub)" <gi...@apache.org>.
freemandealer commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1174514457
##########
be/src/common/config.h:
##########
@@ -414,8 +414,12 @@ CONF_mInt32(streaming_load_rpc_max_alive_time_sec, "1200");
// the timeout of a rpc to open the tablet writer in remote BE.
// short operation time, can set a short timeout
CONF_Int32(tablet_writer_open_rpc_timeout_sec, "60");
+// the timeout of a rpc to open the partition in remote BE.
+// short operation time, can set a short timeout
+CONF_Int32(partition_open_rpc_timeout_sec, "60");
Review Comment:
please add a flag in config.h to enable/disable this feature for compatibility reasons when the user is doing upgrade.
##########
be/src/vec/sink/vtablet_sink.cpp:
##########
@@ -1014,6 +1043,31 @@ Status VOlapTableSink::open(RuntimeState* state) {
return Status::OK();
}
+void VOlapTableSink::_open_partition(const VOlapTablePartition* partition, uint32_t tablet_index) {
Review Comment:
we don't need `tablet_index` passed to this function.
Tablets shoud be deduced using `partition` only.
##########
be/src/vec/sink/vtablet_sink.cpp:
##########
@@ -424,6 +425,34 @@ Status VNodeChannel::open_wait() {
return status;
}
+void VNodeChannel::open_partition(int64_t partition_id) {
+ SCOPED_CONSUME_MEM_TRACKER(_node_channel_tracker.get());
+ PartitionOpenRequest request;
+ request.set_allocated_id(&_parent->_load_id);
+ request.set_index_id(_index_channel->_index_id);
+ for (auto& tablet : _all_tablets) {
+ if (partition_id == tablet.partition_id) {
+ auto ptablet = request.add_tablets();
+ ptablet->set_partition_id(tablet.partition_id);
+ ptablet->set_tablet_id(tablet.tablet_id);
+ }
+ }
+
+ RefCountClosure<PartitionOpenResult>* partition_open_closure =
+ new RefCountClosure<PartitionOpenResult>();
+ partition_open_closure->ref();
+
+ // This ref is for RPC's reference
+ partition_open_closure->ref();
+ partition_open_closure->cntl.set_timeout_ms(config::partition_open_rpc_timeout_sec * 1000);
+ if (config::partition_open_ignore_eovercrowded) {
+ partition_open_closure->cntl.ignore_eovercrowded();
+ }
+ // Lazy open delter writer
+ _stub->partition_open(&partition_open_closure->cntl, &request, &partition_open_closure->result,
+ partition_open_closure);
Review Comment:
we should check RPC status and retry when failure. If still failed after retry 10 times, cancel the loading job.
##########
be/src/vec/sink/vtablet_sink.cpp:
##########
@@ -424,6 +425,34 @@ Status VNodeChannel::open_wait() {
return status;
}
+void VNodeChannel::open_partition(int64_t partition_id) {
+ SCOPED_CONSUME_MEM_TRACKER(_node_channel_tracker.get());
+ PartitionOpenRequest request;
+ request.set_allocated_id(&_parent->_load_id);
+ request.set_index_id(_index_channel->_index_id);
+ for (auto& tablet : _all_tablets) {
+ if (partition_id == tablet.partition_id) {
+ auto ptablet = request.add_tablets();
+ ptablet->set_partition_id(tablet.partition_id);
+ ptablet->set_tablet_id(tablet.tablet_id);
+ }
+ }
+
+ RefCountClosure<PartitionOpenResult>* partition_open_closure =
+ new RefCountClosure<PartitionOpenResult>();
+ partition_open_closure->ref();
+
+ // This ref is for RPC's reference
+ partition_open_closure->ref();
+ partition_open_closure->cntl.set_timeout_ms(config::partition_open_rpc_timeout_sec * 1000);
+ if (config::partition_open_ignore_eovercrowded) {
+ partition_open_closure->cntl.ignore_eovercrowded();
+ }
+ // Lazy open delter writer
Review Comment:
remove this comment because we don't want to expose Delta Writer details to Sink
##########
be/src/runtime/load_channel.cpp:
##########
@@ -70,6 +70,32 @@ Status LoadChannel::open(const PTabletWriterOpenRequest& params) {
return Status::OK();
}
+Status LoadChannel::open_partition(const PartitionOpenRequest& params) {
+ int64_t index_id = params.index_id();
+ std::shared_ptr<TabletsChannel> channel;
+ {
+ std::lock_guard<std::mutex> l(_lock);
+ auto it = _tablets_channels.find(index_id);
+ if (it != _tablets_channels.end()) {
+ channel = it->second;
+ } else {
+ // create a new tablets channel
+ TabletsChannelKey key(params.id(), index_id);
+ channel.reset(new TabletsChannel(key, _load_id, _is_high_priority));
+ {
+ std::lock_guard<SpinLock> l(_tablets_channels_lock);
+ _tablets_channels.insert({index_id, channel});
+ }
+ }
+ }
+
+ RETURN_IF_ERROR(channel->open_all_writers(params));
+
+ _opened = true;
+ _last_updated_time.store(time(nullptr));
Review Comment:
I don't think it necessary to update _last_updated_time here
##########
be/src/vec/sink/vtablet_sink.cpp:
##########
@@ -1183,6 +1237,8 @@ Status VOlapTableSink::send(RuntimeState* state, vectorized::Block* input_block,
}
// each row
_generate_row_distribution_payload(channel_to_payload, partition, tablet_index, i, 1);
+ // open partition
+ _open_partition(partition, tablet_index);
Review Comment:
why we need teblet_index to open partion?
##########
be/src/runtime/tablets_channel.h:
##########
@@ -65,6 +65,9 @@ class TabletsChannel {
Status open(const PTabletWriterOpenRequest& request);
+ // open all writer
+ Status open_all_writers(const PartitionOpenRequest& request);
Review Comment:
misleading naming, maybe rename to `open_all_writers_for_partition`?
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] freemandealer commented on a diff in pull request #18874: [enhancement](storage) lazy-open necessary partitions when load
Posted by "freemandealer (via GitHub)" <gi...@apache.org>.
freemandealer commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1177768923
##########
be/src/common/config.h:
##########
@@ -414,8 +414,14 @@ CONF_mInt32(streaming_load_rpc_max_alive_time_sec, "1200");
// the timeout of a rpc to open the tablet writer in remote BE.
// short operation time, can set a short timeout
CONF_Int32(tablet_writer_open_rpc_timeout_sec, "60");
+// the timeout of a rpc to open the partition in remote BE.
Review Comment:
please add description for these flags in `docs/zh-CN/docs/admin-manual/config/be-config.md` and `docs/en/docs/admin-manual/config/be-config.md`
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on a diff in pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1187489885
##########
be/src/runtime/tablets_channel.cpp:
##########
@@ -107,12 +107,22 @@ Status TabletsChannel::open(const PTabletWriterOpenRequest& request) {
_next_seqs.resize(_num_remaining_senders, 0);
_closed_senders.Reset(_num_remaining_senders);
- RETURN_IF_ERROR(_open_all_writers(request));
-
+ if (!config::enable_lazy_open_partition) {
+ RETURN_IF_ERROR(_open_all_writers(request));
Review Comment:
So in the config. h file **enable_ lazy_ open_ Partition** configuration is used to control exceptions that may occur during upgrades, but the default value I set is true. During the upgrade process, users need to set it to false to avoid any issues.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] platoneko commented on a diff in pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "platoneko (via GitHub)" <gi...@apache.org>.
platoneko commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1185882286
##########
be/src/runtime/tablets_channel.cpp:
##########
@@ -107,12 +107,26 @@ Status TabletsChannel::open(const PTabletWriterOpenRequest& request) {
_next_seqs.resize(_num_remaining_senders, 0);
_closed_senders.Reset(_num_remaining_senders);
- RETURN_IF_ERROR(_open_all_writers(request));
-
+ if (!config::enable_lazy_open_partition) {
+ RETURN_IF_ERROR(_open_all_writers(request));
+ } else {
+ _build_partition_tablets_relation(request);
+ }
_state = kOpened;
return Status::OK();
}
+void TabletsChannel::_build_partition_tablets_relation(const PTabletWriterOpenRequest& request) {
+ for (auto& tablet : request.tablets()) {
+ if (_partition_tablets_map.find(tablet.partition_id()) == _partition_tablets_map.end()) {
Review Comment:
Can directly use `_partition_tablets_map[partition_id]` here, since `std::map::operator[]` inserts value_type(key, T()) if the key does not exist, according to https://en.cppreference.com/w/cpp/container/map/operator_at
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] dataroaring commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "dataroaring (via GitHub)" <gi...@apache.org>.
dataroaring commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1543201686
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1543904191
run compile
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1545089950
run builadll
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1545102584
run p0
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] platoneko commented on a diff in pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "platoneko (via GitHub)" <gi...@apache.org>.
platoneko commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1191360762
##########
be/src/util/open_partition_closure.h:
##########
@@ -0,0 +1,48 @@
+// 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.
+
+#pragma once
+
+#include <google/protobuf/stubs/common.h>
+
+#include "service/brpc.h"
+#include "vec/sink/vtablet_sink.h"
+
+namespace doris {
+
+using namespace stream_load;
+
+class OpenPartitionClosure : public google::protobuf::Closure {
+public:
+ OpenPartitionClosure(VNodeChannel* vnode_channel, IndexChannel* index_channel,
+ int64_t partition_id, int64_t retry_count) {}
+ ~OpenPartitionClosure() = default;
+
+ void Run() override {}
Review Comment:
The function declaration is not written like this.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on a diff in pull request #18874: [WIP][enhancement](storage) lazy-open necessary partitions when load
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1174493284
##########
be/src/vec/sink/vtablet_sink.cpp:
##########
@@ -1014,6 +1071,35 @@
return Status::OK();
}
+void VOlapTableSink::_open_partition(const VOlapTablePartition* partition, uint32_t tablet_index) {
+ SCOPED_CONSUME_MEM_TRACKER(_node_channel_tracker.get());
Review Comment:
warning: use of undeclared identifier '_node_channel_tracker' [clang-diagnostic-error]
```cpp
SCOPED_CONSUME_MEM_TRACKER(_node_channel_tracker.get());
^
```
##########
be/src/vec/sink/vtablet_sink.cpp:
##########
@@ -1014,6 +1071,35 @@
return Status::OK();
}
+void VOlapTableSink::_open_partition(const VOlapTablePartition* partition, uint32_t tablet_index) {
+ SCOPED_CONSUME_MEM_TRACKER(_node_channel_tracker.get());
+
+ for (int j = 0; j < partition->indexes.size(); ++j) {
+ auto tid = partition->indexes[j].tablets[tablet_index];
+ auto it = _channels[j]->_channels_by_tablet.find(tid);
+ DCHECK(it != _channels[j]->_channels_by_tablet.end())
+ << "unknown tablet, tablet_id=" << tablet_index;
+ for (const auto& channel : it->second) {
+ channel->open_partition(partition->id);
+ }
+ }
+}
+
+Status VOlapTableSink::_open_partition_wait(const VOlapTablePartition* partition,
+ uint32_t tablet_index) {
+ SCOPED_CONSUME_MEM_TRACKER(_node_channel_tracker.get());
Review Comment:
warning: use of undeclared identifier '_node_channel_tracker' [clang-diagnostic-error]
```cpp
SCOPED_CONSUME_MEM_TRACKER(_node_channel_tracker.get());
^
```
##########
be/src/vec/sink/vtablet_sink.cpp:
##########
@@ -424,6 +425,62 @@ Status VNodeChannel::open_wait() {
return status;
}
+void VNodeChannel::open_partition(int64_t partition_id) {
+ SCOPED_CONSUME_MEM_TRACKER(_node_channel_tracker.get());
+ PartitionOpenRequest request;
+
+ RefCountClosure<PartitionOpenResult>* partition_open_closure =
+ new RefCountClosure<PartitionOpenResult>();
+ _partition_open_closures.insert(std::pair(partition_id, partition_open_closure));
+ partition_open_closure->ref();
+
+ // This ref is for RPC's reference
+ partition_open_closure->ref();
+ partition_open_closure->cntl.set_timeout_ms(config::partition_open_rpc_timeout_sec * 1000);
+ if (config::partition_open_ignore_eovercrowded) {
+ partition_open_closure->cntl.ignore_eovercrowded();
+ }
+ // Lazy open delter writer
+ _stub->partition_open(&partition_open_closure->cntl, &request, &partition_open_closure->result,
+ partition_open_closure);
+}
+
+Status VNodeChannel::open_partition_wait(int64_t partition_id) {
+ auto partition_it = _partition_open_closures.find(partition_id);
+ RefCountClosure<PartitionOpenResult>* partition_open_closure = nullptr;
+ if (partition_it != _partition_open_closures.end()) {
+ RefCountClosure<PartitionOpenResult>* partition_open_closure = partition_it->second;
Review Comment:
warning: unused variable 'partition_open_closure' [clang-diagnostic-unused-variable]
```cpp
RefCountClosure<PartitionOpenResult>* partition_open_closure = partition_it->second;
^
```
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on a diff in pull request #18874: [WIP][enhancement](storage) lazy-open necessary partitions when load
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1172539768
##########
be/src/service/internal_service.h:
##########
@@ -91,6 +91,11 @@ class PInternalServiceImpl : public PBackendService {
PTabletWriterOpenResult* response,
google::protobuf::Closure* done) override;
+ void delta_writer_open(google::protobuf::RpcController* controller,
+ const DeltaWriterOpenRequest* request,
+ DeltaWriterOpenResult* response,
+ google::protobuf::Closure* done) override;
Review Comment:
warning: only virtual member functions can be marked 'override' [clang-diagnostic-error]
```suggestion
google::protobuf::Closure* done) ;
```
##########
be/src/vec/sink/vtablet_sink.cpp:
##########
@@ -1183,6 +1184,22 @@ Status VOlapTableSink::send(RuntimeState* state, vectorized::Block* input_block,
}
// each row
_generate_row_distribution_payload(channel_to_payload, partition, tablet_index, i, 1);
+ // Lazy open partition
+ const std::vector<OlapTableIndexTablets>& indexs = partition->indexes;
+ std::set<int64_t> index_channels;
+ for (const auto& index : indexs){
+ const auto index_id = index.index_id;
+ index_channels.insert(index_id);
+ }
+ const auto& id = partition->id;
Review Comment:
warning: unused variable 'id' [clang-diagnostic-unused-variable]
```cpp
const auto& id = partition->id;
^
```
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1545022467
run p1
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1544040889
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] platoneko commented on a diff in pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "platoneko (via GitHub)" <gi...@apache.org>.
platoneko commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1185883787
##########
be/src/runtime/tablets_channel.cpp:
##########
@@ -270,6 +284,66 @@ int64_t TabletsChannel::mem_consumption() {
return write_mem_usage + flush_mem_usage;
}
+template <typename TabletWriterAddRequest>
+Status TabletsChannel::_open_all_writers_for_partition(const int64_t& tablet_id,
+ const TabletWriterAddRequest& request) {
+ std::vector<SlotDescriptor*>* index_slots = nullptr;
+ int32_t schema_hash = 0;
+ for (auto& index : _schema->indexes()) {
+ if (index->index_id == _index_id) {
+ index_slots = &index->slots;
+ schema_hash = index->schema_hash;
+ break;
+ }
+ }
+ if (index_slots == nullptr) {
+ std::stringstream ss;
+ ss << "unknown index id, key=" << _key << " tablet_id=" << tablet_id;
+ return Status::InternalError(ss.str());
+ }
+ int64_t partition_id = _tablet_partition_map[tablet_id];
+ DCHECK(partition_id != 0);
+ auto tablets = _partition_tablets_map[partition_id];
Review Comment:
Consider `auto&`
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1537074292
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1537037689
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on a diff in pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1187489885
##########
be/src/runtime/tablets_channel.cpp:
##########
@@ -107,12 +107,22 @@ Status TabletsChannel::open(const PTabletWriterOpenRequest& request) {
_next_seqs.resize(_num_remaining_senders, 0);
_closed_senders.Reset(_num_remaining_senders);
- RETURN_IF_ERROR(_open_all_writers(request));
-
+ if (!config::enable_lazy_open_partition) {
+ RETURN_IF_ERROR(_open_all_writers(request));
Review Comment:
So I set enable in the config. h file '_ lazy_ open_ Partition 'configuration is used to control exceptions that may occur during upgrades, but the default value I set is true. During the upgrade process, users need to set it to false to avoid any issues.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1539655780
clang-tidy review says "All clean, LGTM! :+1:"
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1539352465
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1539403014
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1539363072
run builall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] dataroaring commented on a diff in pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "dataroaring (via GitHub)" <gi...@apache.org>.
dataroaring commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1187427747
##########
be/src/util/open_partition_closure.h:
##########
@@ -0,0 +1,62 @@
+// 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.
+
+#pragma once
+
+#include <google/protobuf/stubs/common.h>
+
+#include <atomic>
+
+#include "runtime/exec_env.h"
+#include "runtime/thread_context.h"
+#include "service/brpc.h"
+#include "vec/sink/vtablet_sink.h"
+
+namespace doris {
+
+using namespace stream_load;
+
+template <typename T>
+class OpenPartitionClosure : public google::protobuf::Closure {
+public:
+ OpenPartitionClosure(VNodeChannel* vnode_channel, IndexChannel* index_channel)
+ : vnode_channel(vnode_channel), index_channel(index_channel) {}
+ ~OpenPartitionClosure() = default;
+
+ void Run() override {
+ SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(ExecEnv::GetInstance()->orphan_mem_tracker());
+ if (cntl.Failed()) {
+ std::stringstream ss;
+ ss << "failed to open partition, error=" << berror(this->cntl.ErrorCode())
+ << ", error_text=" << this->cntl.ErrorText();
+ LOG(WARNING) << ss.str() << " " << vnode_channel->channel_info();
+ vnode_channel->cancel("Open partition error");
+ index_channel->mark_as_failed(vnode_channel->node_id(), vnode_channel->host(),
Review Comment:
what if a open partition is not returned and adding block goes to destination.
##########
be/src/util/open_partition_closure.h:
##########
@@ -0,0 +1,62 @@
+// 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.
+
+#pragma once
+
+#include <google/protobuf/stubs/common.h>
+
+#include <atomic>
+
+#include "runtime/exec_env.h"
+#include "runtime/thread_context.h"
+#include "service/brpc.h"
+#include "vec/sink/vtablet_sink.h"
+
+namespace doris {
+
+using namespace stream_load;
+
+template <typename T>
+class OpenPartitionClosure : public google::protobuf::Closure {
+public:
+ OpenPartitionClosure(VNodeChannel* vnode_channel, IndexChannel* index_channel)
+ : vnode_channel(vnode_channel), index_channel(index_channel) {}
+ ~OpenPartitionClosure() = default;
+
+ void Run() override {
+ SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(ExecEnv::GetInstance()->orphan_mem_tracker());
+ if (cntl.Failed()) {
+ std::stringstream ss;
+ ss << "failed to open partition, error=" << berror(this->cntl.ErrorCode())
+ << ", error_text=" << this->cntl.ErrorText();
+ LOG(WARNING) << ss.str() << " " << vnode_channel->channel_info();
+ vnode_channel->cancel("Open partition error");
+ index_channel->mark_as_failed(vnode_channel->node_id(), vnode_channel->host(),
Review Comment:
what if load has been canceled due to timeout.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1545513261
run p0
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1540038819
PR approved by at least one committer and no changes requested.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1545487014
run p0
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1543250908
run p0
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1539399861
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1543477675
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] freemandealer commented on pull request #18874: [enhancement](storage) lazy-open necessary partitions when load
Posted by "freemandealer (via GitHub)" <gi...@apache.org>.
freemandealer commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1523274462
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] dataroaring commented on a diff in pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "dataroaring (via GitHub)" <gi...@apache.org>.
dataroaring commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1184751344
##########
be/src/vec/sink/vtablet_sink.cpp:
##########
@@ -1062,6 +1135,41 @@ Status VOlapTableSink::open(RuntimeState* state) {
return Status::OK();
}
+void VOlapTableSink::_open_partition(const VOlapTablePartition* partition) {
+ const auto& id = partition->id;
+ auto it = _partition_opened.find(id);
+ if (it == _partition_opened.end()) {
+ {
+ std::unique_lock<std::mutex> l(_partition_opened_mutex);
+ auto it = _partition_opened.find(id);
+ if (it != _partition_opened.end()) {
+ return;
+ }
+ _partition_opened.insert(std::pair(id, false));
+ }
+ for (int j = 0; j < partition->indexes.size(); ++j) {
+ for (const auto& tid : partition->indexes[j].tablets) {
+ auto it = _channels[j]->_channels_by_tablet.find(tid);
+ for (const auto& channel : it->second) {
+ auto open_partition_closure = channel->open_partition(partition->id);
+ auto st = channel->open_partition_wait(open_partition_closure);
+ if (!st.ok()) {
+ _channels[j]->mark_as_failed(
+ channel->node_id(), channel->host(),
+ fmt::format("{}, open failed, err: {}", channel->channel_info(),
+ st.to_string()),
+ -1);
+ }
+ }
+ }
+ }
+ {
+ std::unique_lock<std::mutex> l(_partition_opened_mutex);
Review Comment:
sink is called in only one thread.
##########
be/src/vec/sink/vtablet_sink.cpp:
##########
@@ -1062,6 +1135,41 @@ Status VOlapTableSink::open(RuntimeState* state) {
return Status::OK();
}
+void VOlapTableSink::_open_partition(const VOlapTablePartition* partition) {
+ const auto& id = partition->id;
+ auto it = _partition_opened.find(id);
+ if (it == _partition_opened.end()) {
+ {
+ std::unique_lock<std::mutex> l(_partition_opened_mutex);
+ auto it = _partition_opened.find(id);
+ if (it != _partition_opened.end()) {
+ return;
+ }
+ _partition_opened.insert(std::pair(id, false));
+ }
+ for (int j = 0; j < partition->indexes.size(); ++j) {
+ for (const auto& tid : partition->indexes[j].tablets) {
+ auto it = _channels[j]->_channels_by_tablet.find(tid);
+ for (const auto& channel : it->second) {
+ auto open_partition_closure = channel->open_partition(partition->id);
+ auto st = channel->open_partition_wait(open_partition_closure);
+ if (!st.ok()) {
+ _channels[j]->mark_as_failed(
+ channel->node_id(), channel->host(),
+ fmt::format("{}, open failed, err: {}", channel->channel_info(),
+ st.to_string()),
+ -1);
Review Comment:
We should wait all together to reduce time.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](storage) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1525667066
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1538112325
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1539652428
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1537074112
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1539362319
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] platoneko commented on a diff in pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "platoneko (via GitHub)" <gi...@apache.org>.
platoneko commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1188288103
##########
be/src/runtime/load_channel.cpp:
##########
@@ -84,6 +84,28 @@ Status LoadChannel::open(const PTabletWriterOpenRequest& params) {
return Status::OK();
}
+Status LoadChannel::open_partition(const PartitionOpenRequest& params) {
+ int64_t index_id = params.index_id();
+ std::shared_ptr<TabletsChannel> channel;
+ {
+ std::lock_guard<std::mutex> l(_lock);
+ auto it = _tablets_channels.find(index_id);
+ if (it != _tablets_channels.end()) {
+ channel = it->second;
+ } else {
+ // create a new tablets channel
+ TabletsChannelKey key(params.id(), index_id);
+ channel.reset(new TabletsChannel(key, _load_id, _is_high_priority, _self_profile));
Review Comment:
`std::make_shared<TabletsChannel>` is better
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1539670767
clang-tidy review says "All clean, LGTM! :+1:"
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1539786141
clang-tidy review says "All clean, LGTM! :+1:"
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](storage) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1532394456
run build all
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1537123137
run p0
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1537907705
run p0
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] dataroaring commented on a diff in pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "dataroaring (via GitHub)" <gi...@apache.org>.
dataroaring commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1187419385
##########
be/src/util/open_partition_closure.h:
##########
@@ -0,0 +1,62 @@
+// 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.
+
+#pragma once
+
+#include <google/protobuf/stubs/common.h>
+
+#include <atomic>
+
+#include "runtime/exec_env.h"
+#include "runtime/thread_context.h"
+#include "service/brpc.h"
+#include "vec/sink/vtablet_sink.h"
+
+namespace doris {
+
+using namespace stream_load;
+
+template <typename T>
+class OpenPartitionClosure : public google::protobuf::Closure {
+public:
+ OpenPartitionClosure(VNodeChannel* vnode_channel, IndexChannel* index_channel)
+ : vnode_channel(vnode_channel), index_channel(index_channel) {}
+ ~OpenPartitionClosure() = default;
+
+ void Run() override {
+ SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(ExecEnv::GetInstance()->orphan_mem_tracker());
+ if (cntl.Failed()) {
+ std::stringstream ss;
+ ss << "failed to open partition, error=" << berror(this->cntl.ErrorCode())
+ << ", error_text=" << this->cntl.ErrorText();
+ LOG(WARNING) << ss.str() << " " << vnode_channel->channel_info();
+ vnode_channel->cancel("Open partition error");
+ index_channel->mark_as_failed(vnode_channel->node_id(), vnode_channel->host(),
Review Comment:
we should retry until load is timeout.
##########
docs/en/docs/admin-manual/config/be-config.md:
##########
@@ -1227,12 +1227,30 @@ Metrics: {"filtered_rows":0,"input_row_num":3346807,"input_rowsets_count":42,"in
- The RPC timeout for sending a Batch (1024 lines) during import. The default is 60 seconds. Since this RPC may involve writing multiple batches of memory, the RPC timeout may be caused by writing batches, so this timeout can be adjusted to reduce timeout errors (such as send batch fail errors). Also, if you increase the write_buffer_size configuration, you need to increase this parameter as well.
* Default value: 60
+#### `open_partition_rpc_timeout_sec`
+
+* Description: Update interval of partition state cache
+ - The RPC timeout for sending a Batch (1024 lines) during import. The default is 60 seconds. Since this RPC may involve writing multiple batches of memory, the RPC timeout may be caused by writing batches, so this timeout can be adjusted to reduce timeout errors (such as send batch fail errors). Also, if you increase the write_buffer_size configuration, you need to increase this parameter as well.
+* Default value: 60
+
#### `tablet_writer_ignore_eovercrowded`
* Type: bool
* Description: Used to ignore brpc error '[E1011]The server is overcrowded' when writing data.
* Default value: false
+#### `enable_lazy_open_partition`
+
+* Type: bool
+* Description: When importing, most partitions may not need to be written, and lazy opening can be used to only open the partitions that need to be written.When there is mixed deployment in the upgraded version, it needs to be set to false.
+* Default value: true
+
+#### `open_partition_ignore_eovercrowded`
+
+* Type: bool
+* Description: Used to ignore brpc error '[E1011]The server is overcrowded' when writing data.
+* Default value: false
Review Comment:
set ignore by default and remove the config.
##########
be/src/runtime/tablets_channel.cpp:
##########
@@ -107,12 +107,22 @@ Status TabletsChannel::open(const PTabletWriterOpenRequest& request) {
_next_seqs.resize(_num_remaining_senders, 0);
_closed_senders.Reset(_num_remaining_senders);
- RETURN_IF_ERROR(_open_all_writers(request));
-
+ if (!config::enable_lazy_open_partition) {
+ RETURN_IF_ERROR(_open_all_writers(request));
Review Comment:
If we are rolling upgrade doris, then may be sink's works as old version and tablets_channel works as latest version, then load breaks.
##########
be/src/vec/sink/vtablet_sink.h:
##########
@@ -599,6 +607,9 @@ class VOlapTableSink final : public DataSink {
std::vector<vectorized::VExprContext*> _output_vexpr_ctxs;
RuntimeState* _state = nullptr;
+
+ std::unordered_map<int64_t,bool> opened_partitions;
Review Comment:
_opened_partitions
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1537845415
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1538117810
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1545452401
run p0
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1545089395
clang-tidy review says "All clean, LGTM! :+1:"
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] platoneko commented on a diff in pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "platoneko (via GitHub)" <gi...@apache.org>.
platoneko commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1185878722
##########
be/src/runtime/load_channel_mgr.cpp:
##########
@@ -134,6 +134,21 @@ Status LoadChannelMgr::open(const PTabletWriterOpenRequest& params) {
return Status::OK();
}
+Status LoadChannelMgr::open_partition(const PartitionOpenRequest& params) {
+ UniqueId load_id(params.id());
+ std::shared_ptr<LoadChannel> channel;
+ auto it = _load_channels.find(load_id);
+ if (it != _load_channels.end()) {
+ channel = it->second;
+ } else {
+ std::stringstream ss;
+ ss << "unknown load id, load id=" << load_id;
Review Comment:
No need to use stringstream here, can directly format in `Status::InternalError`.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1537064572
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1539667580
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] platoneko commented on a diff in pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "platoneko (via GitHub)" <gi...@apache.org>.
platoneko commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1188338754
##########
be/src/vec/sink/vtablet_sink.cpp:
##########
@@ -1062,6 +1102,22 @@ Status VOlapTableSink::open(RuntimeState* state) {
return Status::OK();
}
+void VOlapTableSink::_open_partition(const VOlapTablePartition* partition) {
+ const auto& id = partition->id;
+ auto it = _opened_partitions.find(id);
+ if (it == _opened_partitions.end()) {
+ _opened_partitions.insert(id);
+ for (int j = 0; j < partition->indexes.size(); ++j) {
Review Comment:
`for (auto& index : partition->indexes)` is better
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1543808681
clang-tidy review says "All clean, LGTM! :+1:"
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1545068557
clang-tidy review says "All clean, LGTM! :+1:"
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1545306314
run p0
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1545812685
run p0
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1546489469
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1546659103
run p0
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1545348509
run p0
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on a diff in pull request #18874: [enhancement](storage) lazy-open necessary partitions when load
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1174563917
##########
be/src/util/partition_open_closure.h:
##########
@@ -0,0 +1,63 @@
+// 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.
+
+#pragma once
+
+#include <google/protobuf/stubs/common.h>
+
+#include <atomic>
+
+#include "runtime/exec_env.h"
+#include "runtime/thread_context.h"
+#include "service/brpc.h"
+#include "vec/sink/vtablet_sink.h"
+
+namespace doris {
+
+using namespace stream_load;
+
+template <typename T>
+class PartitionOpenClosure : public google::protobuf::Closure {
+public:
+ PartitionOpenClosure(VNodeChannel* channel) : _channel(channel), _refs(0) {}
+ ~PartitionOpenClosure() {}
Review Comment:
warning: use '= default' to define a trivial destructor [modernize-use-equals-default]
```suggestion
~PartitionOpenClosure() = default;
```
##########
be/src/util/partition_open_closure.h:
##########
@@ -0,0 +1,63 @@
+// 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.
+
+#pragma once
+
+#include <google/protobuf/stubs/common.h>
+
+#include <atomic>
+
+#include "runtime/exec_env.h"
+#include "runtime/thread_context.h"
+#include "service/brpc.h"
+#include "vec/sink/vtablet_sink.h"
+
+namespace doris {
+
+using namespace stream_load;
+
+template <typename T>
+class PartitionOpenClosure : public google::protobuf::Closure {
+public:
+ PartitionOpenClosure(VNodeChannel* channel) : _channel(channel), _refs(0) {}
+ ~PartitionOpenClosure() {}
Review Comment:
warning: annotate this function with 'override' or (rarely) 'final' [modernize-use-override]
```suggestion
~PartitionOpenClosure() override {}
```
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](storage) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1530785350
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1534352058
PR approved by at least one committer and no changes requested.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] platoneko commented on a diff in pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "platoneko (via GitHub)" <gi...@apache.org>.
platoneko commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1185882286
##########
be/src/runtime/tablets_channel.cpp:
##########
@@ -107,12 +107,26 @@ Status TabletsChannel::open(const PTabletWriterOpenRequest& request) {
_next_seqs.resize(_num_remaining_senders, 0);
_closed_senders.Reset(_num_remaining_senders);
- RETURN_IF_ERROR(_open_all_writers(request));
-
+ if (!config::enable_lazy_open_partition) {
+ RETURN_IF_ERROR(_open_all_writers(request));
+ } else {
+ _build_partition_tablets_relation(request);
+ }
_state = kOpened;
return Status::OK();
}
+void TabletsChannel::_build_partition_tablets_relation(const PTabletWriterOpenRequest& request) {
+ for (auto& tablet : request.tablets()) {
+ if (_partition_tablets_map.find(tablet.partition_id()) == _partition_tablets_map.end()) {
Review Comment:
Can directly use `_partition_tablets_map[partition_id]` here, since
`std::map::operator[]` inserts value_type(key, T()) if the key does not exist, according to https://en.cppreference.com/w/cpp/container/map/operator_at
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1537058435
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1545067897
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1542261140
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] freemandealer commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "freemandealer (via GitHub)" <gi...@apache.org>.
freemandealer commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1537068905
run beut
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1537845805
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1539665547
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on a diff in pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1187489885
##########
be/src/runtime/tablets_channel.cpp:
##########
@@ -107,12 +107,22 @@ Status TabletsChannel::open(const PTabletWriterOpenRequest& request) {
_next_seqs.resize(_num_remaining_senders, 0);
_closed_senders.Reset(_num_remaining_senders);
- RETURN_IF_ERROR(_open_all_writers(request));
-
+ if (!config::enable_lazy_open_partition) {
+ RETURN_IF_ERROR(_open_all_writers(request));
Review Comment:
So in the config. h file **enable_ lazy_ open_ Partition** configuration is used to control exceptions that may occur during upgrades, but the default value I set is **true**. During the upgrade process, users need to set it to **false** to avoid any issues.At first, I set it to false by default, but I was worried that the added features after closing would conflict with this
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1546489930
clang-tidy review says "All clean, LGTM! :+1:"
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1546782568
run p1
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](storage) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1525500945
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](storage) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1525701674
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on a diff in pull request #18874: [enhancement](storage) lazy-open necessary partitions when load
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1174740626
##########
be/src/runtime/load_channel.cpp:
##########
@@ -84,6 +84,28 @@ Status LoadChannel::open(const PTabletWriterOpenRequest& params) {
return Status::OK();
}
+Status LoadChannel::open_partition(const PartitionOpenRequest& params) {
Review Comment:
warning: out-of-line definition of 'open_partition' does not match any declaration in 'doris::LoadChannel' [clang-diagnostic-error]
```cpp
Status LoadChannel::open_partition(const PartitionOpenRequest& params) {
^
```
##########
be/src/runtime/load_channel.cpp:
##########
@@ -84,6 +84,28 @@
return Status::OK();
}
+Status LoadChannel::open_partition(const PartitionOpenRequest& params) {
+ int64_t index_id = params.index_id();
+ std::shared_ptr<TabletsChannel> channel;
+ {
+ std::lock_guard<std::mutex> l(_lock);
+ auto it = _tablets_channels.find(index_id);
+ if (it != _tablets_channels.end()) {
+ channel = it->second;
+ } else {
+ // create a new tablets channel
+ TabletsChannelKey key(params.id(), index_id);
+ channel.reset(new TabletsChannel(key, _load_id, _is_high_priority));
Review Comment:
warning: no matching constructor for initialization of 'doris::TabletsChannel' [clang-diagnostic-error]
```cpp
channel.reset(new TabletsChannel(key, _load_id, _is_high_priority));
^
```
**be/src/runtime/tablets_channel.h:83:** candidate constructor not viable: requires 4 arguments, but 3 were provided
```cpp
TabletsChannel(const TabletsChannelKey& key, const UniqueId& load_id, bool is_high_priority,
^
```
**be/src/runtime/tablets_channel.h:81:** candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 3 were provided
```cpp
class TabletsChannel {
^
```
##########
be/src/runtime/load_channel.h:
##########
@@ -56,6 +56,8 @@ class LoadChannel {
// open a new load channel if not exist
Status open(const PTabletWriterOpenRequest& request);
+ Status open_partition(const PartitionOpenRequest& params);
Review Comment:
warning: unknown type name 'PartitionOpenRequest' [clang-diagnostic-error]
```cpp
Status open_partition(const PartitionOpenRequest& params);
^
```
##########
be/src/runtime/tablets_channel.h:
##########
@@ -87,6 +87,9 @@ class TabletsChannel {
~TabletsChannel();
Status open(const PTabletWriterOpenRequest& request);
+
+ // Open specific partition all writers
+ Status open_all_writers_for_partition(const PartitionOpenRequest& request);
Review Comment:
warning: unknown type name 'PartitionOpenRequest' [clang-diagnostic-error]
```cpp
Status open_all_writers_for_partition(const PartitionOpenRequest& request);
^
```
##########
be/src/runtime/load_channel_mgr.h:
##########
@@ -54,6 +54,8 @@ class LoadChannelMgr {
// open a new load channel if not exist
Status open(const PTabletWriterOpenRequest& request);
+ Status open_partition(const PartitionOpenRequest& params);
Review Comment:
warning: unknown type name 'PartitionOpenRequest' [clang-diagnostic-error]
```cpp
Status open_partition(const PartitionOpenRequest& params);
^
```
##########
be/src/runtime/load_channel_mgr.cpp:
##########
@@ -134,6 +134,21 @@ Status LoadChannelMgr::open(const PTabletWriterOpenRequest& params) {
return Status::OK();
}
+Status LoadChannelMgr::open_partition(const PartitionOpenRequest& params) {
Review Comment:
warning: out-of-line definition of 'open_partition' does not match any declaration in 'doris::LoadChannelMgr' [clang-diagnostic-error]
```cpp
Status LoadChannelMgr::open_partition(const PartitionOpenRequest& params) {
^
```
##########
be/src/runtime/tablets_channel.cpp:
##########
@@ -319,6 +392,57 @@ Status TabletsChannel::_open_all_writers(const PTabletWriterOpenRequest& request
return Status::OK();
}
+Status TabletsChannel::open_all_writers_for_partition(const PartitionOpenRequest& request) {
Review Comment:
warning: out-of-line definition of 'open_all_writers_for_partition' does not match any declaration in 'doris::TabletsChannel' [clang-diagnostic-error]
```cpp
Status TabletsChannel::open_all_writers_for_partition(const PartitionOpenRequest& request) {
^
```
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] freemandealer commented on a diff in pull request #18874: [WIP][enhancement](storage) lazy-open necessary partitions when load
Posted by "freemandealer (via GitHub)" <gi...@apache.org>.
freemandealer commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1173255009
##########
gensrc/proto/internal_service.proto:
##########
@@ -89,6 +89,14 @@ message PTabletWriterOpenResult {
required PStatus status = 1;
};
+message DeltaWriterOpenRequest{
+ repeated int64 index_ids = 1;
+}
+
+message DeltaWriterOpenResult {
+ required PStatus status = 1;
+}
+
Review Comment:
Also add a rpc claim in PBackendService at the bottom of the file
##########
be/src/service/internal_service.h:
##########
@@ -91,6 +91,11 @@ class PInternalServiceImpl : public PBackendService {
PTabletWriterOpenResult* response,
google::protobuf::Closure* done) override;
+ void delta_writer_open(google::protobuf::RpcController* controller,
Review Comment:
rename it to 'partition_open'
##########
be/src/runtime/load_channel_mgr.h:
##########
@@ -48,6 +48,8 @@ class LoadChannelMgr {
// open a new load channel if not exist
Status open(const PTabletWriterOpenRequest& request);
+ Status open_delta_writer(const DeltaWriterOpenRequest& params);
Review Comment:
rename
##########
be/src/runtime/load_channel_mgr.cpp:
##########
@@ -120,6 +120,10 @@ Status LoadChannelMgr::open(const PTabletWriterOpenRequest& params) {
return Status::OK();
}
+Status LoadChannelMgr::open_delta_writer(const DeltaWriterOpenRequest& params) {
Review Comment:
rename
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1534352193
PR approved by anyone and no changes requested.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] dataroaring commented on a diff in pull request #18874: [enhancement](storage) lazy-open necessary partitions when load
Posted by "dataroaring (via GitHub)" <gi...@apache.org>.
dataroaring commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1174491574
##########
be/src/service/internal_service.cpp:
##########
@@ -244,6 +244,28 @@ void PInternalServiceImpl::tablet_writer_open(google::protobuf::RpcController* c
}
}
+void PInternalServiceImpl::delta_writer_open(google::protobuf::RpcController* controller,
+ const DeltaWriterOpenRequest* request,
+ DeltaWriterOpenResult* response,
+ google::protobuf::Closure* done) {
Review Comment:
open_delta_writer is a better name.
##########
be/src/vec/sink/vtablet_sink.cpp:
##########
@@ -472,6 +473,35 @@ Status VNodeChannel::open_wait() {
return status;
}
+void VNodeChannel::open_partition(int64_t partition_id) {
+ SCOPED_CONSUME_MEM_TRACKER(_node_channel_tracker.get());
+ PartitionOpenRequest request;
+ auto load_id = std::make_shared<PUniqueId>(_parent->_load_id);
+ request.set_allocated_id(load_id.get());
+ request.set_index_id(_index_channel->_index_id);
+ for (auto& tablet : _all_tablets) {
+ if (partition_id == tablet.partition_id) {
+ auto ptablet = request.add_tablets();
+ ptablet->set_partition_id(tablet.partition_id);
+ ptablet->set_tablet_id(tablet.tablet_id);
+ }
+ }
+
+ PartitionOpenClosure<PartitionOpenResult>* partition_open_closure =
+ new PartitionOpenClosure<PartitionOpenResult>(this);
+ partition_open_closure->ref();
+
+ // This ref is for RPC's reference
+ partition_open_closure->ref();
+ partition_open_closure->cntl.set_timeout_ms(config::partition_open_rpc_timeout_sec * 1000);
Review Comment:
We should use remaining of the task timeout to avoid a long running task failed due to a rpc error.
##########
be/src/service/internal_service.cpp:
##########
@@ -244,6 +244,29 @@ void PInternalServiceImpl::tablet_writer_open(google::protobuf::RpcController* c
}
}
+void PInternalServiceImpl::partition_open(google::protobuf::RpcController* controller,
Review Comment:
open_partition is a better name.
##########
be/src/vec/sink/vtablet_sink.cpp:
##########
@@ -1183,6 +1184,22 @@ Status VOlapTableSink::send(RuntimeState* state, vectorized::Block* input_block,
}
// each row
_generate_row_distribution_payload(channel_to_payload, partition, tablet_index, i, 1);
+ // Lazy open partition
+ const std::vector<OlapTableIndexTablets>& indexs = partition->indexes;
+ std::set<int64_t> index_channels;
+ for (const auto& index : indexs){
Review Comment:
a space is expected before {.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1541585850
clang-tidy review says "All clean, LGTM! :+1:"
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] dataroaring commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "dataroaring (via GitHub)" <gi...@apache.org>.
dataroaring commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1542165500
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1543804208
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1545453652
run p0
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1543431861
clang-tidy review says "All clean, LGTM! :+1:"
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] platoneko commented on a diff in pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "platoneko (via GitHub)" <gi...@apache.org>.
platoneko commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1191359445
##########
be/src/util/open_partition_closure.h:
##########
@@ -0,0 +1,48 @@
+// 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.
+
+#pragma once
+
+#include <google/protobuf/stubs/common.h>
+
+#include "service/brpc.h"
+#include "vec/sink/vtablet_sink.h"
Review Comment:
please use forward declaration
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] platoneko commented on a diff in pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "platoneko (via GitHub)" <gi...@apache.org>.
platoneko commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1191362520
##########
be/src/util/open_partition_closure.h:
##########
Review Comment:
Strongly recommend to move these code to vtablet_sink.cpp
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] freemandealer commented on pull request #18874: [WIP][enhancement](storage) lazy-open necessary partitions when load
Posted by "freemandealer (via GitHub)" <gi...@apache.org>.
freemandealer commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1518915963
@HHoflittlefish777 After all is finished, please add a flag in config.h to enable/disable this feature for compatibility reasons when the user is doing upgrade.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] freemandealer commented on a diff in pull request #18874: [enhancement](storage) lazy-open necessary partitions when load
Posted by "freemandealer (via GitHub)" <gi...@apache.org>.
freemandealer commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1174874905
##########
gensrc/proto/internal_service.proto:
##########
@@ -90,6 +90,16 @@ message PTabletWriterOpenResult {
required PStatus status = 1;
};
+message PartitionOpenRequest{
Review Comment:
rename it to PPartitionOpenRequest/Result as P-prefix will indicate it is a protobuf class
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on a diff in pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1187489885
##########
be/src/runtime/tablets_channel.cpp:
##########
@@ -107,12 +107,22 @@ Status TabletsChannel::open(const PTabletWriterOpenRequest& request) {
_next_seqs.resize(_num_remaining_senders, 0);
_closed_senders.Reset(_num_remaining_senders);
- RETURN_IF_ERROR(_open_all_writers(request));
-
+ if (!config::enable_lazy_open_partition) {
+ RETURN_IF_ERROR(_open_all_writers(request));
Review Comment:
So in the config. h file **enable_ lazy_ open_ Partition** configuration is used to control exceptions that may occur during upgrades, but the default value I set is **true**. During the upgrade process, users need to set it to **false** to avoid any issues.At first, I set it to false by default, but I was worried that the added features after it would conflict with this
##########
be/src/runtime/tablets_channel.cpp:
##########
@@ -107,12 +107,22 @@ Status TabletsChannel::open(const PTabletWriterOpenRequest& request) {
_next_seqs.resize(_num_remaining_senders, 0);
_closed_senders.Reset(_num_remaining_senders);
- RETURN_IF_ERROR(_open_all_writers(request));
-
+ if (!config::enable_lazy_open_partition) {
+ RETURN_IF_ERROR(_open_all_writers(request));
Review Comment:
So in the config. h file **enable_ lazy_ open_ Partition** configuration is used to control exceptions that may occur during upgrades, but the default value I set is **true**. During the upgrade process, users need to set it to **false** to avoid any issues.At first, I set it to false by default, but I was worried that the added features after it would conflict with this.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on a diff in pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on code in PR #18874:
URL: https://github.com/apache/doris/pull/18874#discussion_r1187489885
##########
be/src/runtime/tablets_channel.cpp:
##########
@@ -107,12 +107,22 @@ Status TabletsChannel::open(const PTabletWriterOpenRequest& request) {
_next_seqs.resize(_num_remaining_senders, 0);
_closed_senders.Reset(_num_remaining_senders);
- RETURN_IF_ERROR(_open_all_writers(request));
-
+ if (!config::enable_lazy_open_partition) {
+ RETURN_IF_ERROR(_open_all_writers(request));
Review Comment:
So in the config. h file **enable_ lazy_ open_ Partition** configuration is used to control exceptions that may occur during upgrades, but the default value I set is **true**. During the upgrade process, users need to set it to **false** to avoid any issues.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] HHoflittlefish777 commented on pull request #18874: [enhancement](load) lazy-open necessary partitions when load
Posted by "HHoflittlefish777 (via GitHub)" <gi...@apache.org>.
HHoflittlefish777 commented on PR #18874:
URL: https://github.com/apache/doris/pull/18874#issuecomment-1539399224
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org