Skip to content

Commit

Permalink
merge to stable-23-3 (#287)
Browse files Browse the repository at this point in the history
* ignoring TEvBootstrapper::TEvStatus in TVolumeActor::StateZombie (fixing rare crashes in tests) (#272)

* issue #196: implemented block checksum calculation and verification for the blobs stored in blobstorage (#278)

* issue #196: implemented calculation of block checksums in the blobs stored in blobstorage; implemented checksum checking for those blobs upon read; enabled this logic for the first 1GiB of the disk space in most of the uts in part_ut and in tests/loadtest/local large test suite; will add the uts that directly check checksum verification is the next PR;

* issue #196: review-related cleanup;

* issue #196: more review-related cleanup;

* issue #196: added a ut with CheckBlockChecksumsInBlobsUponRead, fixed a bug in block verification in ReadBlocks data path, enabled CheckBlockChecksumsInBlobsUponRead in tests/loadtest/local

* issue #196: added more uts

* fixed build after merge
  • Loading branch information
qkrorlqr authored Jan 30, 2024
1 parent 4b0274a commit a38627f
Show file tree
Hide file tree
Showing 25 changed files with 633 additions and 45 deletions.
15 changes: 15 additions & 0 deletions cloud/blockstore/config/storage.proto
Original file line number Diff line number Diff line change
Expand Up @@ -893,4 +893,19 @@ message TStorageServiceConfig
// Max value is selected between this limit and
// MaxNonReplicatedDeviceMigrationsInProgress.
optional uint32 MaxNonReplicatedDeviceMigrationPercentageInProgress = 343;

// Create shadow disks for checkpoints for DiskRegistry-based disks.
optional bool UseShadowDisksForNonreplDiskCheckpoints = 344;

// Calculate crc32c checksums for blocks in the half-interval [0, X) upon
// write and store these checksums in local db.
// Affects only blobstorage-based disks.
optional uint64 DiskPrefixLengthWithBlockChecksumsInBlobs = 345;

// Read BlobMeta upon ReadBlocks, calculate crc32c checksums for the fetched
// blocks and compare these checksums to the checksums from BlobMeta.
//
// IMPORTANT: Compaction verifies block checksums for the blocks affected by
// it regardless of this flag!
optional bool CheckBlockChecksumsInBlobsUponRead = 346;
}
15 changes: 14 additions & 1 deletion cloud/blockstore/libs/diagnostics/block_digest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,19 @@ namespace NCloud::NBlockStore {

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

ui32 ComputeDefaultDigest(TBlockDataRef blockContent)
{
Y_DEBUG_ABORT_UNLESS(blockContent.Data() != nullptr);

if (blockContent.Data() != nullptr) {
return Crc32c(blockContent.Data(), blockContent.Size());
}

return 0;
}

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

struct TExt4BlockDigestGenerator final
: IBlockDigestGenerator
{
Expand All @@ -38,7 +51,7 @@ struct TExt4BlockDigestGenerator final
}

if (blockContent.Data() != nullptr) {
return Crc32c(blockContent.Data(), blockContent.Size());
return ComputeDefaultDigest(blockContent);
}

double idx = log2(static_cast<double>(blockContent.Size()) / DefaultBlockSize);
Expand Down
4 changes: 4 additions & 0 deletions cloud/blockstore/libs/diagnostics/block_digest.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,8 @@ IBlockDigestGeneratorPtr CreateExt4BlockDigestGenerator(
IBlockDigestGeneratorPtr CreateTestBlockDigestGenerator();
IBlockDigestGeneratorPtr CreateBlockDigestGeneratorStub();

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

ui32 ComputeDefaultDigest(TBlockDataRef blockContent);

} // namespace NCloud::NBlockStore
5 changes: 5 additions & 0 deletions cloud/blockstore/libs/diagnostics/block_digest_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ Y_UNIT_TEST_SUITE(TBlockDigestGeneratorTest)
UNIT_ASSERT(!gen->ShouldProcess(0, 10, 4_KB));
UNIT_ASSERT(!gen->ShouldProcess(100500, 10, 4_KB));
}

Y_UNIT_TEST(TestComputeDefaultDigest)
{
UNIT_ASSERT_VALUES_EQUAL(3387363646, ComputeDefaultDigest({"vasya", 5}));
}
}

} // namespace NCloud::NBlockStore
1 change: 1 addition & 0 deletions cloud/blockstore/libs/diagnostics/critical_events.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ namespace NCloud::NBlockStore {
xxx(EndpointSwitchFailure) \
xxx(DiskAgentSessionCacheUpdateError) \
xxx(DiskAgentSessionCacheRestoreError) \
xxx(BlockDigestMismatchInBlob) \
// BLOCKSTORE_CRITICAL_EVENTS

#define BLOCKSTORE_IMPOSSIBLE_EVENTS(xxx) \
Expand Down
36 changes: 36 additions & 0 deletions cloud/blockstore/libs/storage/core/block_handler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,23 @@ class TReadBlocksHandler final
return SgList.Create(std::move(sglist));
}

TGuardedSgList GetGuardedSgList(
const TVector<ui64>& blockIndices) const override
{
TSgList sglist(Reserve(blockIndices.size()));

for (const auto blockIndex: blockIndices) {
Y_ABORT_UNLESS(ReadRange.Contains(blockIndex));

size_t index = blockIndex - ReadRange.Start;
Y_ABORT_UNLESS(index < Blocks.BuffersSize());
const auto& block = Blocks.GetBuffers(index);
sglist.emplace_back(block.data(), BlockSize);
}

return SgList.Create(std::move(sglist));
}

bool SetBlock(
ui64 blockIndex,
TBlockDataRef blockContent,
Expand Down Expand Up @@ -284,6 +301,25 @@ class TReadBlocksLocalHandler final
return GuardedSgList;
}

TGuardedSgList GetGuardedSgList(
const TVector<ui64>& blockIndices) const override
{
if (auto guard = GuardedSgList.Acquire()) {
const auto& src = guard.Get();
TSgList subset(Reserve(blockIndices.size()));

for (auto blockIndex: blockIndices) {
Y_ABORT_UNLESS(ReadRange.Contains(blockIndex));
const auto index = blockIndex - ReadRange.Start;
subset.push_back(src[index]);
}

return GuardedSgList.Create(std::move(subset));
}

return GuardedSgList;
}

bool SetBlock(
ui64 blockIndex,
TBlockDataRef blockContent,
Expand Down
6 changes: 6 additions & 0 deletions cloud/blockstore/libs/storage/core/block_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,16 @@ struct IReadBlocksHandler
{
virtual ~IReadBlocksHandler() = default;

// TODO: split into multiple funcs:
// * void SetUnencryptedBlockMaskBits(const TVector<ui64>& blockIndices, bool value)
// * TGuardedSgList InitializeBuffers(const TVector<ui64>& blockIndices)
virtual TGuardedSgList GetGuardedSgList(
const TVector<ui64>& blockIndices,
bool baseDisk) = 0;

virtual TGuardedSgList GetGuardedSgList(
const TVector<ui64>& blockIndices) const = 0;

virtual bool SetBlock(
ui64 blockIndex,
TBlockDataRef blockContent,
Expand Down
5 changes: 5 additions & 0 deletions cloud/blockstore/libs/storage/core/config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,11 @@ TDuration MSeconds(ui32 value)
xxx(AssignIdToWriteAndZeroRequestsEnabled, bool, false )\
\
xxx(AgentListExpiredParamsCleanupInterval, TDuration, Seconds(1) )\
\
xxx(UseShadowDisksForNonreplDiskCheckpoints, bool, false )\
\
xxx(DiskPrefixLengthWithBlockChecksumsInBlobs, ui64, 0 )\
xxx(CheckBlockChecksumsInBlobsUponRead, bool, false )\
// BLOCKSTORE_STORAGE_CONFIG_RW

#define BLOCKSTORE_STORAGE_CONFIG(xxx) \
Expand Down
5 changes: 5 additions & 0 deletions cloud/blockstore/libs/storage/core/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,11 @@ class TStorageConfig
TDuration GetAgentListExpiredParamsCleanupInterval() const;

ui64 GetTenantHiveTabletId() const;

bool GetUseShadowDisksForNonreplDiskCheckpoints() const;

ui64 GetDiskPrefixLengthWithBlockChecksumsInBlobs() const;
bool GetCheckBlockChecksumsInBlobsUponRead() const;
};

ui64 GetAllocationUnit(
Expand Down
37 changes: 37 additions & 0 deletions cloud/blockstore/libs/storage/partition/part_actor.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "part_actor.h"

#include <cloud/blockstore/libs/diagnostics/block_digest.h>
#include <cloud/blockstore/libs/diagnostics/critical_events.h>
#include <cloud/blockstore/libs/storage/api/volume_proxy.h>
#include <cloud/storage/core/libs/api/hive_proxy.h>
Expand Down Expand Up @@ -949,4 +950,40 @@ void SetCounters(
counters.SetBlocksCount(blocksCount);
}

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

NProto::TError VerifyBlockChecksum(
const TBlockDataRef& block,
const NKikimr::TLogoBlobID& blobID,
const ui64 blockIndex,
const ui16 blobOffset,
const ui32 expectedChecksum)
{
if (expectedChecksum == 0) {
// 0 is a special case - block digest calculation can be
// switched on and off => some blobs may not have digests for
// some blocks
//
// we should just skip validation for this block

return {};
}

const auto actualChecksum = ComputeDefaultDigest(block);

if (actualChecksum != expectedChecksum) {
ReportBlockDigestMismatchInBlob();
// we might read proper data upon retry - let's give it a chance
return MakeError(
E_REJECTED,
TStringBuilder() << "block digest mismatch, block="
<< blockIndex << ", offset=" << blobOffset
<< ", blobId=" << blobID
<< ", expected=" << expectedChecksum
<< ", actual=" << actualChecksum);
}

return {};
}

} // namespace NCloud::NBlockStore::NStorage::NPartition
10 changes: 10 additions & 0 deletions cloud/blockstore/libs/storage/partition/part_actor.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <cloud/blockstore/libs/storage/partition_common/long_running_operation_companion.h>

#include <cloud/storage/core/libs/api/hive_proxy.h>
#include <cloud/storage/core/libs/tablet/blob_id.h>
#include <cloud/storage/core/libs/tablet/model/commit.h>

#include <library/cpp/actors/core/actor.h>
Expand Down Expand Up @@ -644,4 +645,13 @@ void SetCounters(
const TDuration waitTime,
ui64 blocksCount);

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

NProto::TError VerifyBlockChecksum(
const TBlockDataRef& block,
const NKikimr::TLogoBlobID& blobID,
const ui64 blockIndex,
const ui16 blobOffset,
const ui32 expectedChecksum);

} // namespace NCloud::NBlockStore::NStorage::NPartition
12 changes: 12 additions & 0 deletions cloud/blockstore/libs/storage/partition/part_actor_addblobs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,10 @@ class TAddBlobsExecutor
mixedBlocks.AddBlocks(blockIndex);
}

for (ui32 checksum: blob.Checksums) {
blobMeta.AddBlockChecksums(checksum);
}

db.WriteBlobMeta(blob.BlobId, blobMeta);

if (!IsDeletionMarker(blob.BlobId)) {
Expand Down Expand Up @@ -226,6 +230,10 @@ class TAddBlobsExecutor
mergedBlocks.SetEnd(blob.BlockRange.End);
mergedBlocks.SetSkipped(skipped);

for (ui32 checksum: blob.Checksums) {
blobMeta.AddBlockChecksums(checksum);
}

db.WriteBlobMeta(blob.BlobId, blobMeta);

if (!IsDeletionMarker(blob.BlobId)) {
Expand Down Expand Up @@ -289,6 +297,10 @@ class TAddBlobsExecutor
mixedBlocks.AddCommitIds(block.CommitId);
}

for (ui32 checksum: blob.Checksums) {
blobMeta.AddBlockChecksums(checksum);
}

db.WriteBlobMeta(blob.BlobId, blobMeta);

if (!IsDeletionMarker(blob.BlobId)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,8 @@ void TPartitionActor::HandleAddConfirmedBlobs(
mergedBlobs.emplace_back(
MakePartialBlobId(commitId, blob.UniqueId),
blob.BlockRange,
TBlockMask());
TBlockMask(), // skipMask
TVector<ui32>() /* TODO: checksums */);
}

auto request = std::make_unique<TRequest>(
Expand Down
Loading

0 comments on commit a38627f

Please sign in to comment.