Skip to content

Commit

Permalink
Fix potential segment fault introduced by #1023 (#1100)
Browse files Browse the repository at this point in the history
  • Loading branch information
windtalker authored Sep 17, 2020
1 parent 1c24ca5 commit e68bb98
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 21 deletions.
16 changes: 8 additions & 8 deletions dbms/src/Debug/dbgFuncRegion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ void dbgFuncPutRegion(Context & context, const ASTs & args, DBGInvoker::Printer

std::stringstream ss;
ss << "put region #" << region_id << ", range["
<< RecordKVFormat::DecodedTiKVKeyToHexWithoutTableID(*region->getRange()->rawKeys().first) << ", "
<< RecordKVFormat::DecodedTiKVKeyToHexWithoutTableID(*region->getRange()->rawKeys().second) << ")"
<< RecordKVFormat::DecodedTiKVKeyToReadableHandleString<true>(*region->getRange()->rawKeys().first) << ", "
<< RecordKVFormat::DecodedTiKVKeyToReadableHandleString<false>(*region->getRange()->rawKeys().second) << ")"
<< " to table #" << table_id << " with kvstore.onSnapshot";
output(ss.str());
}
Expand Down Expand Up @@ -193,8 +193,8 @@ void dbgFuncRegionSnapshotWithData(Context & context, const ASTs & args, DBGInvo
region = RegionBench::createRegion(table_info, region_id, start_keys, end_keys);
}
auto & rawkeys = region->getRange()->rawKeys();
auto start_string = RecordKVFormat::DecodedTiKVKeyToHexWithoutTableID(*rawkeys.first);
auto end_string = RecordKVFormat::DecodedTiKVKeyToHexWithoutTableID(*rawkeys.second);
auto start_string = RecordKVFormat::DecodedTiKVKeyToReadableHandleString<true>(*rawkeys.first);
auto end_string = RecordKVFormat::DecodedTiKVKeyToReadableHandleString<false>(*rawkeys.second);

auto args_begin = args.begin() + 3 + handle_column_size * 2;
auto args_end = args.end();
Expand Down Expand Up @@ -325,8 +325,8 @@ void dbgFuncRegionSnapshot(Context & context, const ASTs & args, DBGInvoker::Pri
std::move(region_info), peer_id, SnapshotViewArray(), MockTiKV::instance().getRaftIndex(region_id), RAFT_INIT_LOG_TERM, tmt);

std::stringstream ss;
ss << "put region #" << region_id << ", range[" << RecordKVFormat::DecodedTiKVKeyToHexWithoutTableID(start_decoded_key) << ", "
<< RecordKVFormat::DecodedTiKVKeyToHexWithoutTableID(end_decoded_key) << ")"
ss << "put region #" << region_id << ", range[" << RecordKVFormat::DecodedTiKVKeyToReadableHandleString<true>(start_decoded_key) << ", "
<< RecordKVFormat::DecodedTiKVKeyToReadableHandleString<false>(end_decoded_key) << ")"
<< " to table #" << table_id << " with raft commands";
output(ss.str());
}
Expand Down Expand Up @@ -419,8 +419,8 @@ void dbgFuncDumpAllRegion(Context & context, TableID table_id, bool ignore_none,
if (*rawkeys.first >= *rawkeys.second)
ss << " [none], ";
else
ss << " ranges: [" << RecordKVFormat::DecodedTiKVKeyToHexWithoutTableID(*rawkeys.first) << ", "
<< RecordKVFormat::DecodedTiKVKeyToHexWithoutTableID(*rawkeys.second) << "), ";
ss << " ranges: [" << RecordKVFormat::DecodedTiKVKeyToReadableHandleString<true>(*rawkeys.first) << ", "
<< RecordKVFormat::DecodedTiKVKeyToReadableHandleString<false>(*rawkeys.second) << "), ";
}
ss << "state: " << raft_serverpb::PeerState_Name(region->peerState());
if (auto s = region->dataInfo(); s.size() > 2)
Expand Down
8 changes: 4 additions & 4 deletions dbms/src/Storages/StorageDeltaMerge.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -516,15 +516,15 @@ BlockInputStreams StorageDeltaMerge::read( //
if (!region.required_handle_ranges.empty())
{
for (const auto & range : region.required_handle_ranges)
ss << region.region_id << "[" << RecordKVFormat::DecodedTiKVKeyToHexWithoutTableID(*range.first) << ","
<< RecordKVFormat::DecodedTiKVKeyToHexWithoutTableID(*range.second) << "),";
ss << region.region_id << "[" << RecordKVFormat::DecodedTiKVKeyToReadableHandleString<true>(*range.first) << ","
<< RecordKVFormat::DecodedTiKVKeyToReadableHandleString<false>(*range.second) << "),";
}
else
{
/// only used for test cases
const auto & range = region.range_in_table;
ss << region.region_id << "[" << RecordKVFormat::DecodedTiKVKeyToHexWithoutTableID(*range.first) << ","
<< RecordKVFormat::DecodedTiKVKeyToHexWithoutTableID(*range.second) << "),";
ss << region.region_id << "[" << RecordKVFormat::DecodedTiKVKeyToReadableHandleString<true>(*range.first) << ","
<< RecordKVFormat::DecodedTiKVKeyToReadableHandleString<false>(*range.second) << "),";
}
}
str_query_ranges = ss.str();
Expand Down
17 changes: 9 additions & 8 deletions dbms/src/Storages/Transaction/LearnerRead.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,9 @@ LearnerReadSnapshot doLearnerRead(const TiDB::TableID table_id, //
LOG_WARNING(log,
"Check memory cache, region "
<< region_id << ", version " << region_to_query.version << ", handle range ["
<< RecordKVFormat::DecodedTiKVKeyToHexWithoutTableID(*region_to_query.range_in_table.first) << ", "
<< RecordKVFormat::DecodedTiKVKeyToHexWithoutTableID(*region_to_query.range_in_table.second) << ") , status "
<< RegionException::RegionReadStatusString(status));
<< RecordKVFormat::DecodedTiKVKeyToReadableHandleString<true>(*region_to_query.range_in_table.first) << ", "
<< RecordKVFormat::DecodedTiKVKeyToReadableHandleString<false>(*region_to_query.range_in_table.second)
<< ") , status " << RegionException::RegionReadStatusString(status));
return;
}

Expand Down Expand Up @@ -154,9 +154,9 @@ LearnerReadSnapshot doLearnerRead(const TiDB::TableID table_id, //
LOG_WARNING(log,
"Check memory cache, region "
<< region_id << ", version " << region_to_query.version << ", handle range ["
<< RecordKVFormat::DecodedTiKVKeyToHexWithoutTableID(*region_to_query.range_in_table.first) << ", "
<< RecordKVFormat::DecodedTiKVKeyToHexWithoutTableID(*region_to_query.range_in_table.second) << ") , status "
<< RegionException::RegionReadStatusString(status));
<< RecordKVFormat::DecodedTiKVKeyToReadableHandleString<true>(*region_to_query.range_in_table.first) << ", "
<< RecordKVFormat::DecodedTiKVKeyToReadableHandleString<false>(*region_to_query.range_in_table.second)
<< ") , status " << RegionException::RegionReadStatusString(status));
region_status = status;
}
}
Expand Down Expand Up @@ -215,8 +215,9 @@ void validateQueryInfo(
LOG_WARNING(log,
"Check after read from Storage, region "
<< region_query_info.region_id << ", version " << region_query_info.version //
<< ", handle range [" << RecordKVFormat::DecodedTiKVKeyToHexWithoutTableID(*region_query_info.range_in_table.first)
<< ", " << RecordKVFormat::DecodedTiKVKeyToHexWithoutTableID(*region_query_info.range_in_table.second) << "), status "
<< ", handle range ["
<< RecordKVFormat::DecodedTiKVKeyToReadableHandleString<true>(*region_query_info.range_in_table.first) << ", "
<< RecordKVFormat::DecodedTiKVKeyToReadableHandleString<false>(*region_query_info.range_in_table.second) << "), status "
<< RegionException::RegionReadStatusString(status));
// throw region exception and let TiDB retry
throwRetryRegion(regions_query_info, status);
Expand Down
14 changes: 13 additions & 1 deletion dbms/src/Storages/Transaction/TiKVRecordFormat.h
Original file line number Diff line number Diff line change
Expand Up @@ -319,8 +319,20 @@ inline TiKVValue encodeWriteCfValue(UInt8 write_type, Timestamp ts, const String

inline TiKVValue encodeWriteCfValue(UInt8 write_type, Timestamp ts) { return internalEncodeWriteCfValue(write_type, ts, nullptr); }

inline std::string DecodedTiKVKeyToHexWithoutTableID(const DecodedTiKVKey & decoded_key)
template <bool start>
inline std::string DecodedTiKVKeyToReadableHandleString(const DecodedTiKVKey & decoded_key)
{
if (decoded_key.size() <= RAW_KEY_NO_HANDLE_SIZE)
{
if constexpr (start)
{
return "-INF";
}
else
{
return "+INF";
}
}
return ToHex(decoded_key.data() + RAW_KEY_NO_HANDLE_SIZE, decoded_key.size() - RAW_KEY_NO_HANDLE_SIZE);
}

Expand Down

0 comments on commit e68bb98

Please sign in to comment.