Skip to content

Commit

Permalink
NBSNEBIUS-351: Erase devices using deallocate in 1GB chunks (#1717)
Browse files Browse the repository at this point in the history
* NBSNEBIUS-351: Erase devices using deallocate in 1GB chunks

Deallocating large device chunks (e.g., 93GB) on certain SSD models resulted in
incomplete erasure, leaving ~20 pages uncleared. This commit mitigates the issue
by deallocating in smaller 1GB chunks, ensuring complete device erasure.
  • Loading branch information
budevg committed Aug 12, 2024
1 parent 092879d commit a913d27
Show file tree
Hide file tree
Showing 7 changed files with 138 additions and 19 deletions.
1 change: 0 additions & 1 deletion cloud/blockstore/libs/nvme/nvme.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,5 @@ struct INvmeManager
////////////////////////////////////////////////////////////////////////////////

INvmeManagerPtr CreateNvmeManager(TDuration timeout);
INvmeManagerPtr CreateNvmeManagerStub(bool isDeviceSsd = true);

} // namespace NCloud::NBlockStore::NNvme
20 changes: 14 additions & 6 deletions cloud/blockstore/libs/nvme/nvme_stub.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#include "nvme.h"
#include "nvme_stub.h"

namespace NCloud::NBlockStore::NNvme {

Expand All @@ -13,10 +13,14 @@ class TNvmeManagerStub final
{
private:
bool IsDeviceSsd;
TNvmeDeallocateHistoryPtr DeallocateHistory;

public:
TNvmeManagerStub(bool isDeviceSsd)
TNvmeManagerStub(
bool isDeviceSsd,
TNvmeDeallocateHistoryPtr deallocateHistory)
: IsDeviceSsd(isDeviceSsd)
, DeallocateHistory(std::move(deallocateHistory))
{}

TFuture<NProto::TError> Format(
Expand All @@ -35,8 +39,10 @@ class TNvmeManagerStub final
ui64 sizeBytes) override
{
Y_UNUSED(path);
Y_UNUSED(offsetBytes);
Y_UNUSED(sizeBytes);

if (DeallocateHistory) {
DeallocateHistory->emplace_back(offsetBytes, sizeBytes);
}

return MakeFuture<NProto::TError>();
}
Expand All @@ -60,9 +66,11 @@ class TNvmeManagerStub final

////////////////////////////////////////////////////////////////////////////////

INvmeManagerPtr CreateNvmeManagerStub(bool isDeviceSsd)
INvmeManagerPtr CreateNvmeManagerStub(
bool isDeviceSsd,
TNvmeDeallocateHistoryPtr deallocateHistory)
{
return std::make_shared<TNvmeManagerStub>(isDeviceSsd);
return std::make_shared<TNvmeManagerStub>(isDeviceSsd, std::move(deallocateHistory));
}

} // namespace NCloud::NBlockStore::NNvme
27 changes: 27 additions & 0 deletions cloud/blockstore/libs/nvme/nvme_stub.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#pragma once

#include "nvme.h"

namespace NCloud::NBlockStore::NNvme {

////////////////////////////////////////////////////////////////////////////////

struct TDeallocateReq
{
ui64 Offset;
ui64 Size;

bool operator==(const TDeallocateReq& other) const
{
return Offset == other.Offset && Size == other.Size;
}
};

using TNvmeDeallocateHistory = TVector<TDeallocateReq>;
using TNvmeDeallocateHistoryPtr = std::shared_ptr<TNvmeDeallocateHistory>;

INvmeManagerPtr CreateNvmeManagerStub(
bool isDeviceSsd = true,
TNvmeDeallocateHistoryPtr deallocateHistory = nullptr);

} // namespace NCloud::NBlockStore::NNvme
41 changes: 32 additions & 9 deletions cloud/blockstore/libs/service_local/storage_aio.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#include <util/generic/array_ref.h>
#include <util/generic/noncopyable.h>
#include <util/generic/size_literals.h>
#include <util/random/fast.h>
#include <util/string/builder.h>
#include <util/system/align.h>
Expand Down Expand Up @@ -739,11 +740,13 @@ class TSafeDeallocator
: TFileIOCompletion
{
private:
const ui64 ValidatedBlocksRatio = 1000; // 0.1% of blocks in device
static constexpr ui64 ValidatedBlocksRatio = 1000; // 0.1% of blocks in device
static constexpr ui64 MaxDeallocateChunkSize = 1_GB;
std::shared_ptr<TStorageContext> StorageContext;
TPromise<NProto::TError> Response;
ui64 RemainingBlockCount;
ui64 ValidateRemainingBlockCount;
ui64 ValidatedBlockIndex = 0;
ui64 DeallocateNextBlockIndex = 0;
TAlignedBuffer Buffer;
TAlignedBuffer ZeroBuffer;
TAlignedBuffer FFBuffer;
Expand All @@ -757,8 +760,9 @@ class TSafeDeallocator
: TFileIOCompletion{.Func = &TSafeDeallocator::ReadBlockCompleteCb}
, StorageContext(std::move(storageContext))
, Response(std::move(response))
, RemainingBlockCount(
, ValidateRemainingBlockCount(
StorageContext->StorageBlockCount / ValidatedBlocksRatio)
, DeallocateNextBlockIndex(StorageContext->StorageStartIndex)
, Buffer(AllocateZero(StorageContext->BlockSize))
, ZeroBuffer(AllocateZero(StorageContext->BlockSize))
, FFBuffer(AllocateZero(StorageContext->BlockSize))
Expand All @@ -770,10 +774,30 @@ class TSafeDeallocator

void Deallocate()
{
DeallocateNextChunk();
}

private:
void DeallocateNextChunk()
{
const auto remainingBlockCount = StorageContext->StorageStartIndex +
StorageContext->StorageBlockCount -
DeallocateNextBlockIndex;
if (remainingBlockCount == 0) {
ValidateNextBlock();
return;
}

const auto deallocateBlockCount = std::min(
remainingBlockCount,
MaxDeallocateChunkSize / StorageContext->BlockSize);

auto future = StorageContext->NvmeManager->Deallocate(
StorageContext->File.GetName(),
StorageContext->StorageStartIndex * StorageContext->BlockSize,
StorageContext->StorageBlockCount * StorageContext->BlockSize);
DeallocateNextBlockIndex * StorageContext->BlockSize,
deallocateBlockCount * StorageContext->BlockSize);

DeallocateNextBlockIndex += deallocateBlockCount;

future.Subscribe([this] (auto& future) {
const auto& error = future.GetValue();
Expand All @@ -782,19 +806,18 @@ class TSafeDeallocator
return;
}

ValidateNextBlock();
DeallocateNextChunk();
});
}

private:
void ValidateNextBlock()
{
if (RemainingBlockCount == 0) {
if (ValidateRemainingBlockCount == 0) {
Response.SetValue(NProto::TError());
return;
}

RemainingBlockCount--;
ValidateRemainingBlockCount--;

ValidatedBlockIndex = StorageContext->StorageStartIndex +
Rand.Uniform(StorageContext->StorageBlockCount);
Expand Down
64 changes: 63 additions & 1 deletion cloud/blockstore/libs/service_local/storage_aio_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#include <cloud/blockstore/libs/service/context.h>
#include <cloud/blockstore/libs/service/storage.h>
#include <cloud/blockstore/libs/service/storage_provider.h>
#include <cloud/blockstore/libs/nvme/nvme.h>
#include <cloud/blockstore/libs/nvme/nvme_stub.h>

#include <cloud/storage/core/libs/aio/service.h>
#include <cloud/storage/core/libs/common/error.h>
Expand Down Expand Up @@ -652,6 +652,68 @@ Y_UNIT_TEST_SUITE(TAioStorageTest)

validatePattern(0x0);
}

Y_UNIT_TEST(ShouldDeallocate1GBChunkInEraseDeviceUsingDeallocate)
{
const ui32 blockSize = 4_KB;
const ui64 blockCount = 3_GB / blockSize + 10;
const auto filePath = TryGetRamDrivePath() / "test";

TFile fileData(filePath, EOpenModeFlag::CreateAlways);
fileData.Resize(blockSize * blockCount);

auto service = CreateAIOService();
service->Start();
Y_DEFER { service->Stop(); };

auto deallocateHistory = std::make_shared<TNvmeDeallocateHistory>();
auto provider = CreateAioStorageProvider(
service,
CreateNvmeManagerStub(true, deallocateHistory),
true, // directIO
EAioSubmitQueueOpt::DontUse);

NProto::TVolume volume;
volume.SetDiskId(filePath);
volume.SetBlockSize(blockSize);
volume.SetBlocksCount(blockCount);

auto future = provider->CreateStorage(
volume,
"",
NProto::VOLUME_ACCESS_READ_WRITE);
auto storage = future.GetValue();

auto fillWithPattern = [&] (char p) {
fileData.Seek(0, sSet);
TVector<char> buffer(blockSize, p);
for (ui32 i = 0; i != blockCount; ++i) {
fileData.Write(buffer.data(), blockSize);
}

fileData.Flush();
};


// disk filled with 0x0 validates ok
fillWithPattern(0x0);
auto response =
storage->EraseDevice(NProto::DEVICE_ERASE_METHOD_DEALLOCATE);
UNIT_ASSERT_NO_EXCEPTION(response.Wait(WaitTimeout));
UNIT_ASSERT_SUCCEEDED(response.GetValue());

UNIT_ASSERT_EQUAL(4, deallocateHistory->size());
UNIT_ASSERT_EQUAL(TDeallocateReq(0, 1_GB), deallocateHistory->at(0));
UNIT_ASSERT_EQUAL(
TDeallocateReq(1_GB, 1_GB),
deallocateHistory->at(1));
UNIT_ASSERT_EQUAL(
TDeallocateReq(2_GB, 1_GB),
deallocateHistory->at(2));
UNIT_ASSERT_EQUAL(
TDeallocateReq(3_GB, 10 * 4_KB),
deallocateHistory->at(3));
}
}

} // namespace NCloud::NBlockStore::NServer
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#include <cloud/blockstore/libs/service/context.h>
#include <cloud/blockstore/libs/service/storage.h>
#include <cloud/blockstore/libs/service/storage_provider.h>
#include <cloud/blockstore/libs/nvme/nvme.h>
#include <cloud/blockstore/libs/nvme/nvme_stub.h>

#include <cloud/storage/core/libs/aio/service.h>
#include <cloud/storage/core/libs/common/error.h>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

#include <cloud/blockstore/libs/service/public.h>
#include <cloud/blockstore/libs/service/storage.h>
#include <cloud/blockstore/libs/nvme/nvme.h>
#include <cloud/blockstore/libs/nvme/nvme_stub.h>
#include <cloud/blockstore/libs/storage/core/config.h>
#include <cloud/blockstore/libs/storage/disk_agent/model/config.h>
#include <cloud/blockstore/libs/storage/disk_agent/disk_agent.h>
Expand Down

0 comments on commit a913d27

Please sign in to comment.