Skip to content

Commit 7451e2c

Browse files
ti-chi-botCalvinNeoJaySon-Huang
authored
KVStore: ignore non-exist IStorage instance in releasePreHandledSnapshot (#10607) (#10613)
close #10606 KVStore: ignore non-exist IStorage instance in releasePreHandledSnapshot * `KVStore::releasePreHandledSnapshot` skip for external files is empty or IStorage instance is not exist Signed-off-by: Calvin Neo <calvinneo1995@gmail.com> Signed-off-by: JaySon-Huang <tshent@qq.com> Co-authored-by: Calvin Neo <calvinneo1995@gmail.com> Co-authored-by: JaySon-Huang <tshent@qq.com>
1 parent 62b07d2 commit 7451e2c

File tree

3 files changed

+147
-2
lines changed

3 files changed

+147
-2
lines changed

‎dbms/src/Common/FailPoint.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,8 @@ namespace DB
128128
M(force_join_v2_probe_enable_lm) \
129129
M(force_join_v2_probe_disable_lm) \
130130
M(force_s3_random_access_file_init_fail) \
131-
M(force_s3_random_access_file_read_fail)
131+
M(force_s3_random_access_file_read_fail) \
132+
M(force_release_snap_meet_null_storage)
132133

133134
#define APPLY_FOR_PAUSEABLE_FAILPOINTS_ONCE(M) \
134135
M(pause_with_alter_locks_acquired) \

‎dbms/src/Storages/KVStore/MultiRaft/PrehandleSnapshot.cpp‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@
3434
#include <Storages/StorageDeltaMergeHelpers.h>
3535
#include <TiDB/Schema/SchemaSyncer.h>
3636
#include <TiDB/Schema/TiDBSchemaManager.h>
37+
#include <common/logger_useful.h>
38+
#include <fiu.h>
3739

3840
#include <chrono>
3941

@@ -53,6 +55,7 @@ extern const char force_set_parallel_prehandle_threshold[];
5355
extern const char force_raise_prehandle_exception[];
5456
extern const char pause_before_prehandle_snapshot[];
5557
extern const char pause_before_prehandle_subtask[];
58+
extern const char force_release_snap_meet_null_storage[];
5659
} // namespace FailPoints
5760

5861
namespace ErrorCodes
@@ -872,10 +875,35 @@ void KVStore::releasePreHandledSnapshot<RegionPtrWithSnapshotFiles>(
872875
const RegionPtrWithSnapshotFiles & s,
873876
TMTContext & tmt)
874877
{
875-
auto & storages = tmt.getStorages();
876878
auto keyspace_id = s.base->getKeyspaceID();
877879
auto table_id = s.base->getMappedTableID();
880+
if (s.external_files.empty())
881+
{
882+
LOG_INFO(
883+
log,
884+
"Release prehandled snapshot is skipped, no external files, region_id={} keyspace={} table_id={}",
885+
s.base->id(),
886+
keyspace_id,
887+
table_id);
888+
return;
889+
}
890+
891+
auto & storages = tmt.getStorages();
878892
auto storage = storages.get(keyspace_id, table_id);
893+
fiu_do_on(FailPoints::force_release_snap_meet_null_storage, {
894+
LOG_WARNING(log, "failpoint force_release_snap_meet_null_storage is enabled");
895+
storage = nullptr;
896+
});
897+
if (storage == nullptr)
898+
{
899+
LOG_WARNING(
900+
log,
901+
"Release prehandled snapshot is ignore because Storage is not exist, region_id={} keyspace={} table_id={}",
902+
s.base->id(),
903+
keyspace_id,
904+
table_id);
905+
return;
906+
}
879907
if (storage->engineType() != TiDB::StorageEngine::DT)
880908
{
881909
return;

‎dbms/src/Storages/KVStore/tests/gtest_raftstore_v2.cpp‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15+
#include <Storages/KVStore/Decode/RegionTable.h>
1516
#include <Storages/KVStore/Read/LearnerRead.h>
1617
#include <Storages/KVStore/tests/region_kvstore_test.h>
1718

@@ -22,6 +23,7 @@ namespace FailPoints
2223
extern const char force_raise_prehandle_exception[];
2324
extern const char pause_before_prehandle_subtask[];
2425
extern const char force_set_sst_to_dtfile_block_size[];
26+
extern const char force_release_snap_meet_null_storage[];
2527
} // namespace FailPoints
2628

2729
namespace tests
@@ -528,6 +530,120 @@ try
528530
}
529531
CATCH
530532

533+
// Test when a region is cancel during releasing pre-handled snapshot.
534+
// And the pre-handled snapshot contains no external files.
535+
TEST_F(RegionKVStoreV2Test, KVStoreSingleSnapReleaseNoExternalFiles)
536+
try
537+
{
538+
auto & ctx = TiFlashTestEnv::getGlobalContext();
539+
proxy_instance->cluster_ver = RaftstoreVer::V2;
540+
ASSERT_NE(proxy_helper->sst_reader_interfaces.fn_key, nullptr);
541+
ASSERT_NE(proxy_helper->fn_get_config_json, nullptr);
542+
FailPointHelper::enableFailPoint(FailPoints::force_release_snap_meet_null_storage, static_cast<size_t>(0));
543+
SCOPE_EXIT({ FailPointHelper::disableFailPoint(FailPoints::force_release_snap_meet_null_storage); });
544+
UInt64 region_id = 2;
545+
initStorages();
546+
KVStore & kvs = getKVS();
547+
TableID table_id = proxy_instance->bootstrapTable(ctx, kvs, ctx.getTMTContext());
548+
auto start = RecordKVFormat::genKey(table_id, 0);
549+
auto end = RecordKVFormat::genKey(table_id, 40);
550+
proxy_instance->bootstrapWithRegion( //
551+
kvs,
552+
ctx.getTMTContext(),
553+
region_id,
554+
std::make_pair(start.toString(), end.toString()));
555+
auto r1 = proxy_instance->getRegion(region_id);
556+
557+
auto [value_write, value_default] = proxy_instance->generateTiKVKeyValue(111, 999);
558+
auto kkk = RecordKVFormat::decodeWriteCfValue(TiKVValue::copyFrom(value_write));
559+
{
560+
// mock an empty region
561+
MockSSTReader::getMockSSTData().clear();
562+
MockSSTGenerator default_cf{region_id, table_id, ColumnFamilyType::Default};
563+
default_cf.finish_file(SSTFormatKind::KIND_TABLET);
564+
default_cf.freeze();
565+
MockSSTGenerator write_cf{region_id, table_id, ColumnFamilyType::Write};
566+
write_cf.finish_file(SSTFormatKind::KIND_TABLET);
567+
write_cf.freeze();
568+
auto [prehandle_region, res] = proxy_instance->snapshot( //
569+
kvs,
570+
ctx.getTMTContext(),
571+
region_id,
572+
{default_cf, write_cf},
573+
0,
574+
0,
575+
std::nullopt);
576+
kvs.abortPreHandleSnapshot(region_id, ctx.getTMTContext());
577+
RegionPtrWithSnapshotFiles region_with_snap(prehandle_region, {});
578+
kvs.releasePreHandledSnapshot(region_with_snap, ctx.getTMTContext());
579+
}
580+
}
581+
CATCH
582+
583+
// Test when a region is cancel during releasing pre-handled snapshot.
584+
// And the pre-handled snapshot's storage is null.
585+
TEST_F(RegionKVStoreV2Test, KVStoreSingleSnapReleaseNullStorage)
586+
try
587+
{
588+
auto & ctx = TiFlashTestEnv::getGlobalContext();
589+
proxy_instance->cluster_ver = RaftstoreVer::V2;
590+
ASSERT_NE(proxy_helper->sst_reader_interfaces.fn_key, nullptr);
591+
ASSERT_NE(proxy_helper->fn_get_config_json, nullptr);
592+
FailPointHelper::enableFailPoint(FailPoints::force_release_snap_meet_null_storage, static_cast<size_t>(0));
593+
SCOPE_EXIT({ FailPointHelper::disableFailPoint(FailPoints::force_release_snap_meet_null_storage); });
594+
UInt64 region_id = 2;
595+
initStorages();
596+
KVStore & kvs = getKVS();
597+
TableID table_id = proxy_instance->bootstrapTable(ctx, kvs, ctx.getTMTContext());
598+
auto start = RecordKVFormat::genKey(table_id, 0);
599+
auto end = RecordKVFormat::genKey(table_id, 40);
600+
HandleID sst_limit = 40;
601+
proxy_instance->bootstrapWithRegion( //
602+
kvs,
603+
ctx.getTMTContext(),
604+
region_id,
605+
std::make_pair(start.toString(), end.toString()));
606+
auto r1 = proxy_instance->getRegion(region_id);
607+
608+
auto [value_write, value_default] = proxy_instance->generateTiKVKeyValue(111, 999);
609+
auto kkk = RecordKVFormat::decodeWriteCfValue(TiKVValue::copyFrom(value_write));
610+
{
611+
MockSSTReader::getMockSSTData().clear();
612+
MockSSTGenerator default_cf{region_id, table_id, ColumnFamilyType::Default};
613+
for (HandleID h = 1; h < sst_limit; h++)
614+
{
615+
auto k = RecordKVFormat::genKey(table_id, h, 111);
616+
default_cf.insert_raw(k, value_default);
617+
}
618+
default_cf.finish_file(SSTFormatKind::KIND_TABLET);
619+
default_cf.freeze();
620+
MockSSTGenerator write_cf{region_id, table_id, ColumnFamilyType::Write};
621+
for (HandleID h = 1; h < sst_limit; h++)
622+
{
623+
auto k = RecordKVFormat::genKey(table_id, h, 111);
624+
write_cf.insert_raw(k, value_write);
625+
}
626+
write_cf.finish_file(SSTFormatKind::KIND_TABLET);
627+
write_cf.freeze();
628+
629+
auto [prehandle_region, res] = proxy_instance->snapshot( //
630+
kvs,
631+
ctx.getTMTContext(),
632+
region_id,
633+
{default_cf, write_cf},
634+
0,
635+
0,
636+
std::nullopt,
637+
[] {
638+
// set the function so that will call releasePreHandledSnapshot
639+
// and verify the null storage handling logic
640+
return nullptr;
641+
});
642+
UNUSED(prehandle_region, res);
643+
}
644+
}
645+
CATCH
646+
531647
// Test several uncommitted keys with only one version.
532648
TEST_F(RegionKVStoreV2Test, KVStoreSingleSnap1)
533649
try

0 commit comments

Comments
 (0)