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 secur...@mariadb.org _______________________________________________ developers mailing list -- developers@lists.mariadb.org To unsubscribe send an email to developers-le...@lists.mariadb.org
