- developers - lists.mariadb.org

Re: [Maria-developers] [Commits] d3f10e3: MDEV-8431 Feedback plugin needs an option for http proxy.
by Sergei Golubchik 26 Sep '15

26 Sep '15

Hi, Holyfoot! On Sep 21, holyfoot(a)askmonty.org wrote: > revision-id: d3f10e30ec39bccf1172c7b2eb159f40e412bdfd (mariadb-10.0.21-14-gd3f10e3) > parent(s): abd31ca2b6cde024772fcac0f4953219d76c6344 > committer: Alexey Botchkov > timestamp: 2015-09-21 14:06:15 +0500 > message: > > MDEV-8431 Feedback plugin needs an option for http proxy. > 'feedback_proxy_server' system variable added to specify the > proxy server. Not a dynamic one. How did you test this feature? > diff --git a/plugin/feedback/feedback.cc b/plugin/feedback/feedback.cc > index f644bd5..f76c52a 100644 > --- a/plugin/feedback/feedback.cc > +++ b/plugin/feedback/feedback.cc > @@ -250,6 +251,9 @@ static int init(void *p) > > prepare_linux_info(); > > + proxy_url= proxy_server ? > + Url::create(proxy_server, strlen(proxy_server)) : 0; Some diagnostics would be nice here. At least "feedback: invalid proxy url: '%.*s'". See how ivalid feedback urls are reported. > + > url_count= 0; > if (*url) > { > @@ -347,6 +354,9 @@ static MYSQL_SYSVAR_ULONG(send_timeout, send_timeout, PLUGIN_VAR_RQCMDARG, > static MYSQL_SYSVAR_ULONG(send_retry_wait, send_retry_wait, PLUGIN_VAR_RQCMDARG, > "Wait this many seconds before retrying a failed send.", > NULL, NULL, 60, 1, 60*60*24, 1); > +static MYSQL_SYSVAR_STR(proxy_server, proxy_server, > + PLUGIN_VAR_READONLY | PLUGIN_VAR_RQCMDARG, > + "Proxy server address.", NULL, NULL,0); Mention that it must be an url. "Proxy server URL" or "Proxy server address, in the correct URI format", or something. > static struct st_mysql_sys_var* settings[] = { > MYSQL_SYSVAR(server_uid), > diff --git a/plugin/feedback/url_http.cc b/plugin/feedback/url_http.cc > index f214f7a..eade67c 100644 > --- a/plugin/feedback/url_http.cc > +++ b/plugin/feedback/url_http.cc > @@ -148,9 +148,12 @@ int Url_http::send(const char* data, size_t data_length) > my_socket fd= INVALID_SOCKET; > char buf[1024]; > uint len= 0; > + Url_http *proxy_loc= (Url_http *) proxy_url; > > addrinfo *addrs, *addr, filter= {0, AF_UNSPEC, SOCK_STREAM, 6, 0, 0, 0, 0}; > - int res= getaddrinfo(host.str, port.str, &filter, &addrs); > + int res= proxy_url ? > + getaddrinfo(proxy_loc->host.str, proxy_loc->port.str, &filter, &addrs) : > + getaddrinfo(host.str, port.str, &filter, &addrs); > First, you don't know whether proxy_url is Url_http. It might be easier to have an option --feedback-http-proxy=host:port (not the url). It also solves the potential future confusion if we'll have more than one transport. Second, I don't think proxies work this way at all, for a http proxy, you need to specify the host in th GET request, like POST http://foo.bar/bla/bla So, I think, a better implementation would be to take proxy into account not in Url_http::send(), but already in http_create(). And create Url_http objects for feedback url with proxy's host and port, and with the appropriately adjusted path. Regards, Sergei

1 0

Re: [Maria-developers] [Commits] efcbbed: MDEV-8682 - CSV engine does not properly process "", in quotes
by Sergei Golubchik 26 Sep '15

26 Sep '15

Hi, Sergey! On Sep 22, Sergey Vojtovich wrote: > revision-id: efcbbed977dfcd9ee0ed035cbb0df4404483b0c8 (mariadb-10.1.7-53-gefcbbed) > parent(s): 7cbecadf9b59d06f4d9a9e776f99418b273c2ebc > committer: Sergey Vojtovich > timestamp: 2015-09-22 16:52:30 +0400 > message: > > MDEV-8682 - CSV engine does not properly process "", in quotes > > Added per-table boolean IETF_QUOTES variable to CSV storage engine. It allows to > enable IETF-compatible parsing of embedded quote and comma characters. Disabled > by default. > > This patch is based on Percona revision: > https://github.com/percona/percona-server/commit/b32fbf02766f8db00f9ff4e6dc… > > Note that original patch adds server variable, while this patch adds per-table > variable. I agree that a per-table option is safer. Ok to push. Regards, Sergei

1 0

Re: [Maria-developers] [Commits] 5f461a9: MDEV-7640: CHANGE MASTER TO doesn't work with prepared statements
by Kristian Nielsen 21 Sep '15

21 Sep '15

nirbhay(a)mariadb.com (Nirbhay Choubey) writes: > revision-id: 5f461a94a5aa2e1a20fa559ca612d96b53201e4e > parent(s): 29ac245dd04c5416d57a90b0ab3c4d08cbc4d723 > committer: Nirbhay Choubey > branch nick: 5.5 > timestamp: 2015-09-16 18:10:06 -0400 > message: > > MDEV-7640: CHANGE MASTER TO doesn't work with prepared statements > > When CHANGE MASTER was executed as a PS, its attributes were wrongly > getting reset toward the end of PREPARE. As a result, the subsequent > executions had no effect. Fixed by making sure that the CHANGE MASTER > attributes are preserved during the lifetime of the PS. Hm, this MDEV seems now assigned to me, but I did not see any mail from you about it? Did you expect a review? But in any case, this code around prepare/execute is not something I am familiar with. I seem to recall that Sanja worked on several issues related to incorrect prepare/execute, maybe he can have some ideas or suggest a reviewer. Apparently this is something that used to work but started to break sometimes in 5.5. Do you know how the problem was introduced? - Kristian (sadder by the hour over devs not using the mailing list).

2 1

[Maria-developers] Super Read Only in MariaDB (10.1) ?
by Jean-François Gagné 19 Sep '15

19 Sep '15

Hi, a super_read_only option is making its way in MySQL 5.7: * http://www.tocker.ca/2015/07/09/mysql-5-7-8-now-featuring-super_read_only-a… * http://dev.mysql.com/doc/refman/5.7/en/server-system-variables.html#sysvar_… Are there plans to have a similar option in MariaDB (it would be great to have it in MariaDB 10.1) ? Many thanks, -- Jean-François Gagné System Engineer Booking.com BV Rembrandt Square Office Herengracht 597, 1017 CE Amsterdam

2 1

[Maria-developers] New programmer
by Acha Bill 18 Sep '15

18 Sep '15

Hello guys, I'm Acha Bill. Final year undergraduate in Computer Engineering. I would like to be an active contributor to your ongoing projects. I have good knowledge of C/C++ and MySQL. As this is my first approach to open source, any guidance will be much appreciated. Bill.

3 3

Re: [Maria-developers] Please review MDEV-8468 MDEV-8466
by Sergei Golubchik 17 Sep '15

17 Sep '15

Hi, Alexander! On Jul 21, Alexander Barkov wrote: > > I have one question about "HANDLER FOR SQLWARNING". > It does not catch messages of level "Note". > Looks like a bug. Please confirm, I'll file a report if so. Hmm, I don't know. According to the standard, SQLWARNING should match sqlstates in the category W, while SQLEXCEPTION should be for sqlstates in the category X. But we don't do that, 22007 is an exception (category X), but we treat it as a warning. So, the standard doesn't help to answer your question. We seem to use the logic where a level "error" means an SQL exception and a level "warning" means an SQL warning. Using this logic, "notes" are neither exceptions nor warnings, and neither SQLEXCEPTION nor SQLWARNING should match them. I'd say it's not a bug. The patch itself looks great, these changes in behavior are very welcomed. Just one small comment regarding THD::check_edom_and_truncation(), see below. > diff --git a/sql/sql_class.h b/sql/sql_class.h > index a8d8444..c19174b 100644 > --- a/sql/sql_class.h > +++ b/sql/sql_class.h > @@ -3270,6 +3270,53 @@ class THD :public Statement, > (!transaction.stmt.modified_non_trans_table || > (variables.sql_mode & MODE_STRICT_ALL_TABLES))); > } > + /** > + Check string-to-number conversion and produce a warning if > + - could not convert any digits (EDOM-alike error) > + - found garbage at the end of the string > + See also Field_num::check_edom_and_truncation() for a similar function. > + > + @param thd - the thread > + @param type - name of the data type (e.g. "INTEGER", "DECIMAL", "DOUBLE") > + @param edom - of a EDOM-alike error happened > + @param cs - character set of the original string > + @param str - the original string > + @param end_of_num - the position where the string-to-number routine stopped. > + @param end - the end of the string > + > + Unlike Field_num::check_edom_and_truncation(), this function does not > + distinguish between EDOM and truncation and reports the same warning for > + both cases. Perhaps we should eventually print different warnings, to make > + the explicit CAST work closer to the implicit cast in Field_xxx::store(). Why haven't you done it now? It causes too many changes in tests? > + */ > + void check_edom_and_truncation(const char *type, bool edom, > + CHARSET_INFO *cs, > + const char *str, > + const char *end_of_num, > + const char *end) Move this out of THD please. You don't need to call current_thd every time for it, but only when a warning should be issued. > + { > + DBUG_ASSERT(str <= end_of_num); > + DBUG_ASSERT(end_of_num <= end); > + if (edom || > + (end_of_num < end && !check_if_only_end_space(cs, end_of_num, end))) > + { > + /* > + We can use err.ptr() here as ErrConvString is guranteed to put an > + end \0 here. > + */ > + push_warning_printf(this, Sql_condition::WARN_LEVEL_WARN, > + ER_TRUNCATED_WRONG_VALUE, > + ER(ER_TRUNCATED_WRONG_VALUE), type, > + ErrConvString(str, end - str, cs).ptr()); > + } > + else if (end_of_num < end) > + { > + push_warning_printf(this, Sql_condition::WARN_LEVEL_NOTE, > + ER_TRUNCATED_WRONG_VALUE, > + ER(ER_TRUNCATED_WRONG_VALUE), type, > + ErrConvString(str, end - str, cs).ptr()); > + } > + } Regards, Sergei

2 1

[Maria-developers] unsubscribe
by Abdul Hakeem 16 Sep '15

16 Sep '15

1 0

Re: [Maria-developers] [Commits] a1f31cf: MDEV-6756: map a linux pid (child pid) to a connection id shown in the output of SHOW PROCESSLIST
by Sergei Golubchik 16 Sep '15

16 Sep '15

Hi, Sanja! Looks good, thanks. Ok to push! On Sep 15, sanja(a)mariadb.com wrote: > revision-id: a1f31cfa8d0f04cb166813786f7854db3dbcf715 (mariadb-10.1.6-12-ga1f31cf) > parent(s): 86a3613d4e981c341e38291c9eeec5dc9f836fae > committer: Oleksandr Byelkin > timestamp: 2015-09-15 21:50:28 +0200 > message: > > MDEV-6756: map a linux pid (child pid) to a connection id shown in the > output of SHOW PROCESSLIST > > Added tid (thread ID) for system where it is present. > > ps -eL -o tid,pid,command > > shows the thread on Linux Regards, Sergei

1 0

Re: [Maria-developers] [Commits] 8bf23ef: MDEV-8208: Sporadic SEGFAULT on startup
by Sergei Golubchik 16 Sep '15

16 Sep '15

Hi, Nirbhay! On Aug 19, Nirbhay Choubey wrote: > revision-id: 8bf23efc914f18c687067e9314cadf4f36a85eec > parent(s): 71d1f35847a575239deff856590bf6f13afd74ed > committer: Nirbhay Choubey > branch nick: 5.5-galera.b8208 > timestamp: 2015-08-19 17:17:56 -0400 > message: > > MDEV-8208: Sporadic SEGFAULT on startup A couple of questions: > diff --git a/sql/mysqld.cc b/sql/mysqld.cc > index 2008fc5..4c05999 100644 > --- a/sql/mysqld.cc > +++ b/sql/mysqld.cc > @@ -4584,6 +4579,29 @@ static int init_server_components() > } > plugins_are_initialized= TRUE; /* Don't separate from init function */ > > +#ifdef WITH_WSREP > + /* Wait for wsrep threads to get created. */ > + if (wsrep_creating_startup_threads == 1) { > + mysql_mutex_lock(&LOCK_thread_count); > + while (wsrep_running_threads < 2) > + { > + mysql_cond_wait(&COND_thread_count, &LOCK_thread_count); > + } > + > + /* Now is the time to initialize threads for queries. */ > + THD *tmp; > + I_List_iterator<THD> it(threads); > + while ((tmp= it++)) > + if (tmp->wsrep_applier == true) > + tmp->init_for_queries(); Do you really need to iterate over all THD's? There is only one applier thread, right? And it's THD is not stored anywhere, only in the global list? > + mysql_mutex_unlock(&LOCK_thread_count); > + } > +#endif > + > have_csv= plugin_status(STRING_WITH_LEN("csv"), > MYSQL_STORAGE_ENGINE_PLUGIN); > have_ndbcluster= plugin_status(STRING_WITH_LEN("ndbcluster"), > @@ -4890,12 +4902,21 @@ pthread_handler_t start_wsrep_THD(void *arg) > //thd->version= refresh_version; > thd->proc_info= 0; > thd->command= COM_SLEEP; > - thd->set_time(); > - thd->init_for_queries(); > + > + if (plugins_are_initialized) > + { > + thd->init_for_queries(); > + } What happens if plugins are not initialized yet? thd->init_for_queries() will be done in that THD loop above? In that case why do you need to do it here? Regards, Sergei

2 2

Re: [Maria-developers] [Commits] 6ed7307: MDEV-8576: Bootstrap should ignore --enforce-storage-engine option
by Sergei Golubchik 16 Sep '15

16 Sep '15

Hi, Jan! On Sep 16, Jan Lindström wrote: > revision-id: 6ed73070e120e1be5456288556019db5bd3f9f60 > parent(s): 6cc1befcf26b2dc43e5cd7ad85d29c576073b1c5 > committer: Jan Lindström > branch nick: 10.1-tests > timestamp: 2015-09-16 11:47:34 +0300 > message: > > MDEV-8576: Bootstrap should ignore --enforce-storage-engine option > > Allow enforce-storage-engine="" option and use that on mysql_install_db > when doing bootstrap. Agree. After I commented on the previous patch it occurred to me that --disable-enforce-storage-engine is a bad idea, because --enable-enforce-storage-engine makes no sense, and one normally expects --enable-xxx for every --disable-xxx. So --enforce-storage-engine="" is a better way of switching this feature off. > diff --git a/sql/mysqld.cc b/sql/mysqld.cc > index b45d4e1..ee3b870 100644 > --- a/sql/mysqld.cc > +++ b/sql/mysqld.cc > @@ -4843,7 +4843,10 @@ static void add_file_to_crash_report(char *file) > static int init_default_storage_engine_impl(const char *opt_name, > char *engine_name, plugin_ref *res) > { > - if (!engine_name) > + /* Storage engine can be NULL and enforced storage engine > + can be empty. */ > + if (!engine_name || > + (strcmp(opt_name, "enforced_storage_engine") == 0 && strlen(engine_name) == 0)) > { > *res= 0; > return 0; In fact, default_tmp_storage_engine can be NULL too. See in mysqld.cc: if (default_tmp_storage_engine && !*default_tmp_storage_engine) default_tmp_storage_engine= NULL; if (init_default_storage_engine(default_tmp_storage_engine, tmp_table_plugin)) unireg_abort(1); And it looks like it's important to have default_tmp_storage_engine=NULL, not leave it as an empty string. Because Sys_var_plugin::do_check() doesn't allow empty strings as plugins names, but translates NULL to "no plugin". So, you either need to set enforced_storage_engine=NULL if it was an empty string, or you need to fix Sys_var_plugin::do_check() to treat empty strings as "no plugin" too. Regards, Sergei

1 0