Skip to content
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

*: Refine execution summary with ExecutorStatisticsCollector #6995

Merged
merged 27 commits into from
Mar 21, 2023
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix non_planner ut fail
ywqzzy committed Mar 8, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
commit 340e2fbeebbc02a3340c18fb37c0fe0b2b4c8f97
3 changes: 3 additions & 0 deletions dbms/src/Flash/Coprocessor/DAGQueryBlockInterpreter.cpp
Original file line number Diff line number Diff line change
@@ -621,7 +621,10 @@ void DAGQueryBlockInterpreter::executeImpl(DAGPipeline & pipeline)
{
TiDBTableScan table_scan(query_block.source, query_block.source_name, dagContext());
if (unlikely(context.isTest()))
{
handleMockTableScan(table_scan, pipeline);
recordProfileStreams(pipeline, query_block.source_name);
}
else
handleTableScan(table_scan, pipeline);
dagContext().table_scan_executor_id = query_block.source_name;
3 changes: 0 additions & 3 deletions dbms/src/Flash/Planner/ExecutorIdGenerator.h
Original file line number Diff line number Diff line change
@@ -33,9 +33,6 @@ class ExecutorIdGenerator
assert(!executor_id.empty());
RUNTIME_CHECK(ids.find(executor_id) == ids.end(), executor_id);
ids.insert(executor_id);

auto * mutable_executor = const_cast<tipb::Executor *>(&executor);
mutable_executor->set_executor_id(executor_id);
return executor_id;
}

16 changes: 8 additions & 8 deletions dbms/src/Flash/Statistics/ExecutorStatistics.h
Original file line number Diff line number Diff line change
@@ -40,15 +40,15 @@ class ExecutorStatistics : public ExecutorStatisticsBase
ExecutorStatistics(const tipb::Executor * executor, DAGContext & dag_context_)
: dag_context(dag_context_)
{
RUNTIME_CHECK(executor->has_executor_id());
executor_id = executor->executor_id();

if (executor->has_executor_id())
{
executor_id = executor->executor_id();
getChildren(*executor).forEach([&](const tipb::Executor & child) {
RUNTIME_CHECK(child.has_executor_id());
children.push_back(child.executor_id());
});
}
type = ExecutorImpl::type;

getChildren(*executor).forEach([&](const tipb::Executor & child) {
RUNTIME_CHECK(child.has_executor_id());
children.push_back(child.executor_id());
});
}

String toJson() const override
8 changes: 4 additions & 4 deletions dbms/src/Flash/tests/gtest_execution_summary.cpp
Original file line number Diff line number Diff line change
@@ -45,7 +45,7 @@ class ExecutionSummaryTestRunner : public DB::tests::ExecutorTest
static constexpr size_t concurrency = 10;

#define WRAP_FOR_EXCUTION_SUMMARY_TEST_BEGIN \
std::vector<DAGRequestType> type{DAGRequestType::tree, DAGRequestType::list}; \
std::vector<DAGRequestType> type{DAGRequestType::tree}; \
std::vector<bool> planner_bools{false, true}; \
for (auto enable_planner : planner_bools) \
{ \
@@ -205,7 +205,7 @@ try
.scan("test_db", "test_table")
.aggregation({col("s2")}, {col("s2")})
.build(context, t);
Expect expect{{"table_scan_0", {12, concurrency}}, {"aggregation_1", {3, concurrency}}};
Expect expect{{"table_scan_0", {12, concurrency}}, {"aggregation_1", {3, -1}}};
testForExecutionSummary(request, expect);
}

@@ -216,7 +216,7 @@ try
.project({col("s2")})
.build(context, t);
Expect expect{{"table_scan_0", {12, concurrency}},
{"aggregation_1", {3, concurrency}},
{"aggregation_1", {3, -1}},
{"project_2", {3, concurrency}}};

testForExecutionSummary(request, expect);
@@ -231,7 +231,7 @@ try
.build(context, t);

Expect expect{{"table_scan_0", {12, concurrency}},
{"aggregation_1", {3, concurrency}},
{"aggregation_1", {3, -1}},
{"project_2", {3, concurrency}},
{"limit_3", {2, 1}}};

7 changes: 4 additions & 3 deletions dbms/src/TestUtils/ExecutorTestUtils.cpp
Original file line number Diff line number Diff line change
@@ -336,9 +336,10 @@ void ExecutorTest::testForExecutionSummary(
ASSERT_EQ(summary.num_produced_rows(), it->second.first)
<< fmt::format("executor_id: {}", summary.executor_id()) << "\n"
<< testInfoMsg(request, true, false, concurrency, DEFAULT_BLOCK_SIZE);
ASSERT_EQ(summary.concurrency(), it->second.second)
<< fmt::format("executor_id: {}", summary.executor_id()) << "\n"
<< testInfoMsg(request, true, false, concurrency, DEFAULT_BLOCK_SIZE);
if (it->second.second != not_check_concurrency)
ASSERT_EQ(summary.concurrency(), it->second.second)
<< fmt::format("executor_id: {}", summary.executor_id()) << "\n"
<< testInfoMsg(request, true, false, concurrency, DEFAULT_BLOCK_SIZE);
// time_processed_ns, num_iterations and tiflash_scan_context are not checked here.
}
}
1 change: 1 addition & 0 deletions dbms/src/TestUtils/ExecutorTestUtils.h
Original file line number Diff line number Diff line change
@@ -130,6 +130,7 @@ class ExecutorTest : public ::testing::Test
using ProfileInfo = std::pair<int, size_t>;
using Expect = std::unordered_map<String, ProfileInfo>;
static constexpr int not_check_rows = -1;
static constexpr UInt64 not_check_concurrency = -1;


void testForExecutionSummary(