Hi, Yuchen,
On Sep 01, Yuchen Pei wrote:
> revision-id: 04b99a30a3f (mariadb-11.0.1-103-g04b99a30a3f)
> parent(s): 4f39ae75606
> author: Yuchen Pei
> committer: Yuchen Pei
> timestamp: 2023-08-16 11:31:16 +1000
> message:
>
> MDEV-15935 Adding global/session var redirect_url
>
> Adding a global/session var `redirect_url' of string type. The initial
> value is empty. Can be supplied in mysqld with --redirect-url. A valid
> redirect_url should be of the format
>
> {mysql,mariadb}://host[:port]
>
> where <host> is an arbitrary string not containing colons, and <port>
> is a number between 0 and 65535 inclusive.
>
> diff --git a/mysql-test/main/redirect.opt b/mysql-test/main/redirect.opt
move this to the sys_vars suite, please
> --- /dev/null
> +++ b/mysql-test/main/redirect.opt
> @@ -0,0 +1 @@
> +--redirect_url=mysql://foobar
ok, as long as this isn't a complete feature, but just one piece of it.
but when client support will be implemented (CONC-599) and the server
will actually send this to the connecting clients (MDEV-31609),
this test will need to be fixed, as foobar is not something a client can
be redirected to.
Also, it'd be more interesting to try something like
--init-connect='set @@redirect_url="mysql://foobar"'
instead of the simple --redirect_url=mysql://foobar
because init-connect is much more flexible, it can generate the url on
the fly, it can send different users to different locations. So it'd
make sense to test init-connect approach.
> diff --git a/mysql-test/main/redirect.test b/mysql-test/main/redirect.test
> new file mode 100644
> --- /dev/null
> +++ b/mysql-test/main/redirect.test
> @@ -0,0 +1,56 @@
> +--echo #
> +--echo # MDEV-15935 Connection Redirection Mechanism in MariaDB
> Client/Server Protocol
> +--echo #
> +
> +set @old_redirect_url=@@redirect_url;
> +select @@global.redirect_url;
> +set global redirect_url=default;
> +select @@global.redirect_url;
> +--error ER_WRONG_VALUE_FOR_VAR
> +set global redirect_url="mariadb.org";
> +--error ER_WRONG_VALUE_FOR_VAR
> +set global redirect_url="https://mariadb.org";;
before doing all this fun, please remove redirect_url from the
session_track_system_variables, or just set it to the empty string.
> diff --git a/sql/sql_class.h b/sql/sql_class.h
> --- a/sql/sql_class.h
> +++ b/sql/sql_class.h
> @@ -3585,6 +3587,7 @@ class THD: public THD_count, /* this must be first */
> */
> LEX_CSTRING connection_name;
> char default_master_connection_buff[MAX_CONNECTION_NAME+1];
> + char redirect_url_buff[MAX_CONNECTION_NAME+1];
This isn't such a good idea (I'm afraid, I missed it when
default_master_connection_buff was added). A couple of years ago
Monty have spent significant efforts trying to make THD smaller as it
has grown to be too large. Now THD keeps growing again. Let's try not to
encourage it, if we can help it.
> uint8 password; /* 0, 1 or 2 */
> uint8 failed_com_change_user;
> bool slave_thread;
> diff --git a/sql/sys_vars.inl b/sql/sys_vars.inl
> --- a/sql/sys_vars.inl
> +++ b/sql/sys_vars.inl
> @@ -637,6 +637,129 @@ class Sys_var_charptr_fscs: public Sys_var_charptr
...
> +class Sys_var_redirect_url: public Sys_var_charptr_base
please, no. new classes are needed only for variables with very special,
unlike anything else, behavior.
redirect_url is just a session string variable. See the comment at the
top of sys_vars.cc
But (!) there are no other session string variables yet.
So I'd recomment to fix Sys_var_charptr to support session variables.
And then create redirect_url out of it.
Practically it might need no changes at all, if you put explicit
strmake (or strdup/free) into THD::init like you did anyway.
After that, I suspect, you might be able to remove Sys_var_charptr_base,
Sys_var_sesvartrack, and Sys_var_session_lexstring, because
Sys_var_charptr/Sys_var_lexstring will be able to serve all those use
cases.
> +{
> + static const size_t max_length = MAX_CONNECTION_NAME;
> +public:
> + Sys_var_redirect_url(const char *name_arg,
> + const char *comment,
> + CMD_LINE getopt,
> + const char *def_val, PolyLock *lock= 0) :
> + Sys_var_charptr_base(name_arg, comment,
> + SESSION_VAR(redirect_url),
> + getopt, def_val, lock,
> + VARIABLE_NOT_IN_BINLOG, 0, 0, 0)
> + {
> + global_var(LEX_CSTRING).length= strlen(def_val);
> + option.var_type|= GET_STR;
> + *const_cast<SHOW_TYPE*>(&show_val_type)= SHOW_LEX_STRING;
> + }
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
