Skip to content

Commit

Permalink
Make "SET @@rocksdb_bulk_load=0" return an error instead of crashing …
Browse files Browse the repository at this point in the history
…the server

- This is more in line with MariaDB environment
- And help with rocksdb.bulk_load_errors test, too
  • Loading branch information
spetrunia committed Aug 3, 2017
1 parent fcb8d8e commit eda0332
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 15 deletions.
3 changes: 2 additions & 1 deletion sql/sql_parse.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4821,7 +4821,8 @@ mysql_execute_command(THD *thd)
goto error;
if (!(res= sql_set_variables(thd, lex_var_list, true)))
{
my_ok(thd);
if (!thd->is_error())
my_ok(thd);
}
else
{
Expand Down
9 changes: 8 additions & 1 deletion storage/rocksdb/ha_rocksdb.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11493,7 +11493,14 @@ void rocksdb_set_bulk_load(THD *const thd, struct st_mysql_sys_var *const var
sql_print_error("RocksDB: Error %d finalizing last SST file while "
"setting bulk loading variable",
rc);
abort_with_stack_traces();
/*
MariaDB doesn't do the following:
abort_with_stack_traces();
because it doesn't seem a good idea to crash a server when a user makes
a mistake.
Instead, we return an error to the user. The error has already been
produced inside ha_rocksdb::finalize_bulk_load().
*/
}
}

Expand Down
12 changes: 11 additions & 1 deletion storage/rocksdb/mysql-test/rocksdb/r/bulk_load_errors.result
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,16 @@ INSERT INTO t1 VALUES(1);
INSERT INTO t1 VALUES(2);
INSERT INTO t1 VALUES(20);
INSERT INTO t1 VALUES(21);
#
# In MyRocks, the following statement will intentionally crash the server.
# In MariaDB, it will cause an error
SET rocksdb_bulk_load=0;
ERROR HY000: Lost connection to MySQL server during query
ERROR HY000: Rows inserted during bulk load must not overlap existing rows
#
# Despite the error, bulk load operation is over so the variable value
# will be 0:
select @@rocksdb_bulk_load;
@@rocksdb_bulk_load
0
call mtr.add_suppression('finalizing last SST file while setting bulk loading variable');
DROP TABLE t1;
21 changes: 9 additions & 12 deletions storage/rocksdb/mysql-test/rocksdb/t/bulk_load_errors.test
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,17 @@ INSERT INTO t1 VALUES(2);
INSERT INTO t1 VALUES(20);
INSERT INTO t1 VALUES(21);

# This last crashes the server (intentionally) because we can't return any
# error information from a SET <variable>=<value>
--exec echo "wait" > $MYSQLTEST_VARDIR/tmp/mysqld.1.expect
--error 2013
--echo #
--echo # In MyRocks, the following statement will intentionally crash the server.
--echo # In MariaDB, it will cause an error
--error ER_OVERLAPPING_KEYS
SET rocksdb_bulk_load=0;

--exec grep "RocksDB: Error 197 finalizing last SST file while setting bulk loading variable" $MYSQLTEST_VARDIR/log/mysqld.1.err | cut -d] -f2
--exec echo "" >$MYSQLTEST_VARDIR/log/mysqld.1.err
--echo #
--echo # Despite the error, bulk load operation is over so the variable value
--echo # will be 0:
select @@rocksdb_bulk_load;

# restart the crashed server
--exec echo "restart" > $MYSQLTEST_VARDIR/tmp/mysqld.1.expect

# Make sure the error exists in the .err log and then restart the server
--enable_reconnect
--source include/wait_until_connected_again.inc
call mtr.add_suppression('finalizing last SST file while setting bulk loading variable');

DROP TABLE t1;

0 comments on commit eda0332

Please sign in to comment.