Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Fix task_manager_test (Issue #1556) #1560

Merged
merged 5 commits into from
Apr 25, 2021
Merged

Fix task_manager_test (Issue #1556) #1560

merged 5 commits into from
Apr 25, 2021

Conversation

17zhangw
Copy link
Member

Description

util::QueryExecUtil does not synchronize access to the callback row function. Patch introduces a std::mutex to guarantee that regardless of how the DML query is executed (i.e., parallel or not), the tuple output function can only be invoked by a single thread at a time.

@17zhangw 17zhangw self-assigned this Apr 22, 2021
@17zhangw 17zhangw added the ready-for-ci Indicate that this build should be run through CI. label Apr 22, 2021
@noisepage-checks
Copy link

Minor Decrease in Performance

Be warned: this PR may have decreased the throughput of the system slightly.

tps (%change) benchmark_type wal_device details
-1.79% tpcc RAM disk
Detailsmaster tps=22530.23, commit tps=22127.78, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=RAM disk, max_connection_threads=32
1.97% tpcc None
Detailsmaster tps=29053.25, commit tps=29625.31, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=None, max_connection_threads=32
0.34% tpcc HDD
Detailsmaster tps=21306.08, commit tps=21378.73, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=HDD, max_connection_threads=32
7.72% tatp RAM disk
Detailsmaster tps=6471.86, commit tps=6971.48, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=RAM disk, max_connection_threads=32
-2.12% tatp None
Detailsmaster tps=7441.97, commit tps=7284.27, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=None, max_connection_threads=32
7.21% tatp HDD
Detailsmaster tps=6369.09, commit tps=6828.32, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=HDD, max_connection_threads=32

@codecov
Copy link

codecov bot commented Apr 22, 2021 •
edited
Loading

Codecov Report

Merging #1560 (5f24e99) into master (aa73737) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1560      +/-   ##
==========================================
+ Coverage   81.97%   81.98%   +0.01%     
==========================================
  Files         735      735              
  Lines       51600    51601       +1     
==========================================
+ Hits        42297    42307      +10     
+ Misses       9303     9294       -9
Impacted Files Coverage Δ
src/include/transaction/transaction_context.h 97.82% <ø> (-0.05%) ⬇️
src/util/query_exec_util.cpp 85.91% <100.00%> (+0.20%) ⬆️
src/execution/sema/sema_checking.cpp 71.52% <0.00%> (-0.67%) ⬇️
src/execution/vm/bytecode_generator.cpp 84.18% <0.00%> (+0.04%) ⬆️
src/execution/sema/sema_builtin.cpp 61.54% <0.00%> (+0.09%) ⬆️
src/storage/garbage_collector.cpp 94.21% <0.00%> (+0.82%) ⬆️
...xecution/compiler/operator/operator_translator.cpp 62.71% <0.00%> (+0.84%) ⬆️
src/planner/plannodes/plan_node_defs.cpp 32.97% <0.00%> (+1.06%) ⬆️
src/storage/index/hash_index.cpp 91.13% <0.00%> (+1.26%) ⬆️
src/execution/sema/sema_decl.cpp 81.13% <0.00%> (+1.88%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b63fa5a...5f24e99. Read the comment docs.

@17zhangw 17zhangw added ready-for-review This PR passes all checks and is ready to be reviewed. Mark PRs with this. and removed ready-for-ci Indicate that this build should be run through CI. labels Apr 22, 2021
@17zhangw 17zhangw requested a review from linmagit April 22, 2021 10:06
@17zhangw 17zhangw added the ready-for-ci Indicate that this build should be run through CI. label Apr 22, 2021
@noisepage-checks
Copy link

Minor Decrease in Performance

Be warned: this PR may have decreased the throughput of the system slightly.

tps (%change) benchmark_type wal_device details
-1.53% tpcc RAM disk
Detailsmaster tps=22530.23, commit tps=22184.89, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=RAM disk, max_connection_threads=32
-2.15% tpcc None
Detailsmaster tps=29053.25, commit tps=28429.06, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=None, max_connection_threads=32
1.81% tpcc HDD
Detailsmaster tps=21306.08, commit tps=21691.78, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=HDD, max_connection_threads=32
5.81% tatp RAM disk
Detailsmaster tps=6471.86, commit tps=6847.83, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=RAM disk, max_connection_threads=32
0.53% tatp None
Detailsmaster tps=7441.97, commit tps=7481.54, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=None, max_connection_threads=32
7.6% tatp HDD
Detailsmaster tps=6369.09, commit tps=6853.12, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=HDD, max_connection_threads=32

linmagit
linmagit approved these changes Apr 23, 2021
View reviewed changes
Copy link
Member

@linmagit linmagit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just a minor formatting comment.

src/util/query_exec_util.cpp Outdated Show resolved Hide resolved
@noisepage-checks
Copy link

Minor Decrease in Performance

Be warned: this PR may have decreased the throughput of the system slightly.

tps (%change) benchmark_type wal_device details
-1.55% tpcc RAM disk
Detailsmaster tps=22349.67, commit tps=22003.84, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=RAM disk, max_connection_threads=32
0.68% tpcc None
Detailsmaster tps=28713.58, commit tps=28909.8, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=None, max_connection_threads=32
-1.12% tpcc HDD
Detailsmaster tps=21905.68, commit tps=21661.03, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=HDD, max_connection_threads=32
6.4% tatp RAM disk
Detailsmaster tps=6544.27, commit tps=6963.31, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=RAM disk, max_connection_threads=32
-0.39% tatp None
Detailsmaster tps=7495.21, commit tps=7465.73, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=None, max_connection_threads=32
5.07% tatp HDD
Detailsmaster tps=6470.89, commit tps=6799.15, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=HDD, max_connection_threads=32

@noisepage-checks
Copy link

Minor Decrease in Performance

Be warned: this PR may have decreased the throughput of the system slightly.

tps (%change) benchmark_type wal_device details
-0.74% tpcc RAM disk
Detailsmaster tps=22349.67, commit tps=22184.89, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=RAM disk, max_connection_threads=32
-0.99% tpcc None
Detailsmaster tps=28713.58, commit tps=28429.06, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=None, max_connection_threads=32
-0.98% tpcc HDD
Detailsmaster tps=21905.68, commit tps=21691.78, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=HDD, max_connection_threads=32
4.64% tatp RAM disk
Detailsmaster tps=6544.27, commit tps=6847.83, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=RAM disk, max_connection_threads=32
-0.18% tatp None
Detailsmaster tps=7495.21, commit tps=7481.54, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=None, max_connection_threads=32
5.91% tatp HDD
Detailsmaster tps=6470.89, commit tps=6853.12, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=HDD, max_connection_threads=32

@lmwnshn lmwnshn added ready-to-merge This PR is ready to be merged. Mark PRs with this. and removed ready-for-review This PR passes all checks and is ready to be reviewed. Mark PRs with this. labels Apr 25, 2021
@noisepage-checks
Copy link

Performance Boost!

Nice job! This PR has increased the throughput of the system.

tps (%change) benchmark_type wal_device details
3.27% tpcc RAM disk
Detailsmaster tps=21730.95, commit tps=22441.37, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=RAM disk, max_connection_threads=32
4.07% tpcc None
Detailsmaster tps=27966.54, commit tps=29103.93, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=None, max_connection_threads=32
1.96% tpcc HDD
Detailsmaster tps=20950.37, commit tps=21362.04, query_mode=extended, benchmark_type=tpcc, scale_factor=32.0000, terminals=32, client_time=60, weights={'Payment': 43, 'Delivery': 4, 'NewOrder': 45, 'StockLevel': 4, 'OrderStatus': 4}, wal_device=HDD, max_connection_threads=32
5.89% tatp RAM disk
Detailsmaster tps=6546.54, commit tps=6931.86, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=RAM disk, max_connection_threads=32
1.99% tatp None
Detailsmaster tps=7346.73, commit tps=7493.21, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=None, max_connection_threads=32
7.15% tatp HDD
Detailsmaster tps=6300.87, commit tps=6751.31, query_mode=extended, benchmark_type=tatp, scale_factor=1.0000, terminals=16, client_time=60, weights={'GetAccessData': 35, 'UpdateLocation': 14, 'GetNewDestination': 10, 'GetSubscriberData': 35, 'DeleteCallForwarding': 2, 'InsertCallForwarding': 2, 'UpdateSubscriberData': 2}, wal_device=HDD, max_connection_threads=32

@linmagit linmagit merged commit 11a11c7 into cmu-db:master Apr 25, 2021
@lmwnshn lmwnshn mentioned this pull request Apr 26, 2021
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lmwnshn lmwnshn lmwnshn left review comments

@linmagit linmagit linmagit approved these changes

Assignees

@17zhangw 17zhangw

Labels
ready-for-ci Indicate that this build should be run through CI. ready-to-merge This PR is ready to be merged. Mark PRs with this.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@17zhangw @linmagit @lmwnshn