FB8-76: Add current master host:port to server read only error when a…

…vailable (facebook#958)

Summary:
Jira ticket: https://jira.percona.com/browse/FB8-76

Reference Patch: facebook@270854f
Reference Patch: facebook@9f00827
Reference Patch: facebook@3e50e46
Reference Patch: facebook@9f18725

When a server is in read_only, we append the current master_host and
master_port to the error message, if the server is a slave. The master_host
could be either hostname or ip address, depending on how a slave is connected
to the master.

A new generic error code `ER_OPTION_PREVENTS_STATEMENT_EXTRA_INFO` is added, so
that we can append any additional info to the fixed error message.

Proposing a way to set a custom message in the `--read-only (super)` error.
Note: the variable can only be set when server is in read-only (super) state.

p.s. I am hesitant to add a mutex for the variable, so we will get a snapshot
of the current pointer of the variable (const char*) when printing the error.

Per request, we allow setting read_only_error_msg_extra without being in
read_only state. The custom message can be set/reset at any time.

D61383 added a new error code to allow appending extra info to read_only error
message. The new error code breaks existing dependencies which expects to
receive code 1290. Reverting back to use the old error, and extending the 1290
error message instead of adding a new one. This involves updating many files
(which was the motivation I added a new one in D61383).

Ideally we should have a more elegant solution to send extra info back to
clients, which will be worked on separately. This is to fix the broken
dependencies.
Pull Request resolved: facebook#958

Test Plan:
Updated rpl.rpl_read_only. Master_port is non-deterministic in the test, so I
masked in the test result.

Will update any test that comes up in the sandcastle if needed.

Originally Reviewed By: santoshb

Reviewed By: lth

Differential Revision: D14137345

Pulled By: lth

fbshipit-source-id: 13d23a2

mysql-test/r/bug58669.result

@@ -11,6 +11,7 @@ user1@localhost
 SHOW VARIABLES LIKE "read_only%";
 Variable_name	Value
 read_only	ON
+read_only_error_msg_extra	
 read_only_slave	OFF
 INSERT INTO db1.t1 VALUES (1);
 ERROR HY000: The MySQL server is running with the --read-only option so it cannot execute this statement

mysql-test/r/mysqld--help-notwin.result

@@ -1052,6 +1052,9 @@ The following options may be given as the first argument:
  --read-only         Make all non-temporary tables read-only, with the
  exception for replication (slave) threads and users with
  the SUPER privilege
+ --read-only-error-msg-extra[=name] 
+ Set this variable to print out extra error information,
+ which will be appended to read_only error messages.
  --read-only-slave   Blocks disabling read_only if the server is a slave. This
  is helpful in asserting that read_only is never disabled
  on a slave. Slave with read_only=0 may generate new GTID
@@ -1759,6 +1762,7 @@ range-alloc-block-size 4096
 range-optimizer-max-mem-size 8388608
 read-buffer-size 131072
 read-only FALSE
+read-only-error-msg-extra 
 read-only-slave TRUE
 read-rnd-buffer-size 262144
 regexp-stack-limit 8000000

mysql-test/suite/rpl_nogtid/r/rpl_read_only.result

@@ -78,7 +78,11 @@ a
 1004
 1005
 insert into t1 values(1006);
-ERROR HY000: The MySQL server is running with the --read-only option so it cannot execute this statement
+ERROR HY000: The MySQL server is running with the --read-only option so it cannot execute this statement. Current master_host: 127.0.0.1, master_port: MASTER_PORT
+set global read_only_error_msg_extra = "This is a custom message";
+insert into t1 values(1006);
+ERROR HY000: The MySQL server is running with the --read-only option so it cannot execute this statement. Current master_host: 127.0.0.1, master_port: MASTER_PORT. This is a custom message
+set global read_only_error_msg_extra = default;
 drop user test;
 drop table t1;
 include/sync_slave_sql_with_master.inc

mysql-test/suite/rpl_nogtid/t/rpl_read_only.test

@@ -13,6 +13,7 @@ create user test;
 
 connect (master2,127.0.0.1,test,,test,$MASTER_MYPORT,);
 connect (slave2,127.0.0.1,test,,test,$SLAVE_MYPORT,);
+connect (slave2_root,127.0.0.1,root,,test,$SLAVE_MYPORT,);
 
 connection master1;
 
@@ -94,8 +95,20 @@ select * from t1;
 
 # Non root user can not write on the slave
 connection slave2;
+--replace_result $MASTER_MYPORT MASTER_PORT
 --error ER_OPTION_PREVENTS_STATEMENT
 insert into t1 values(1006);
+--replace_result $MASTER_MYPORT MASTER_PORT
+# set extra info for the error message
+connection slave2_root;
+set global read_only_error_msg_extra = "This is a custom message";
+connection slave2;
+--replace_result $MASTER_MYPORT MASTER_PORT
+--error ER_OPTION_PREVENTS_STATEMENT
+insert into t1 values(1006);
+--replace_result $MASTER_MYPORT MASTER_PORT
+connection slave2_root;
+set global read_only_error_msg_extra = default;
 
 ## Cleanup
 connection master;

mysql-test/suite/sys_vars/r/read_only_error_msg_extra_basic.result

@@ -0,0 +1,22 @@
+set @@global.read_only_error_msg_extra='Custom message before read_only';
+select @@global.read_only_error_msg_extra;
+@@global.read_only_error_msg_extra
+Custom message before read_only
+set global read_only = true;
+set @@global.read_only_error_msg_extra = default;
+select @@global.read_only_error_msg_extra;
+@@global.read_only_error_msg_extra
+
+set @saved_read_only_error_msg_extra = @@global.read_only_error_msg_extra;
+set @@global.read_only_error_msg_extra='This is a custom message';
+set global super_read_only = true;
+set @@global.read_only_error_msg_extra='This is another custom message';
+set @@global.read_only_error_msg_extra=1;
+ERROR 42000: Incorrect argument type to variable 'read_only_error_msg_extra'
+select @@session.read_only_error_msg_extra;
+ERROR HY000: Variable 'read_only_error_msg_extra' is a GLOBAL variable
+set @@session.read_only_error_msg_extra='This is a custom message';
+ERROR HY000: Variable 'read_only_error_msg_extra' is a GLOBAL variable and should be set with SET GLOBAL
+set global read_only_error_msg_extra = @saved_read_only_error_msg_extra;
+set global super_read_only = false;
+set global read_only = false;

mysql-test/suite/sys_vars/t/read_only_error_msg_extra_basic.test

@@ -0,0 +1,25 @@
+set @@global.read_only_error_msg_extra='Custom message before read_only';
+select @@global.read_only_error_msg_extra;
+
+set global read_only = true;
+set @@global.read_only_error_msg_extra = default;
+select @@global.read_only_error_msg_extra;
+set @saved_read_only_error_msg_extra = @@global.read_only_error_msg_extra;
+
+set @@global.read_only_error_msg_extra='This is a custom message';
+
+set global super_read_only = true;
+set @@global.read_only_error_msg_extra='This is another custom message';
+
+--error ER_WRONG_TYPE_FOR_VAR
+set @@global.read_only_error_msg_extra=1;
+
+--error ER_INCORRECT_GLOBAL_LOCAL_VAR
+select @@session.read_only_error_msg_extra;
+--error ER_GLOBAL_VARIABLE
+set @@session.read_only_error_msg_extra='This is a custom message';
+
+set global read_only_error_msg_extra = @saved_read_only_error_msg_extra;
+
+set global super_read_only = false;
+set global read_only = false;

share/messages_to_clients.txt

@@ -5120,12 +5120,12 @@ ER_FEATURE_DISABLED
         spa "El recurso '%s' fue deshabilitado; usted necesita construir MySQL con '%s' para tener eso funcionando"
         swe "'%s' är inte aktiverad; För att aktivera detta måste du bygga om MySQL med '%s' definierad"
 ER_OPTION_PREVENTS_STATEMENT
-        eng "The MySQL server is running with the %s option so it cannot execute this statement"
-        ger "Der MySQL-Server läuft mit der Option %s und kann diese Anweisung deswegen nicht ausführen"
-        jpn "MySQLサーバーが %s オプションで実行されているので、このステートメントは実行できません。"
-        por "O servidor MySQL está rodando com a opção %s razão pela qual não pode executar esse commando"
-        spa "El servidor MySQL está rodando con la opción %s tal que no puede ejecutar este comando"
-        swe "MySQL är startad med %s. Pga av detta kan du inte använda detta kommando"
+        eng "The MySQL server is running with the %s option so it cannot execute this statement%s"
+        ger "Der MySQL-Server läuft mit der Option %s und kann diese Anweisung deswegen nicht ausführen%s"
+        jpn "MySQLサーバーが %s オプションで実行されているので、このステートメントは実行できません。%s"
+        por "O servidor MySQL está rodando com a opção %s razão pela qual não pode executar esse commando%s"
+        spa "El servidor MySQL está rodando con la opción %s tal que no puede ejecutar este comando%s"
+        swe "MySQL är startad med %s. Pga av detta kan du inte använda detta kommando%s"
 ER_DUPLICATED_VALUE_IN_TYPE
         eng "Column '%-.100s' has duplicated value '%-.64s' in %s"
         ger "Feld '%-.100s' hat doppelten Wert '%-.64s' in %s"

sql/auth/sql_authorization.cc

@@ -105,6 +105,7 @@
 #include "sql/mysqld.h" /* lower_case_table_names */
 #include "sql/nested_join.h"
 #include "sql/protocol.h"
+#include "sql/rpl_slave.h"  /* get_active_master_info */
 #include "sql/sp.h"         /* sp_exist_routines */
 #include "sql/sql_admin.h"  // enum role_enum
 #include "sql/sql_alter.h"
@@ -1910,13 +1911,15 @@ bool check_readonly(THD *thd, bool err_if_readonly) {
 
 */
 void err_readonly(THD *thd) {
+  std::string extra_info = get_active_master_info();
   my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0),
            thd->security_context()->check_access(SUPER_ACL) ||
                    thd->security_context()
                        ->has_global_grant(STRING_WITH_LEN("CONNECTION_ADMIN"))
                        .first
                ? "--super-read-only"
-               : "--read-only");
+               : "--read-only",
+           extra_info.c_str());
 }
 
 /**

sql/auth/sql_user_table.cc

@@ -1554,7 +1554,7 @@ int replace_routine_table(THD *thd, GRANT_NAME *grant_name, TABLE *table,
   DBUG_TRACE;
 
   if (!initialized) {
-    my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--skip-grant-tables");
+    my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--skip-grant-tables", "");
     return -1;
   }
 
@@ -1766,7 +1766,7 @@ int open_grant_tables(THD *thd, TABLE_LIST *tables,
   DBUG_TRACE;
 
   if (!initialized) {
-    my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--skip-grant-tables");
+    my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--skip-grant-tables", "");
     return -1;
   }
 

sql/dd/impl/sdi_file.cc

@@ -189,7 +189,7 @@ bool expand_sdi_pattern(const Dir_pat_tuple &dpt,
     x.append(opt_secure_file_priv);
     x.append("'");
     /* Read only allowed from within dir specified by secure_file_priv */
-    my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), x.c_str());
+    my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), x.c_str(), "");
     return true;
   }
 

sql/mysqld.cc

@@ -1063,6 +1063,7 @@ handlerton *temptable_hton;
 handlerton *myisam_hton;
 handlerton *innodb_hton;
 
+char *opt_read_only_error_msg_extra;
 char *opt_disabled_storage_engines;
 uint opt_server_id_bits = 0;
 ulong opt_server_id_mask = 0;

sql/mysqld.h

@@ -198,6 +198,7 @@ extern ulonglong slave_type_conversions_options;
 extern bool read_only, opt_readonly;
 extern bool super_read_only, opt_super_readonly;
 extern bool send_error_before_closing_timed_out_connection;
+extern char *opt_read_only_error_msg_extra;
 extern bool lower_case_file_system;
 
 enum enum_slave_rows_search_algorithms {

sql/query_result.cc

@@ -211,7 +211,7 @@ static File create_file(THD *thd, char *path, sql_exchange *exchange,
 
   if (!is_secure_file_path(path)) {
     /* Write only allowed to dir or subdir specified by secure_file_priv */
-    my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--secure-file-priv");
+    my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--secure-file-priv", "");
     return -1;
   }
 

sql/rpl_slave.cc

@@ -10275,6 +10275,29 @@ static int check_slave_sql_config_conflict(const Relay_log_info *rli) {
   return 0;
 }
 
+// This function will generate the string containing current master host and
+// port info if available.
+std::string get_active_master_info() {
+  std::string str_ptr;
+
+  channel_map.rdlock();
+  Master_info *mi = channel_map.get_default_channel_mi();
+
+  if (Master_info::is_configured(mi)) {
+    str_ptr = ". Current master_host: ";
+    str_ptr += mi->host;
+    str_ptr += ", master_port: ";
+    str_ptr += std::to_string(mi->port);
+    if (opt_read_only_error_msg_extra && opt_read_only_error_msg_extra[0]) {
+      str_ptr += ". ";
+      str_ptr += opt_read_only_error_msg_extra;
+    }
+  }
+
+  channel_map.unlock();
+  return str_ptr;
+}
+
 /**
   @} (end of group Replication)
 */

sql/rpl_slave.h

@@ -501,6 +501,7 @@ bool start_slave_thread(
     std::atomic<uint> *slave_running, std::atomic<ulong> *slave_run_id,
     Master_info *mi);
 
+std::string get_active_master_info();
 bool show_slave_status(THD *thd, Master_info *mi);
 bool show_slave_status(THD *thd);
 bool rpl_master_has_bug(const Relay_log_info *rli, uint bug_id, bool report,

sql/sql_load.cc

@@ -461,12 +461,13 @@ bool Sql_cmd_load_table::execute_inner(THD *thd,
           --slave-load-tmpdir". This should never happen. Please, report a bug.
         */
         LogErr(ERROR_LEVEL, ER_LOAD_DATA_INFILE_FAILED_IN_UNEXPECTED_WAY);
-        my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--slave-load-tmpdir");
+        my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--slave-load-tmpdir",
+                 "");
         return true;
       }
     } else if (!is_secure_file_path(name)) {
       /* Read only allowed from within dir specified by secure_file_priv */
-      my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--secure-file-priv");
+      my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--secure-file-priv", "");
       return true;
     }
 

sql/sql_parse.cc

@@ -3200,7 +3200,7 @@ int mysql_execute_command(THD *thd, bool first_level) {
 
   /* Check if the statement fulfill the requirements on ACL CACHE */
   if (!command_satisfy_acl_cache_requirement(lex->sql_command)) {
-    my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--skip-grant-tables");
+    my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--skip-grant-tables", "");
     goto error;
   }
 

sql/sys_vars.cc

@@ -2001,7 +2001,7 @@ static bool event_scheduler_check(sys_var *, THD *, set_var *var) {
   if (var->save_result.ulonglong_value == Events::EVENTS_DISABLED) return true;
   if (Events::opt_event_scheduler == Events::EVENTS_DISABLED) {
     my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0),
-             "--event-scheduler=DISABLED or --skip-grant-tables");
+             "--event-scheduler=DISABLED or --skip-grant-tables", "");
     return true;
   }
   return false;
@@ -7009,8 +7009,16 @@ static Sys_var_charptr Sys_per_user_session_var_default_val(
     IN_FS_CHARSET, DEFAULT(0), NO_MUTEX_GUARD, NOT_IN_BINLOG,
     ON_CHECK(check_per_user_session_var));
 
+
 static Sys_var_bool Sys_send_error_before_closing_timed_out_connection(
     "send_error_before_closing_timed_out_connection",
     "Send error before closing connections due to timeout.",
     GLOBAL_VAR(send_error_before_closing_timed_out_connection),
     CMD_LINE(OPT_ARG), DEFAULT(true));
+
+static Sys_var_charptr Sys_read_only_error_msg_extra(
+    "read_only_error_msg_extra",
+    "Set this variable to print out extra error information, "
+    "which will be appended to read_only error messages.",
+    GLOBAL_VAR(opt_read_only_error_msg_extra), CMD_LINE(OPT_ARG),
+    IN_SYSTEM_CHARSET, DEFAULT(""), NO_MUTEX_GUARD, NOT_IN_BINLOG);