Repository navigation
MDEV-19629 Implement MySQL 8.0 native functions: format_bytes(), format_pico_time() and ps_thread_id() - #6080
Shubhamnish1030 wants to merge 2 commits into
Conversation
…at_pico_time() and ps_thread_id()
FORMAT_PICO_TIME() and FORMAT_BYTES() are already native. This adds
the remaining MySQL 8.0.16 functions:
PS_THREAD_ID(connection_id) Performance Schema THREAD_ID of the
connection, NULL if there is none
PS_CURRENT_THREAD_ID() THREAD_ID of the current connection
Behaviour follows MySQL 8.4: BIGINT UNSIGNED, NULL for NULL, zero,
negative and non-integer arguments and for unknown connections, and
ER_WRONG_PERFSCHEMA_USAGE when the Performance Schema is disabled.
Like CONNECTION_ID(), both are allowed in VIRTUAL but not in
PERSISTENT generated columns.
The lookup goes through two new PSI thread calls, get_thread_by_id()
and get_thread_internal_id(), appended to PSI_v1 like
set_thread_peer_port(), so the result always matches
performance_schema.threads, including threads that never call
THD::set_psi().
The sys schema keeps its own sys.ps_thread_id(), where NULL means the
current connection. Calling it now emits Note 1585 on first use, the
same as sys.format_bytes() after FORMAT_BYTES() became native, so the
affected sysschema results are updated.
Signed-off-by: Shubham Chandrawanshi <179670574+Shubhamnish1030@users.noreply.github.com>
488f9be to
d7d4074
Compare
gkodinov
left a comment
There was a problem hiding this comment.
Thank you for your contribution. But I fail to see the usefulness of it, given its current implementation.
Can you please reply my questions below?
| end_metadata_wait_v1_t end_metadata_wait; | ||
|
|
||
| set_thread_peer_port_v1_t set_thread_peer_port; | ||
| /** @sa get_thread_by_id_v1_t. */ |
There was a problem hiding this comment.
You can't just add functions without changing the PSI interface version. That's not the intent. The PSI interfaces are supposed to be immutable and then when you need to change the interface you bump the version (v1, v2 etc).
But this brings me to my next question: Why do you insist on having the function call down to perfschema from the SQL layer?
That's not what Chris did in mysql/mysql-server@260e09e#diff-bbb008e05393ffaad9f35966758d0aa1988a9adff23f40489ab187860dc62e7d.
He implemented the functions in the SQL layer: there he has everything needed to do that.
And, finally, what is the point in implementing these functions to start with?
Chris implemented these to speed up the SYS schema joins. Do you need to speed up the sys schema joins? Then why is this not part of the fix, as it is with Chris's change above?
Do you need these functions for something? What is it?
There's no point in just dumping it into MariaDB because MySQL did it. IMHO at least.
If you really want to implement them regardless, why not just do them as pluggable functions? So that at least they are optional and not baked into the core server.
There was a problem hiding this comment.
Agreed. In 4b5eb03 the PSI headers are back to what main has. The functions are now function plugins inside the perfschema library and read PFS_thread directly. Details, including the sys schema / WL#13439 part: #6080 (comment)
|
It failed when compiled without perfschema, on Appveyor. Please also ensure that tests that need perfschema have --source include/have_perfschema.inc in them. To compile without perfschema yourself for testing, pass -DPLUGIN_PERFSCHEMA=NO to cmake |
…hema function plugins Review: no PSI interface change, no SQL layer change, and the server must build with -DPLUGIN_PERFSCHEMA=NO. PS_THREAD_ID(connection_id) and PS_CURRENT_THREAD_ID() are now function plugins declared by the performance schema plugin library (storage/perfschema/pfs_native_func.cc): - They read PFS_thread directly, so the PSI interface is unchanged, nothing is added to the SQL layer, and the functions do not exist in a server built without the performance schema. Each one can be switched off with --ps-thread-id=OFF / --ps-current-thread-id=OFF. - Behaviour follows MySQL 8.4: BIGINT UNSIGNED, NULL for NULL, zero, negative and non-integer arguments and for unknown connections, and ER_WRONG_PERFSCHEMA_USAGE when the performance schema is disabled. Like CONNECTION_ID(), both are allowed in VIRTUAL but not in PERSISTENT generated columns. sys.ps_thread_id() keeps its interface (NULL means the current connection, and it returns NULL when the performance schema is disabled), but it now calls the native functions instead of selecting from performance_schema.threads. That makes it about twice as fast when it is evaluated per row, and it no longer adds a statement event of its own for every call. Calling it emits Note 1585, the same as sys.format_bytes() since FORMAT_BYTES() became native, so the affected sysschema results are updated. mtr does not treat the two plugins as optional, because the sys schema uses them, the same as the InnoDB I_S plugins that sys uses. Signed-off-by: Shubham Chandrawanshi <179670574+Shubhamnish1030@users.noreply.github.com>
|
Thanks for the review, @gkodinov and @vaintroub. I reworked the PR in 4b5eb03, pushed on top of the original commit so the change is easy to review. I'll squash it into one commit whenever you prefer. PSI interface. You're right, appending to Pluggable functions. Building without perfschema (@vaintroub). With What it's for, and WL#13439. I read your comment on the Jira and went through 7e4f32bc. In MariaDB the format half of WL#13439 is already done: the sys views and procedures call the native The difference shows when the function is evaluated per row, for example when filtering
The old function also runs a Apart from sys, the reasons in the ticket still apply: scripts written for MySQL 8.0.16+ call If you'd rather drop The amd64-windows failure was |
Description
Jira: MDEV-19629
FORMAT_PICO_TIME()andFORMAT_BYTES()from this ticket are already native. This PR adds the remaining MySQL 8.0.16 functions:PS_THREAD_ID(connection_id)THREAD_IDof that connection,NULLif there is nonePS_CURRENT_THREAD_ID()THREAD_IDof the current connectionBehaviour follows MySQL 8.4, which I checked against a local 8.4.11 server before writing the code:
BIGINT UNSIGNEDNULLforNULL,0, negative and non-integer arguments and for unknown connection ids ('abc'also raises the usual 1292 truncation warning, same as MySQL)ER_WRONG_PERFSCHEMA_USAGE("The Performance Schema is not enabled.") whenperformance_schema=OFFCONNECTION_ID(), allowed inVIRTUALbut not inPERSISTENTgenerated columnsImplementation (reworked after review)
PS_THREAD_ID()andPS_CURRENT_THREAD_ID()areMariaDB_FUNCTION_PLUGINs declared by the performance schema plugin library:storage/perfschema/pfs_native_func.cc, registered inha_perfschema.cc. They readPFS_threaddirectly (PFS_thread::get_current_thread(), and a scan ofglobal_thread_containerunder the optimistic lock, the same waytable_threadsreads it).sql/. Without the performance schema (-DPLUGIN_PERFSCHEMA=NO) the functions don't exist. Each one can be switched off with--ps-thread-id=OFF/--ps-current-thread-id=OFF.safe_to_cache_query= 0andBINLOG_STMT_UNSAFE_SYSTEM_FUNCTION.sys.ps_thread_id()keeps its interface (NULL= current connection,NULLwhen performance_schema is off) but calls the native functions instead of selecting fromperformance_schema.threads(MariaDB's part of MySQL's WL#13439; theformat_bytes()/format_pico_time()part is already in sys). Calling it emits Note 1585, likesys.format_bytes(), so 4 sysschema results gain that note, andfn_ps_thread_id/fn_ps_thread_accountdisable the ps/cursor/view protocols around the call, likefn_format_bytes.mariadb-test-run.pldoes not make the two plugins optional, because sys uses them (same exception as the InnoDB I_S plugins sys uses).Per-row cost, filtering
events_statements_history_long(20,000 rows, Debug build, so only the ratios matter):WHERE THREAD_ID = ...sys.ps_thread_id(NULL), beforesys.ps_thread_id(NULL), this PRPS_CURRENT_THREAD_ID()How can this PR be tested?
New tests:
perfschema.native_func_thread_id: the plugins are listed ininformation_schema.PLUGINS, results matchperformance_schema.threadsfor every connection, a second connection before and after disconnect,NULLcases, prepared statements, result type, argument count, generated columns,sys.ps_thread_id()on top of the native functionsperfschema.native_func_thread_id_off(--loose-performance-schema=OFF): the error path, andsys.ps_thread_id()still returnsNULLBoth source
include/have_perfschema.inc.Run locally (macOS arm64, Debug build of
main@ 7b96ca4, head 4b5eb03):perfschema.native_func_thread_id,perfschema.native_func_thread_id_off: pass, also with--ps-protocol,--cursor-protocoland--view-protocol(same for the changed sysschema tests)--suite=perfschema,sysschema,plugins: 555/556 pass; the one failure,perfschema.socket_summary_by_instance_func, passes on its own (socket timing under 6-way parallel load)main.mysqld--help(the two plugin switches are listed with the other optional plugins),main.plugin,main.plugin_load,main.information_schema,main.information_schema_all_engines,main.func_misc,main.sp,main.ps: pass-DPLUGIN_PERFSCHEMA=NObuild: compiles and links; the perfschema-dependent tests skip themselvesninja abi_check: passes, PSI headers unchangedBasing the PR against the correct MariaDB version
main.Copyright
I am contributing this under the BSD-new (3-clause) license.