Hi, Yuchen, On Jun 13, Yuchen Pei wrote:
Hi Sergei,
Thanks for the comments. I have addressed them and the url of the new patch can be found in the new request for review comment in the ticket.
Thanks. I'll send a separate email with comments on the new patch. Here I'll just reply to your email.
diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 32392ab882e..caf85736770 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -365,6 +365,7 @@ bool opt_disable_networking=0, opt_skip_show_db=0; bool opt_skip_name_resolve=0; my_bool opt_character_set_client_handshake= 1; bool opt_endinfo, using_udf_functions; +bool udf_initialized= 0;
I don't think you need that. You can use exising mysqld_server_started.
That won't work, because mysqld_server_started will only be turned on very late, towards the end of mysqld_main(). If spider is initialised between udf_init() and mysqld_server_started is 1 (for example in an init-file), then the udf will fail. See https://github.com/MariaDB/server/commit/91176a7be7c where the test udf_mysql_func_early fails with "query 'SELECT SPIDER_DIRECT_SQL('select * from tbl_a', 'results', 'srv "s_2_1", database "auto_test_remote"')' failed: ER_SP_DOES_NOT_EXIST (1305): FUNCTION auto_test_local.SPIDER_DIRECT_SQL does not exist" because of this.
Now, I assume MDEV-31401, the task to include the udf insertion trick in early calls CREATE FUNCTION, will fix this issue without introduce any new global booleans because it should has access to the internal variable `initialized`, though I do not know how complex or doable this task is. So I guess there's a trade-off here between fixing the spider init bugs now and investigating MDEV-31401. Would you like me to explore MDEV-31401 first?
As you like. You can do MDEV-31401 or work around the issue with, for example BEGIN NOT ATOMIC DECLARE EXIT HANDLER FOR 1123 INSERT mysql.func (...), (...), ...; CREATE FUNCTION ...; CREATE FUNCTION ...; CREATE FUNCTION ...; CREATE FUNCTION ...; END; just don't add new spider-specific global variables to the server.
my_bool locked_in_memory; bool opt_using_transactions; bool volatile abort_loop;
2. about (ret == -1). I looked at what plugin init functions return now: 33 plugins don't have an init function 76 plugins always return 0 13 plugins return 0 or 1 3 plugins return 0 or -1 3 plugins return 0 or HA_ERR_INITIALIZATION, HA_ERR_OUT_OF_MEM and innodb inconsistently return 0, HA_ERR_xxx as above, or 1
so, I suggest to introduce, like, HA_ERR_DEPENDENCIES (or whatever) and only retry plugins that return that specific error code.
Done. The HA_ERR_ namespace is tightly packed and bounded by HA_ERR_FIRST (120) and HA_ERR_LAST (198), and I crammed in a #define HA_ERR_RETRY_INIT 129.
It's not tightly packed, HA_ERR_LAST is just a high water mark. You can add a new HA_ERR_ at the end and increment HA_ERR_LAST. But 129 works too, that's fine.
goto err; } }
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org