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
Mirror acl_users Array with Map #201
Conversation
The acl_users Prealloced_array is used widely, this commit does not replace it, rather this commit creates a table of usernames to lists of matching ACL_USER objects so that the find_mpvio_user and find_acl_user functions can return more quickly when there are many tens of thousands of users. To test the setup, a script was created which creates many users: https://github.com/ericherman/code-snips/blob/master/shell/create-many-mysql-users For this testing, 32768 user names were created, each user has six hosts. While there is a small amount of jitter, the following was typical for the "before" case: $ time { for i in {0..10000}; do \ /home/eric/builds/mysql-8.0/bin/mysql --port=13306 \ --ssl-mode=DISABLED --host=127.0.0.1 --user=user0032768 -pnone \ -e "SELECT 1;" >/dev/null 2>&1 || echo "DARN $?"; done; } real 0m40.236s user 0m25.599s sys 0m6.986s With the patch, the following was typical of the "after" case: $ time { for i in {0..10000}; do \ /home/eric/builds/mysql-8.0/bin/mysql --port=23306 \ --ssl-mode=DISABLED --host=127.0.0.1 --user=user0032768 -pnone \ -e "SELECT 1;" >/dev/null 2>&1 || echo "DARN $?"; done; } real 0m33.984s user 0m22.060s sys 0m9.205s Examining with "perf" shows us where time is spent originally: perf record -F 20000 -a \ -p $(cat /data/mysql-8.0-data/mysqld.pid) & sleep 0.3; \ /home/eric/builds/mysql-8.0/bin/mysql --user=root \ --host=127.0.0.1 --port=13306 --ssl-mode=DISABLED <<<"SELECT 1"; \ killall perf; perf report -i ./perf.data # Overhead Command Shared Object Symbol 44.63% mysqld libc-2.26.so [.] __strcmp_sse2_unaligned 3.48% mysqld mysqld [.] setup_fields 2.05% mysqld mysqld [.] vio_io_wait 1.90% mysqld [vdso] [.] 0x0000000000000c1e 1.88% mysqld mysqld [.] find_mpvio_user 1.87% mysqld libc-2.26.so [.] __tzstring 1.85% mysqld mysqld [.] alloc_and_copy_thd_dynamic_variables 1.82% mysqld mysqld [.] pfs_get_thread_statement_locker_v1 1.79% mysqld mysqld [.] dd::cache::Object_registry::Object_registry 1.79% mysqld mysqld [.] Protocol_classic::parse_packet 1.77% mysqld mysqld [.] thd_increment_bytes_received 1.77% mysqld mysqld [.] TaoCrypt::SHA256::Transform 1.72% mysqld mysqld [.] Item_int::init 1.71% mysqld mysqld [.] dispatch_command With the change we see different results: perf record -F 20000 -a \ -p $(cat /data/mysql-8.0-acl_user-data/mysqld.pid) & sleep 0.3; \ /home/eric/builds/mysql-8.0/bin/mysql --user=root \ --host=127.0.0.1 --port=23306 --ssl-mode=DISABLED <<<"SELECT 1"; \ killall perf; perf report -i ./perf.data # Overhead Command Shared Object Symbol 9.91% mysqld libc-2.26.so [.] _int_malloc 7.19% mysqld mysqld [.] thd_increment_bytes_received 7.13% mysqld mysqld [.] net_write_command 6.93% mysqld mysqld [.] get_thread_attributes 6.73% mysqld mysqld [.] Protocol_classic::net_store_data 6.68% mysqld mysqld [.] Item::charset_for_protocol 6.57% mysqld mysqld [.] pfs_unlock_rwlock_v1 6.37% mysqld mysqld [.] plugin_foreach_with_mask 6.34% mysqld mysqld [.] MYSQLparse 6.34% mysqld mysqld [.] lex_start 6.32% mysqld mysqld [.] TaoCrypt::Transform256 6.10% mysqld mysqld [.] pfs_memory_free_v1 5.58% mysqld mysqld [.] my_hash_sort_uca_900_tmpl<Mb_wc_utf8mb4, 1> 4.53% mysqld mysqld [.] THD::THD From the differences in perf output, we see that the changed code is no longer spending an over-whelming amount of its time comparing strings or in find_mpvio_user. I would like to thank Mattias Jonsson, as he was a great help in this work: in an earlier version, he figured out my bug where the map would have no-longer-valid pointers (because of the sorting of acl_users). Also, he found my NULL vs. empty-string bug. I would like to thank Gonzalo Diethelm for being willing and able to act as a sounding board during my discussions, and helping me write more idiomatic C++ code and giving me some good debugging ideas. I would like to thank Dmitry Lenev for helping me think-through the ACL_compare logic, Philippe Bruhat for patiently debugging with me, Kendrick Shaw for helping me with allocators, and Daniël van Eeden for pointing out the issue in the first place and encouraging me to hack on it.
Hi, thank you for your contribution. Please confirm this code is submitted under the terms of the OCA (Oracle's Contribution Agreement) you have previously signed by cutting and pasting the following text as a comment: |
I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it. |
Hi, thank you for your contribution. Your code has been assigned to an internal queue. Please follow |
The acl_users Prealloced_array is used widely, this commit does not
replace it, rather this commit creates a table of usernames to lists
of matching ACL_USER objects so that the find_mpvio_user and
find_acl_user functions can return more quickly when there are many
tens of thousands of users.
To test the setup, a script was created which creates many users:
https://github.com/ericherman/code-snips/blob/master/shell/create-many-mysql-users
For this testing, 32768 user names were created, each user has six
hosts.
While there is a small amount of jitter, the following was typical
for the "before" case:
$ time { for i in {0..10000}; do
/home/eric/builds/mysql-8.0/bin/mysql --port=13306
--ssl-mode=DISABLED --host=127.0.0.1 --user=user0032768 -pnone
-e "SELECT 1;" >/dev/null 2>&1 || echo "DARN $?"; done; }
real 0m40.236s
user 0m25.599s
sys 0m6.986s
With the patch, the following was typical of the "after" case:
$ time { for i in {0..10000}; do
/home/eric/builds/mysql-8.0/bin/mysql --port=23306
--ssl-mode=DISABLED --host=127.0.0.1 --user=user0032768 -pnone
-e "SELECT 1;" >/dev/null 2>&1 || echo "DARN $?"; done; }
real 0m33.984s
user 0m22.060s
sys 0m9.205s
Examining with "perf" shows us where time is spent originally:
perf record -F 20000 -a
-p $(cat /data/mysql-8.0-data/mysqld.pid) & sleep 0.3;
/home/eric/builds/mysql-8.0/bin/mysql --user=root
--host=127.0.0.1 --port=13306 --ssl-mode=DISABLED <<<"SELECT 1";
killall perf; perf report -i ./perf.data
Overhead Command Shared Object Symbol
With the change we see different results:
perf record -F 20000 -a
-p $(cat /data/mysql-8.0-acl_user-data/mysqld.pid) & sleep 0.3;
/home/eric/builds/mysql-8.0/bin/mysql --user=root
--host=127.0.0.1 --port=23306 --ssl-mode=DISABLED <<<"SELECT 1";
killall perf; perf report -i ./perf.data
Overhead Command Shared Object Symbol
From the differences in perf output, we see that the changed code is
no longer spending an over-whelming amount of its time comparing
strings or in find_mpvio_user.
I would like to thank Mattias Jonsson, as he was a great help in
this work: in an earlier version, he figured out my bug where the
map would have no-longer-valid pointers (because of the sorting of
acl_users). Also, he found my NULL vs. empty-string bug. I would
like to thank Gonzalo Diethelm for being willing and able to act as
a sounding board during my discussions, and helping me write more
idiomatic C++ code and giving me some good debugging ideas. I would
like to thank Dmitry Lenev for helping me think-through the
ACL_compare logic, Philippe Bruhat for patiently debugging with me,
Kendrick Shaw for helping me with allocators, and Daniël van Eeden
for pointing out the issue in the first place and encouraging me to
hack on it.