Skip to content

Commit da5623c

Browse files
alexvrualexvru
authored
Harden huge blob placement checks in VDisk, fix HugeKeeper WAL (#10034)
1 parent ce8c6eb commit da5623c

10 files changed

+204
-60
lines changed

‎ydb/core/blobstorage/vdisk/huge/blobstorage_hullhuge.cpp

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -693,8 +693,8 @@ LWTRACE_USING(BLOBSTORAGE_PROVIDER);
693693
NHuge::THugeSlot hugeSlot;
694694
ui32 slotSize;
695695
if (State.Pers->Heap->Allocate(msg.Data.GetSize(), &hugeSlot, &slotSize)) {
696-
const bool inserted = State.Pers->AllocatedSlots.insert(hugeSlot).second;
697-
Y_ABORT_UNLESS(inserted);
696+
State.Pers->AddSlotInFlight(hugeSlot);
697+
State.Pers->AddChunkSize(hugeSlot);
698698
const ui64 lsnInfimum = HugeKeeperCtx->LsnMngr->GetLsn();
699699
CheckLsn(lsnInfimum, "WriteHugeBlob");
700700
const ui64 wId = State.LsnFifo.Push(lsnInfimum);
@@ -889,6 +889,7 @@ LWTRACE_USING(BLOBSTORAGE_PROVIDER);
889889
for (const auto &x : msg->HugeBlobs) {
890890
slotSizes.insert(State.Pers->Heap->SlotSizeOfThisSize(x.Size));
891891
NHuge::TFreeRes freeRes = State.Pers->Heap->Free(x);
892+
State.Pers->DeleteChunkSize(State.Pers->Heap->ConvertDiskPartToHugeSlot(x));
892893
LOG_DEBUG(ctx, BS_HULLHUGE,
893894
VDISKP(HugeKeeperCtx->VCtx->VDiskLogPrefix,
894895
"THullHugeKeeper: TEvHullFreeHugeSlots: one slot: addr# %s",
@@ -961,8 +962,8 @@ LWTRACE_USING(BLOBSTORAGE_PROVIDER);
961962
// manage allocated slots
962963
const TDiskPart &hugeBlob = msg->HugeBlob;
963964
NHuge::THugeSlot hugeSlot(State.Pers->Heap->ConvertDiskPartToHugeSlot(hugeBlob));
964-
auto nErased = State.Pers->AllocatedSlots.erase(hugeSlot);
965-
Y_ABORT_UNLESS(nErased == 1);
965+
const bool deleted = State.Pers->DeleteSlotInFlight(hugeSlot);
966+
Y_ABORT_UNLESS(deleted);
966967
// depending on SlotIsUsed...
967968
if (msg->SlotIsUsed) {
968969
Y_VERIFY_S(State.Pers->LogPos.HugeBlobLoggedLsn < msg->RecLsn,
@@ -972,6 +973,8 @@ LWTRACE_USING(BLOBSTORAGE_PROVIDER);
972973
} else {
973974
// ...free slot
974975
State.Pers->Heap->Free(hugeBlob);
976+
// and remove chunk size record
977+
State.Pers->DeleteChunkSize(hugeSlot);
975978
}
976979
}
977980

@@ -1090,8 +1093,7 @@ LWTRACE_USING(BLOBSTORAGE_PROVIDER);
10901093
: HugeKeeperCtx(std::move(hugeKeeperCtx))
10911094
, State(std::move(persState))
10921095
{
1093-
Y_ABORT_UNLESS(State.Pers->Recovered &&
1094-
State.Pers->AllocatedSlots.empty());
1096+
Y_ABORT_UNLESS(State.Pers->Recovered && State.Pers->SlotsInFlight.empty());
10951097
}
10961098

10971099
void Bootstrap(const TActorContext &ctx) {

‎ydb/core/blobstorage/vdisk/huge/blobstorage_hullhuge_ut.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,6 @@ namespace NKikimr {
3434
overhead, freeChunksReservation, logf));
3535

3636
state->LogPos = THullHugeRecoveryLogPos(0, 0, 100500, 50000, 70000, 56789, 39482);
37-
NHuge::THugeSlot hugeSlot(453, 0, 234);
38-
state->AllocatedSlots.insert(hugeSlot);
3937

4038
TString serialized(state->Serialize());
4139
UNIT_ASSERT(THullHugeKeeperPersState::CheckEntryPoint(serialized));

‎ydb/core/blobstorage/vdisk/huge/blobstorage_hullhugedefs.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,14 @@ namespace NKikimr {
2323
ui32 ChunkId = 0;
2424
TMask Mask;
2525
ui32 MaskSize = 0;
26+
bool InLockedChunks = false;
2627

2728
TFreeRes() = default;
28-
TFreeRes(ui32 chunkId, TMask mask, ui32 maskSize)
29+
TFreeRes(ui32 chunkId, TMask mask, ui32 maskSize, bool inLockedChunks)
2930
: ChunkId(chunkId)
3031
, Mask(mask)
3132
, MaskSize(maskSize)
33+
, InLockedChunks(inLockedChunks)
3234
{}
3335

3436
void Output(IOutputStream &str) const;

‎ydb/core/blobstorage/vdisk/huge/blobstorage_hullhugeheap.cpp

Lines changed: 46 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ namespace NKikimr {
138138
Y_VERIFY_S(chunkId, VDiskLogPrefix << "chunkId# " << chunkId);
139139
TFreeSpace::iterator it;
140140

141-
auto freeFoundSlot = [&] (TFreeSpace &container, const char *containerName) {
141+
auto freeFoundSlot = [&] (TFreeSpace &container, const char *containerName, bool inLockedChunks) {
142142
TMask &mask = it->second;
143143
Y_VERIFY_S(!mask.Get(slotId), VDiskLogPrefix << "TChain::Free: containerName# " << containerName
144144
<< " id# " << id.ToString() << " State# " << ToString());
@@ -148,15 +148,15 @@ namespace NKikimr {
148148
// free chunk
149149
container.erase(it);
150150
FreeSlotsInFreeSpace -= SlotsInChunk;
151-
return TFreeRes(chunkId, ConstMask, SlotsInChunk);
151+
return TFreeRes(chunkId, ConstMask, SlotsInChunk, inLockedChunks);
152152
} else
153-
return TFreeRes(0, mask, SlotsInChunk);
153+
return TFreeRes(0, mask, SlotsInChunk, false);
154154
};
155155

156156
if ((it = FreeSpace.find(chunkId)) != FreeSpace.end()) {
157-
return freeFoundSlot(FreeSpace, "FreeSpace");
157+
return freeFoundSlot(FreeSpace, "FreeSpace", false);
158158
} else if ((it = LockedChunks.find(chunkId)) != LockedChunks.end()) {
159-
return freeFoundSlot(LockedChunks, "LockedChunks");
159+
return freeFoundSlot(LockedChunks, "LockedChunks", true);
160160
} else {
161161
// chunk is neither in FreeSpace nor in LockedChunks
162162
TDynBitMap mask;
@@ -166,13 +166,13 @@ namespace NKikimr {
166166
++FreeSlotsInFreeSpace;
167167

168168
FreeSpace.emplace(chunkId, mask);
169-
return TFreeRes(0, mask, SlotsInChunk); // no empty chunk
169+
return TFreeRes(0, mask, SlotsInChunk, false); // no empty chunk
170170
}
171171
}
172172

173173
bool TChain::LockChunkForAllocation(TChunkID chunkId) {
174-
if (TFreeSpace::iterator it = FreeSpace.find(chunkId); it != FreeSpace.end()) {
175-
LockedChunks.insert(FreeSpace.extract(it));
174+
if (auto nh = FreeSpace.extract(chunkId)) {
175+
LockedChunks.insert(std::move(nh));
176176
return true;
177177
} else {
178178
// chunk is already freed
@@ -181,8 +181,8 @@ namespace NKikimr {
181181
}
182182

183183
void TChain::UnlockChunk(TChunkID chunkId) {
184-
if (auto it = LockedChunks.find(chunkId); it != LockedChunks.end()) {
185-
FreeSpace.insert(LockedChunks.extract(it));
184+
if (auto nh = LockedChunks.extract(chunkId)) {
185+
FreeSpace.insert(std::move(nh));
186186
}
187187
}
188188

@@ -211,15 +211,20 @@ namespace NKikimr {
211211
ui32 chunkId = id.GetChunkId();
212212
ui32 slotId = id.GetSlotId();
213213

214-
TFreeSpace::iterator it = FreeSpace.find(chunkId);
215-
if (it != FreeSpace.end()) {
214+
TFreeSpace *map = &FreeSpace;
215+
TFreeSpace::iterator it = map->find(chunkId);
216+
if (it == map->end()) {
217+
map = &LockedChunks;
218+
it = map->find(chunkId);
219+
}
220+
if (it != map->end()) {
216221
TMask &mask = it->second;
217222
Y_VERIFY_S(mask.Get(slotId), VDiskLogPrefix << "RecoveryModeAllocate:"
218223
<< " id# " << id.ToString() << " State# " << ToString());
219224
mask.Reset(slotId);
220225

221226
if (mask.Empty()) {
222-
FreeSpace.erase(it);
227+
map->erase(it);
223228
}
224229

225230
--FreeSlotsInFreeSpace;
@@ -230,11 +235,11 @@ namespace NKikimr {
230235
}
231236
}
232237

233-
void TChain::RecoveryModeAllocate(const NPrivate::TChunkSlot &id, TChunkID chunkId) {
234-
Y_VERIFY_S(id.GetChunkId() == chunkId && FreeSpace.find(chunkId) == FreeSpace.end(),
238+
void TChain::RecoveryModeAllocate(const NPrivate::TChunkSlot &id, TChunkID chunkId, bool inLockedChunks) {
239+
Y_VERIFY_S(id.GetChunkId() == chunkId && !FreeSpace.contains(chunkId) && !LockedChunks.contains(chunkId),
235240
VDiskLogPrefix << " id# " << id.ToString() << " chunkId# " << chunkId << " State# " << ToString());
236241

237-
FreeSpace.emplace(chunkId, ConstMask);
242+
(inLockedChunks ? LockedChunks : FreeSpace).emplace(chunkId, ConstMask);
238243
FreeSlotsInFreeSpace += SlotsInChunk;
239244
bool res = RecoveryModeAllocate(id);
240245

@@ -358,6 +363,10 @@ namespace NKikimr {
358363
return NPrivate::TChunkSlot(addr.ChunkIdx, slotId);
359364
}
360365

366+
NPrivate::TChunkSlot TChainDelegator::Convert(const THugeSlot &slot) const {
367+
return Convert(slot.GetDiskPart());
368+
}
369+
361370
void TChainDelegator::Save(IOutputStream *s) const {
362371
::Save(s, *ChainPtr);
363372
}
@@ -791,7 +800,7 @@ namespace NKikimr {
791800
TFreeChunks::iterator it = FreeChunks.find(chunkId);
792801
Y_VERIFY_S(it != FreeChunks.end(), VDiskLogPrefix << "addr# " << addr.ToString() << " State# " << ToString());
793802
FreeChunks.erase(it);
794-
chainD->ChainPtr->RecoveryModeAllocate(id, chunkId);
803+
chainD->ChainPtr->RecoveryModeAllocate(id, chunkId, false);
795804
}
796805
}
797806

@@ -807,6 +816,26 @@ namespace NKikimr {
807816
}
808817
}
809818

819+
bool THeap::ReleaseSlot(THugeSlot slot) {
820+
TChainDelegator* const chain = Chains.GetChain(slot.GetSize());
821+
Y_VERIFY_S(chain, VDiskLogPrefix << "State# " << ToString() << " slot# " << slot.ToString());
822+
if (TFreeRes res = chain->ChainPtr->Free(chain->Convert(slot)); res.ChunkId) {
823+
PutChunkIdToFreeChunks(res.ChunkId);
824+
return res.InLockedChunks;
825+
}
826+
return false;
827+
}
828+
829+
void THeap::OccupySlot(THugeSlot slot, bool inLockedChunks) {
830+
TChainDelegator* const chain = Chains.GetChain(slot.GetSize());
831+
Y_VERIFY_S(chain, VDiskLogPrefix << "State# " << ToString() << " slot# " << slot.ToString());
832+
if (!chain->ChainPtr->RecoveryModeAllocate(chain->Convert(slot))) {
833+
const size_t numErased = FreeChunks.erase(slot.GetChunkId());
834+
Y_VERIFY_S(numErased, VDiskLogPrefix << "State# " << ToString() << " slot# " << slot.ToString());
835+
chain->ChainPtr->RecoveryModeAllocate(chain->Convert(slot), slot.GetChunkId(), inLockedChunks);
836+
}
837+
}
838+
810839
//////////////////////////////////////////////////////////////////////////////////////////
811840
// THeap: Serialize/Parse/Check
812841
//////////////////////////////////////////////////////////////////////////////////////////

‎ydb/core/blobstorage/vdisk/huge/blobstorage_hullhugeheap.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ namespace NKikimr {
124124
THeapStat GetStat() const;
125125
// returns true is allocated, false otherwise
126126
bool RecoveryModeAllocate(const NPrivate::TChunkSlot &id);
127-
void RecoveryModeAllocate(const NPrivate::TChunkSlot &id, TChunkID chunkId);
127+
void RecoveryModeAllocate(const NPrivate::TChunkSlot &id, TChunkID chunkId, bool inLockedChunks);
128128
void Save(IOutputStream *s) const;
129129
void Load(IInputStream *s);
130130
bool HaveBeenUsed() const;
@@ -158,6 +158,7 @@ namespace NKikimr {
158158
TChainDelegator &operator =(const TChainDelegator &) = delete;
159159
THugeSlot Convert(const NPrivate::TChunkSlot &id) const;
160160
NPrivate::TChunkSlot Convert(const TDiskPart &addr) const;
161+
NPrivate::TChunkSlot Convert(const THugeSlot &slot) const;
161162
void Save(IOutputStream *s) const;
162163
void Load(IInputStream *s);
163164
bool HaveBeenUsed() const;
@@ -299,6 +300,8 @@ namespace NKikimr {
299300
void RecoveryModeAllocate(const TDiskPart &addr);
300301
void RecoveryModeAddChunk(ui32 chunkId);
301302
void RecoveryModeRemoveChunks(const TVector<ui32> &chunkIds);
303+
bool ReleaseSlot(THugeSlot slot);
304+
void OccupySlot(THugeSlot slot, bool inLockedChunks);
302305

303306
//////////////////////////////////////////////////////////////////////////////////////////
304307
// Serialize/Parse/Check

‎ydb/core/blobstorage/vdisk/huge/blobstorage_hullhugeheap_ut.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,13 @@ namespace NKikimr {
1717
TMask mask;
1818
mask.Set(0, 8);
1919
mask.Reset(1);
20-
TFreeRes res = {15, mask, 8};
20+
TFreeRes res = {15, mask, 8, false};
2121

2222
STR << "TFreeRes# " << res.ToString() << "\n";
2323
UNIT_ASSERT_EQUAL(res.ToString(), "{ChunkIdx: 15 Mask# 10111111}");
2424

2525
TMask constMask = TChain::BuildConstMask("", 8);
26-
TFreeRes constRes = {0, constMask, 8};
26+
TFreeRes constRes = {0, constMask, 8, false};
2727
STR << "constMask# " << constRes.ToString() << "\n";
2828
UNIT_ASSERT_EQUAL(constRes.ToString(), "{ChunkIdx: 0 Mask# 11111111}");
2929

0 commit comments

Comments
 (0)