From 32f2910552eeada00b1554add3d42f1430a1d58b Mon Sep 17 00:00:00 2001 From: lx0 Date: Tue, 1 Mar 2022 15:51:49 +0800 Subject: [PATCH] fix check_row_lock bug --- .../blocksstable/ob_micro_block_reader.cpp | 93 +++++++++++-------- src/storage/ob_sstable_row_iterator.cpp | 7 +- src/storage/transaction/ob_trans_part_ctx.cpp | 3 +- 3 files changed, 63 insertions(+), 40 deletions(-) diff --git a/src/storage/blocksstable/ob_micro_block_reader.cpp b/src/storage/blocksstable/ob_micro_block_reader.cpp index ca400167b6..f511c8fec6 100644 --- a/src/storage/blocksstable/ob_micro_block_reader.cpp +++ b/src/storage/blocksstable/ob_micro_block_reader.cpp @@ -252,45 +252,62 @@ int ObMicroBlockGetReader::check_row_locked_(ObIRowReader* row_reader_ptr, memta const int64_t sql_sequence_col_idx = ObMultiVersionRowkeyHelpper::get_sql_sequence_col_store_index(rowkey_cnt, extra_multi_version_col_cnt); - if (OB_FAIL(row_reader_ptr->setup_row( - data_begin_, index_data_[row_idx_ + 1], index_data_[row_idx_], header_->column_count_, &trans_id))) { - LOG_WARN("fail to setup row", K(ret)); - } else if (OB_FAIL(row_reader_ptr->get_row_header(row_header))) { // get row header - LOG_WARN("fail to get row header", K(ret)); - } else if (OB_ISNULL(row_header)) { - ret = OB_ERR_UNEXPECTED; - LOG_WARN("row header is null", K(ret)); - } else if (OB_FAIL(row_reader_ptr->read_column(version_column_meta, - allocator_, - trans_version_col_idx, - version_cell))) { // read trans version - LOG_WARN("fail to read version column", K(ret)); - } else if (OB_FAIL(version_cell.get_int(lock_state.trans_version_))) { - LOG_WARN("fail to convert version cell to int", K(ret), K(version_cell)); - } else { - lock_state.trans_version_ = -lock_state.trans_version_; - } - if (OB_SUCC(ret)) { - flag.flag_ = row_header->get_row_type_flag(); - // check if row is uncommitted - if (flag.is_uncommitted_row()) { - lock_state.trans_version_ = INT64_MAX; - if (OB_FAIL(row_reader_ptr->read_column(sql_sequence_meta, - allocator_, - sql_sequence_col_idx, - sql_sequence_cell))) { // read sql sequence - LOG_WARN("fail to read version column", K(ret)); - } else if (OB_FAIL(sql_sequence_cell.get_int(sql_sequence))) { - LOG_WARN("fail to convert sql_sequence cell to int32", K(ret), K(sql_sequence_cell)); - } else { - sql_sequence = -sql_sequence; - ret = const_cast(trans_table_guard) - .get_trans_state_table() - .check_row_locked(rowkey, ctx, read_trans_id, trans_id, sql_sequence, lock_state); - } + while (OB_SUCC(ret) && row_idx_ < header_->row_count_) { + if (OB_FAIL(row_reader_ptr->setup_row( + data_begin_, index_data_[row_idx_ + 1], index_data_[row_idx_], header_->column_count_, &trans_id))) { + LOG_WARN("fail to setup row", K(ret)); + } else if (OB_FAIL(row_reader_ptr->get_row_header(row_header))) { // get row header + LOG_WARN("fail to get row header", K(ret)); + } else if (OB_ISNULL(row_header)) { + ret = OB_ERR_UNEXPECTED; + LOG_WARN("row header is null", K(ret)); + } else if (OB_FAIL(row_reader_ptr->read_column(version_column_meta, + allocator_, + trans_version_col_idx, + version_cell))) { // read trans version + LOG_WARN("fail to read version column", K(ret)); + } else if (OB_FAIL(version_cell.get_int(lock_state.trans_version_))) { + LOG_WARN("fail to convert version cell to int", K(ret), K(version_cell)); + } else { + lock_state.trans_version_ = -lock_state.trans_version_; + } + if (OB_SUCC(ret)) { + flag.flag_ = row_header->get_row_type_flag(); + // check if row is uncommitted + if (flag.is_uncommitted_row()) { + lock_state.trans_version_ = INT64_MAX; + if (OB_FAIL(row_reader_ptr->read_column(sql_sequence_meta, + allocator_, + sql_sequence_col_idx, + sql_sequence_cell))) { // read sql sequence + LOG_WARN("fail to read version column", K(ret)); + } else if (OB_FAIL(sql_sequence_cell.get_int(sql_sequence))) { + LOG_WARN("fail to convert sql_sequence cell to int32", K(ret), K(sql_sequence_cell)); + } else { + sql_sequence = -sql_sequence; + ret = const_cast(trans_table_guard) + .get_trans_state_table() + .check_row_locked(rowkey, ctx, read_trans_id, trans_id, sql_sequence, lock_state); + if (flag.is_last_multi_version_row() // meet last row + || 0 != lock_state.trans_version_ // trans is commit or abort + || lock_state.is_locked_) { + break; + } + } + } else { // meet committed row, should break + break; + } + ++row_idx_; + + STORAGE_LOG(DEBUG, + "check row lock", + K(ret), + K(rowkey), + K(read_trans_id), + K(trans_id), + K(sql_sequence), + K(lock_state)); } - STORAGE_LOG( - DEBUG, "check row lock", K(ret), K(rowkey), K(read_trans_id), K(trans_id), K(sql_sequence), K(lock_state)); } } } diff --git a/src/storage/ob_sstable_row_iterator.cpp b/src/storage/ob_sstable_row_iterator.cpp index 6c261bfdab..4fc46e5bb1 100644 --- a/src/storage/ob_sstable_row_iterator.cpp +++ b/src/storage/ob_sstable_row_iterator.cpp @@ -1740,6 +1740,7 @@ int ObSSTableRowIterator::check_row_locked(ObSSTableReadHandle& read_handle, ObS { int ret = OB_SUCCESS; lock_state.is_locked_ = false; + lock_state.trans_version_ = 0; switch (read_handle.state_) { case ObSSTableRowState::NOT_EXIST: @@ -1778,7 +1779,11 @@ int ObSSTableRowIterator::check_block_row_lock(ObSSTableReadHandle& read_handle, } if (OB_SUCC(ret)) { - for (int64_t i = read_handle.micro_begin_idx_; OB_SUCC(ret) && i <= read_handle.micro_end_idx_; ++i) { + // there is only one trans in one sstable, + // when 0 != lock_state.trans_version_ means cur trans is commit or abort, no need loop + for (int64_t i = read_handle.micro_begin_idx_; + OB_SUCC(ret) && i <= read_handle.micro_end_idx_ && !lock_state.is_locked_ && 0 == lock_state.trans_version_; + ++i) { if (OB_FAIL(get_block_data(i, block_data))) { STORAGE_LOG(WARN, "Fail to get block data, ", K(ret), K(read_handle)); } else { diff --git a/src/storage/transaction/ob_trans_part_ctx.cpp b/src/storage/transaction/ob_trans_part_ctx.cpp index ac1bb617e9..64e6ed935d 100644 --- a/src/storage/transaction/ob_trans_part_ctx.cpp +++ b/src/storage/transaction/ob_trans_part_ctx.cpp @@ -11206,6 +11206,7 @@ int ObPartTransCtx::check_row_locked_(const ObTransStatusInfo& trans_info, { int ret = OB_SUCCESS; + // when lock_state.trans_version_ != 0 means cur trans is commit or abort switch (trans_info.status_) { case ObTransTableStatusType::COMMIT: { lock_state.is_locked_ = false; @@ -11219,7 +11220,7 @@ int ObPartTransCtx::check_row_locked_(const ObTransStatusInfo& trans_info, } case ObTransTableStatusType::ABORT: { lock_state.is_locked_ = false; - lock_state.trans_version_ = 0; + lock_state.trans_version_ = OB_INVALID_VERSION; break; } }