-
Notifications
You must be signed in to change notification settings - Fork 3.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[feature](move-memtable) adapt sink v2 to pipelineX #27004
Conversation
run buildall |
1 similar comment
run buildall |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
}; | ||
|
||
using RowsForTablet = std::unordered_map<int64_t, Rows>; | ||
inline constexpr char VOLAP_TABLE_SINK_V2[] = "VOlapTableSinkV2"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: do not declare C-style arrays, use std::array<> instead [modernize-avoid-c-arrays]
inline constexpr char VOLAP_TABLE_SINK_V2[] = "VOlapTableSinkV2";
^
// specific language governing permissions and limitations | ||
// under the License. | ||
|
||
#include <brpc/uri.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: 'brpc/uri.h' file not found [clang-diagnostic-error]
#include <brpc/uri.h>
^
return static_cast<VTabletWriterV2*>(writer)->on_partitions_created(result); | ||
} | ||
|
||
Status VTabletWriterV2::_incremental_open_streams( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: method '_incremental_open_streams' can be made static [readability-convert-member-functions-to-static]
Status VTabletWriterV2::_incremental_open_streams( | |
static Status VTabletWriterV2::_incremental_open_streams( |
return Status::OK(); | ||
} | ||
|
||
Status VTabletWriterV2::_open_streams(int64_t src_id) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: method '_open_streams' can be made static [readability-convert-member-functions-to-static]
be/src/vec/sink/writer/vtablet_writer_v2.h:126:
- Status _open_streams(int64_t src_id);
+ static Status _open_streams(int64_t src_id);
|
||
Status VTabletWriterV2::_open_streams_to_backend(int64_t dst_id, | ||
::doris::stream_load::LoadStreams& streams) { | ||
auto node_info = _nodes_info->find_node(dst_id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: 'auto node_info' can be declared as 'const auto *node_info' [readability-qualified-auto]
auto node_info = _nodes_info->find_node(dst_id); | |
const auto *node_info = _nodes_info->find_node(dst_id); |
return Status::OK(); | ||
} | ||
|
||
void VTabletWriterV2::_build_tablet_node_mapping() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: method '_build_tablet_node_mapping' can be made static [readability-convert-member-functions-to-static]
be/src/vec/sink/writer/vtablet_writer_v2.h:132:
- void _build_tablet_node_mapping();
+ static void _build_tablet_node_mapping();
// under the License. | ||
|
||
#pragma once | ||
#include <brpc/controller.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: 'brpc/controller.h' file not found [clang-diagnostic-error]
#include <brpc/controller.h>
^
#include <gen_cpp/types.pb.h> | ||
#include <glog/logging.h> | ||
#include <google/protobuf/stubs/callback.h> | ||
#include <stddef.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: inclusion of deprecated C++ header 'stddef.h'; consider using 'cstddef' instead [modernize-deprecated-headers]
#include <stddef.h> | |
#include <cstddef> |
#include <glog/logging.h> | ||
#include <google/protobuf/stubs/callback.h> | ||
#include <stddef.h> | ||
#include <stdint.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: inclusion of deprecated C++ header 'stdint.h'; consider using 'cstdint' instead [modernize-deprecated-headers]
#include <stdint.h> | |
#include <cstdint> |
TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
(From new machine)TeamCity pipeline, clickbench performance test result: |
TeamCity be ut coverage result: |
TeamCity be ut coverage result: |
9890da2
to
32f054b
Compare
run buildall |
1 similar comment
run buildall |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
namespace doris { | ||
|
||
namespace vectorized { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
namespace doris { | |
namespace vectorized { | |
namespace doris::vectorized { |
be/src/vec/sink/writer/vtablet_writer_v2.cpp:549:
- } // namespace vectorized
- } // namespace doris
+ } // namespace doris
c2e6db5
to
c5b855b
Compare
run buildall |
TeamCity be ut coverage result: |
run buildall |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
return std::make_shared<OlapTableSinkV2Operator>(this, _sink); | ||
} | ||
|
||
Status OlapTableSinkV2LocalState::init(RuntimeState* state, LocalSinkStateInfo& info) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: method 'init' can be made static [readability-convert-member-functions-to-static]
be/src/pipeline/exec/olap_table_sink_v2_operator.h:54:
- Status init(RuntimeState* state, LocalSinkStateInfo& info) override;
+ static Status init(RuntimeState* state, LocalSinkStateInfo& info) override;
return Status::OK(); | ||
} | ||
|
||
Status OlapTableSinkV2LocalState::close(RuntimeState* state, Status exec_status) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: method 'close' can be made static [readability-convert-member-functions-to-static]
be/src/pipeline/exec/olap_table_sink_v2_operator.h:61:
- Status close(RuntimeState* state, Status exec_status) override;
+ static Status close(RuntimeState* state, Status exec_status) override;
OlapTableSinkV2LocalState(DataSinkOperatorXBase* parent, RuntimeState* state) | ||
: Base(parent, state) {}; | ||
Status init(RuntimeState* state, LocalSinkStateInfo& info) override; | ||
Status open(RuntimeState* state) override { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: method 'open' can be made static [readability-convert-member-functions-to-static]
Status open(RuntimeState* state) override { | |
static Status open(RuntimeState* state) override { |
_group_commit(group_commit), | ||
_pool(pool) {}; | ||
|
||
Status init(const TDataSink& thrift_sink) override { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: method 'init' can be made static [readability-convert-member-functions-to-static]
Status init(const TDataSink& thrift_sink) override { | |
static Status init(const TDataSink& thrift_sink) override { |
return Status::OK(); | ||
} | ||
|
||
Status prepare(RuntimeState* state) override { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: method 'prepare' can be made static [readability-convert-member-functions-to-static]
Status prepare(RuntimeState* state) override { | |
static Status prepare(RuntimeState* state) override { |
return vectorized::VExpr::prepare(_output_vexpr_ctxs, state, _row_desc); | ||
} | ||
|
||
Status open(RuntimeState* state) override { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: method 'open' can be made static [readability-convert-member-functions-to-static]
Status open(RuntimeState* state) override { | |
static Status open(RuntimeState* state) override { |
return vectorized::VExpr::open(_output_vexpr_ctxs, state); | ||
} | ||
|
||
Status sink(RuntimeState* state, vectorized::Block* in_block, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: method 'sink' can be made static [readability-convert-member-functions-to-static]
Status sink(RuntimeState* state, vectorized::Block* in_block, | |
static Status sink(RuntimeState* state, vectorized::Block* in_block, |
(From new machine)TeamCity pipeline, clickbench performance test result: |
TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
|
run buildall |
(From new machine)TeamCity pipeline, clickbench performance test result: |
TeamCity be ut coverage result: |
run buildall |
TeamCity be ut coverage result: |
TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
c9dfc1a
to
f1aece4
Compare
run buildall |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
namespace doris { | ||
|
||
namespace vectorized { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: nested namespaces can be concatenated [modernize-concat-nested-namespaces]
namespace doris { | |
namespace vectorized { | |
namespace doris::vectorized { |
be/src/vec/sink/writer/vtablet_writer_v2.cpp:539:
- } // namespace vectorized
- } // namespace doris
+ } // namespace doris
TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
TeamCity be ut coverage result: |
This reverts commit f972e68.
f1aece4
to
140078b
Compare
run buildall |
run buildall |
TeamCity be ut coverage result: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
@@ -1005,4 +1010,23 @@ Status PipelineXFragmentContext::send_report(bool done) { | |||
std::placeholders::_2)}, | |||
shared_from_this()); | |||
} | |||
|
|||
bool PipelineXFragmentContext::_has_inverted_index_or_partial_update(TOlapTableSink sink) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warning: method '_has_inverted_index_or_partial_update' can be made static [readability-convert-member-functions-to-static]
be/src/pipeline/pipeline_x/pipeline_x_fragment_context.h:148:
- bool _has_inverted_index_or_partial_update(TOlapTableSink sink);
+ static bool _has_inverted_index_or_partial_update(TOlapTableSink sink);
TeamCity be ut coverage result: |
(From new machine)TeamCity pipeline, clickbench performance test result: |
TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
Proposed changes
Issue Number: close #xxx
Further comments
If this is a relatively large or complex change, kick off the discussion at [email protected] by explaining why you chose the solution you did and what alternatives you considered, etc...