Skip to content
Permalink
Browse files
MDEV-15304: Server crash in print_keydup_error / key_unpack or unexpe…
…cted ER_DUP_KEY

Fix two issues:
1. Rdb_ddl_manager::rename() loses the value of m_hidden_pk_val. new
object used to get 0, which means "not loaded from the db yet".

2. ha_rocksdb::load_hidden_pk_value() uses current transaction (and its
snapshot) when loading hidden PK value from disk. This may cause it to
load an out-of-date value.
  • Loading branch information
spetrunia committed May 18, 2018
1 parent de86997 commit 727d0d4
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 8 deletions.
@@ -5235,8 +5235,17 @@ int ha_rocksdb::load_hidden_pk_value() {
active_index = m_tbl_def->m_key_count - 1;
const uint8 save_table_status = table->status;

Rdb_transaction *const tx = get_or_create_tx(table->in_use);
const bool is_new_snapshot = !tx->has_snapshot();
/*
We should read the latest committed value in the database.
That is, if we have an open transaction with a snapshot, we should not use
it as we may get old data. Start a new transaction to read the latest
value.
*/
Rdb_transaction *const temp_tx = new Rdb_transaction_impl(table->in_use);
temp_tx->start_tx();
Rdb_transaction *&tx = get_tx_from_thd(table->in_use);
Rdb_transaction *save_tx= tx;
tx= temp_tx;

longlong hidden_pk_id = 1;
// Do a lookup.
@@ -5246,9 +5255,8 @@ int ha_rocksdb::load_hidden_pk_value() {
*/
auto err = read_hidden_pk_id_from_rowkey(&hidden_pk_id);
if (err) {
if (is_new_snapshot) {
tx->release_snapshot();
}
delete tx;
tx= save_tx;
return err;
}

@@ -5260,9 +5268,8 @@ int ha_rocksdb::load_hidden_pk_value() {
!m_tbl_def->m_hidden_pk_val.compare_exchange_weak(old, hidden_pk_id)) {
}

if (is_new_snapshot) {
tx->release_snapshot();
}
delete tx;
tx= save_tx;

table->status = save_table_status;
active_index = save_active_index;
@@ -262,3 +262,34 @@ SELECT * FROM t1;
a b
36 foo
DROP TABLE t1;
#
# Issue #834/MDEV-15304 ALTER TABLE table_with_hidden_pk causes Can't
# write; duplicate key in table error and/or crash
#
CREATE TABLE t1 (a INT, KEY(a)) ENGINE=RocksDB;
INSERT INTO t1 VALUES (1),(1+1);
create table t2 (a int);
insert into t2 values (10),(20),(30);
BEGIN;
select * from t2;
a
10
20
30
connect con1,localhost,root,,;
connection con1;
alter table t1 force;
connection default;
select * from t1;
a
connection con1;
insert into t1 values (100);
select * from t1;
a
1
2
100
disconnect con1;
connection default;
rollback;
drop table t1,t2;
@@ -96,3 +96,31 @@ DELETE FROM t1 WHERE a = 35 AND b = 'foo';
--sorted_result
SELECT * FROM t1;
DROP TABLE t1;

--echo #
--echo # Issue #834/MDEV-15304 ALTER TABLE table_with_hidden_pk causes Can't
--echo # write; duplicate key in table error and/or crash
--echo #
CREATE TABLE t1 (a INT, KEY(a)) ENGINE=RocksDB;
INSERT INTO t1 VALUES (1),(1+1);
create table t2 (a int);
insert into t2 values (10),(20),(30);

BEGIN;
select * from t2;

connect (con1,localhost,root,,);
connection con1;
alter table t1 force;

connection default;
select * from t1;

connection con1;
insert into t1 values (100);
select * from t1;

disconnect con1;
connection default;
rollback;
drop table t1,t2;
@@ -4271,6 +4271,9 @@ bool Rdb_ddl_manager::rename(const std::string &from, const std::string &to,
rec->m_auto_incr_val.load(std::memory_order_relaxed);
new_rec->m_key_descr_arr = rec->m_key_descr_arr;

new_rec->m_hidden_pk_val =
rec->m_hidden_pk_val.load(std::memory_order_relaxed);

// so that it's not free'd when deleting the old rec
rec->m_key_descr_arr = nullptr;

0 comments on commit 727d0d4

Please sign in to comment.