Skip to content

Commit 70bad79

Browse files
authored
Fix owners in PDisk can write to other owner's chunks (#9869)
1 parent d9ec412 commit 70bad79

File tree

2 files changed

+55
-2
lines changed

2 files changed

+55
-2
lines changed

ydb/core/blobstorage/pdisk/blobstorage_pdisk_impl.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2892,7 +2892,16 @@ bool TPDisk::PreprocessRequest(TRequestBase *request) {
28922892
}
28932893
}
28942894
TChunkState &state = ChunkState[ev.ChunkIdx];
2895-
if (state.OwnerId == OwnerSystem) {
2895+
if (state.OwnerId != ev.Owner) {
2896+
err << "Can't write chunkIdx# " << ev.ChunkIdx
2897+
<< " chunk is owner by another owner."
2898+
<< " chunk's owner# " << state.OwnerId
2899+
<< " request's owner# " << ev.Owner;
2900+
SendChunkWriteError(ev, err.Str(), NKikimrProto::ERROR);
2901+
delete request;
2902+
return false;
2903+
}
2904+
if (!IsOwnerUser(state.OwnerId)) {
28962905
err << "Can't write chunkIdx# " << ev.ChunkIdx
28972906
<< " destination chunk is owned by the system! ownerId# " << ev.Owner;
28982907
SendChunkWriteError(ev, err.Str(), NKikimrProto::ERROR);

ydb/core/blobstorage/pdisk/blobstorage_pdisk_ut.cpp

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -926,6 +926,50 @@ Y_UNIT_TEST_SUITE(TPDiskTest) {
926926
UNIT_ASSERT_VALUES_EQUAL(writeLog(), NKikimrProto::OK);
927927
}
928928

929+
Y_UNIT_TEST(TestChunkWriteCrossOwner) {
930+
TActorTestContext testCtx({ false });
931+
932+
TVDiskMock vdisk1(&testCtx);
933+
TVDiskMock vdisk2(&testCtx);
934+
935+
vdisk1.InitFull();
936+
vdisk2.InitFull();
937+
938+
vdisk1.ReserveChunk();
939+
vdisk2.ReserveChunk();
940+
941+
vdisk1.CommitReservedChunks();
942+
vdisk2.CommitReservedChunks();
943+
944+
UNIT_ASSERT(vdisk1.Chunks[EChunkState::COMMITTED].size() == 1);
945+
UNIT_ASSERT(vdisk2.Chunks[EChunkState::COMMITTED].size() == 1);
946+
947+
auto chunk1 = *vdisk1.Chunks[EChunkState::COMMITTED].begin();
948+
auto chunk2 = *vdisk2.Chunks[EChunkState::COMMITTED].begin();
949+
950+
TString data(123, '0');
951+
auto parts = MakeIntrusive<NPDisk::TEvChunkWrite::TStrokaBackedUpParts>(data);
952+
953+
// write to own chunk is OK
954+
testCtx.TestResponse<NPDisk::TEvChunkWriteResult>(new NPDisk::TEvChunkWrite(
955+
vdisk1.PDiskParams->Owner, vdisk1.PDiskParams->OwnerRound,
956+
chunk1, 0, parts, nullptr, false, 0),
957+
NKikimrProto::OK);
958+
testCtx.TestResponse<NPDisk::TEvChunkWriteResult>(new NPDisk::TEvChunkWrite(
959+
vdisk2.PDiskParams->Owner, vdisk2.PDiskParams->OwnerRound,
960+
chunk2, 0, parts, nullptr, false, 0),
961+
NKikimrProto::OK);
962+
963+
// write to neighbour's chunk is ERROR
964+
testCtx.TestResponse<NPDisk::TEvChunkWriteResult>(new NPDisk::TEvChunkWrite(
965+
vdisk1.PDiskParams->Owner, vdisk1.PDiskParams->OwnerRound,
966+
chunk2, 0, parts, nullptr, false, 0),
967+
NKikimrProto::ERROR);
968+
testCtx.TestResponse<NPDisk::TEvChunkWriteResult>(new NPDisk::TEvChunkWrite(
969+
vdisk2.PDiskParams->Owner, vdisk2.PDiskParams->OwnerRound,
970+
chunk1, 0, parts, nullptr, false, 0),
971+
NKikimrProto::ERROR);
972+
}
929973
}
930974

931975
Y_UNIT_TEST_SUITE(PDiskCompatibilityInfo) {
@@ -961,7 +1005,7 @@ Y_UNIT_TEST_SUITE(PDiskCompatibilityInfo) {
9611005

9621006
void TestMajorVerionMigration(TCurrent oldInfo, TCurrent intermediateInfo, TCurrent newInfo) {
9631007
TCompatibilityInfoTest::Reset(&oldInfo);
964-
1008+
9651009
TActorTestContext testCtx({
9661010
.IsBad = false,
9671011
.SuppressCompatibilityCheck = false,

0 commit comments

Comments
 (0)