Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FB8-76: Add current master host:port to server read only error when available #958

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions mysql-test/r/all_persisted_variables.result
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,17 @@ include/assert.inc ['Expect 500+ variables in the table. Due to open Bugs, we ar

# Test SET PERSIST

include/assert.inc ['Expect 385 persisted variables in the table. Due to open Bugs, we are checking for 379']
include/assert.inc ['Expect 386 persisted variables in the table. Due to open Bugs, we are checking for 380']

************************************************************
* 3. Restart server, it must preserve the persisted variable
* settings. Verify persisted configuration.
************************************************************
# restart

include/assert.inc ['Expect 379 persisted variables in persisted_variables table.']
include/assert.inc ['Expect 379 persisted variables shown as PERSISTED in variables_info table.']
include/assert.inc ['Expect 379 persisted variables with matching peristed and global values.']
include/assert.inc ['Expect 380 persisted variables in persisted_variables table.']
include/assert.inc ['Expect 380 persisted variables shown as PERSISTED in variables_info table.']
include/assert.inc ['Expect 380 persisted variables with matching peristed and global values.']

************************************************************
* 4. Test RESET PERSIST IF EXISTS. Verify persisted variable
Expand Down
1 change: 1 addition & 0 deletions mysql-test/r/bug58669.result
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions mysql-test/r/mysqld--help-notwin.result
Original file line number Diff line number Diff line change
Expand Up @@ -986,6 +986,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
Expand Down Expand Up @@ -1659,6 +1662,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
Expand Down
10 changes: 8 additions & 2 deletions mysql-test/suite/rpl_nogtid/r/rpl_read_only.result
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,15 @@ a
2004
2005
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
insert into t2 values(2006);
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
insert into t2 values(2006);
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;
drop table t2;
Expand Down
15 changes: 15 additions & 0 deletions mysql-test/suite/rpl_nogtid/t/rpl_read_only.test
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,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;

Expand Down Expand Up @@ -112,10 +113,24 @@ select * from t2;

# 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
--error ER_OPTION_PREVENTS_STATEMENT
insert into t2 values(2006);
# 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
--error ER_OPTION_PREVENTS_STATEMENT
insert into t2 values(2006);
connection slave2_root;
set global read_only_error_msg_extra = default;

## Cleanup
connection master;
Expand Down
22 changes: 22 additions & 0 deletions mysql-test/suite/sys_vars/r/read_only_error_msg_extra_basic.result
Original file line number Diff line number Diff line change
@@ -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;
25 changes: 25 additions & 0 deletions mysql-test/suite/sys_vars/t/read_only_error_msg_extra_basic.test
Original file line number Diff line number Diff line change
@@ -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;
2 changes: 1 addition & 1 deletion mysql-test/t/all_persisted_variables.test
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
--source include/have_binlog_format_row.inc

let $total_global_vars=`SELECT COUNT(*) FROM performance_schema.global_variables where variable_name NOT LIKE 'ndb_%'`;
let $total_persistent_vars=385;
let $total_persistent_vars=386;
# Due to open bugs, there are fewer variables
--let $total_persistent_vars_sans_bugs=`SELECT $total_persistent_vars - 6;`

Expand Down
12 changes: 6 additions & 6 deletions share/errmsg-utf8.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5080,12 +5080,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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original code added a new ER_OPTION_PREVENTS_STATEMENT_EXTRA_INFO code. It would be better to keep to the original code instead. The value of the error code returned would match the existing 5.6 version. The return codes would be different between servers that have a master vs. those that do not, without having to parse the return string to determine if host:port were appended to it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change matches existing 5.6 version:
ER_OPTION_PREVENTS_STATEMENT_EXTRA_INFO was removed with 9f18725 in favor of ER_OPTION_PREVENTS_STATEMENT with additional string param.

Copy link
Contributor Author

@inikep inikep Feb 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only changed output to match upstream when additional parameter is an empty string e.g.

ERROR HY000: The MySQL server is running with the --secure-file-priv option so it cannot execute this statement

instead of

ERROR HY000: The MySQL server is running with the --secure-file-priv option so it cannot execute this statement. 

in 5.6 with additional . at the end.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, thanks. I missed checking the last diff that reverted this behavior.

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"
Expand Down
5 changes: 4 additions & 1 deletion sql/auth/sql_authorization.cc
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,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"
Expand Down Expand Up @@ -1638,13 +1639,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());
}

/**
Expand Down
4 changes: 2 additions & 2 deletions sql/auth/sql_user_table.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2035,7 +2035,7 @@ int replace_routine_table(THD *thd, GRANT_NAME *grant_name, TABLE *table,
DBUG_ENTER("replace_routine_table");

if (!initialized) {
my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--skip-grant-tables");
my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--skip-grant-tables", "");
DBUG_RETURN(-1);
}

Expand Down Expand Up @@ -2258,7 +2258,7 @@ int open_grant_tables(THD *thd, TABLE_LIST *tables,
DBUG_ENTER("open_grant_tables");

if (!initialized) {
my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--skip-grant-tables");
my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--skip-grant-tables", "");
DBUG_RETURN(-1);
}

Expand Down
2 changes: 1 addition & 1 deletion sql/dd/impl/sdi_file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
1 change: 1 addition & 0 deletions sql/mysqld.cc
Original file line number Diff line number Diff line change
Expand Up @@ -989,6 +989,7 @@ uint opt_server_id_bits = 0;
ulong opt_server_id_mask = 0;
bool read_only = 0, opt_readonly = 0;
bool super_read_only = 0, opt_super_readonly = 0;
char *opt_read_only_error_msg_extra;
bool send_error_before_closing_timed_out_connection = 0;
bool opt_require_secure_transport = 0;
bool relay_log_purge;
Expand Down
1 change: 1 addition & 0 deletions sql/mysqld.h
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ extern ulonglong slave_type_conversions_options;

extern bool read_only, opt_readonly;
extern bool super_read_only, opt_super_readonly;
extern char *opt_read_only_error_msg_extra;
extern bool send_error_before_closing_timed_out_connection;
extern bool lower_case_file_system;

Expand Down
2 changes: 1 addition & 1 deletion sql/query_result.cc
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,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;
}

Expand Down
23 changes: 23 additions & 0 deletions sql/rpl_slave.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9749,6 +9749,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)
*/
1 change: 1 addition & 0 deletions sql/rpl_slave.h
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,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,
Expand Down
5 changes: 3 additions & 2 deletions sql/sql_load.cc
Original file line number Diff line number Diff line change
Expand Up @@ -443,12 +443,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",
"");
DBUG_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", "");
DBUG_RETURN(true);
}

Expand Down
2 changes: 1 addition & 1 deletion sql/sql_parse.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3089,7 +3089,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;
}

Expand Down
9 changes: 8 additions & 1 deletion sql/sys_vars.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1741,7 +1741,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;
Expand Down Expand Up @@ -6421,3 +6421,10 @@ static Sys_var_bool Sys_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);