Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Checksum write buffer before and after write #2985

Merged
merged 7 commits into from
Feb 11, 2025
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions cloud/blockstore/config/server.proto
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,16 @@ message TLocation

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

message TChecksumFlags
{
// If enabled, the checksum for data buffer calculated before and after
// writing. This is done to ensure that the same data is written to all
// mirror disk replicas.
optional bool CheckBufferModificationForMirrorDisk = 1;
drbasic marked this conversation as resolved.
Show resolved Hide resolved
}

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

message TServerConfig
{
// Host name or address to listen on.
Expand Down Expand Up @@ -204,6 +214,9 @@ message TServerConfig
// How many seconds will the external vhost server continue to work after
// the parent process die.
optional uint32 VhostServerTimeoutAfterParentExit = 118;

// Flags for managing checksums.
optional TChecksumFlags ChecksumFlags = 119;
}

////////////////////////////////////////////////////////////////////////////////
Expand Down
6 changes: 4 additions & 2 deletions cloud/blockstore/libs/daemon/common/bootstrap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,8 @@ void TBootstrapBase::Init()
auto nbdEndpointListener = CreateNbdEndpointListener(
NbdServer,
Logging,
ServerStats);
ServerStats,
Configs->ServerConfig->GetChecksumFlags());

endpointListeners.emplace(
NProto::IPC_NBD,
Expand All @@ -413,7 +414,8 @@ void TBootstrapBase::Init()
STORAGE_INFO("VHOST Server initialized");

auto vhostEndpointListener = CreateVhostEndpointListener(
VhostServer);
VhostServer,
Configs->ServerConfig->GetChecksumFlags());

if (Configs->ServerConfig->GetVhostServerPath()
&& !Configs->Options->TemporaryServer)
Expand Down
3 changes: 2 additions & 1 deletion cloud/blockstore/libs/diagnostics/critical_events.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ namespace NCloud::NBlockStore {
xxx(MirroredDiskDeviceReplacementRateLimitExceeded) \
xxx(MirroredDiskMinorityChecksumMismatch) \
xxx(MirroredDiskMajorityChecksumMismatch) \
xxx(MirroredDiskChecksumMismatchUponRead) \
xxx(MirroredDiskChecksumMismatchUponWrite) \
xxx(CounterUpdateRace) \
xxx(EndpointStartingError) \
xxx(ResyncFailed) \
Expand Down Expand Up @@ -64,7 +66,6 @@ namespace NCloud::NBlockStore {
xxx(DiskRegistryPurgeHostError) \
xxx(DiskRegistryCleanupAgentConfigError) \
xxx(DiskRegistryOccupiedDeviceConfigurationHasChanged) \
xxx(MirroredDiskChecksumMismatchUponRead) \
xxx(DiskRegistryWrongMigratedDeviceOwnership) \
xxx(DiskRegistryInitialAgentRejectionThresholdExceeded) \
// BLOCKSTORE_CRITICAL_EVENTS
Expand Down
15 changes: 12 additions & 3 deletions cloud/blockstore/libs/endpoints_nbd/nbd_server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <cloud/blockstore/libs/nbd/server.h>
#include <cloud/blockstore/libs/nbd/server_handler.h>
#include <cloud/blockstore/libs/service/device_handler.h>
#include <cloud/storage/core/libs/common/media.h>
#include <cloud/storage/core/libs/diagnostics/logging.h>

namespace NCloud::NBlockStore::NServer {
Expand All @@ -25,15 +26,18 @@ class TNbdEndpointListener final
const NBD::IServerPtr Server;
const ILoggingServicePtr Logging;
const IServerStatsPtr ServerStats;
const NProto::TChecksumFlags ChecksumFlags;

public:
TNbdEndpointListener(
NBD::IServerPtr server,
ILoggingServicePtr logging,
IServerStatsPtr serverStats)
IServerStatsPtr serverStats,
NProto::TChecksumFlags checksumFlags)
: Server(std::move(server))
, Logging(std::move(logging))
, ServerStats(std::move(serverStats))
, ChecksumFlags(std::move(checksumFlags))
{}

TFuture<NProto::TError> StartEndpoint(
Expand All @@ -48,6 +52,9 @@ class TNbdEndpointListener final
options.BlocksCount = volume.GetBlocksCount();
options.UnalignedRequestsDisabled = request.GetUnalignedRequestsDisabled();
options.SendMinBlockSize = request.GetSendNbdMinBlockSize();
options.UnalignedRequestsDisabled =
drbasic marked this conversation as resolved.
Show resolved Hide resolved
ChecksumFlags.GetCheckBufferModificationForMirrorDisk() &&
IsReliableDiskRegistryMediaKind(volume.GetStorageMediaKind());

auto requestFactory = CreateServerHandlerFactory(
CreateDefaultDeviceHandlerFactory(),
Expand Down Expand Up @@ -109,12 +116,14 @@ class TNbdEndpointListener final
IEndpointListenerPtr CreateNbdEndpointListener(
NBD::IServerPtr server,
ILoggingServicePtr logging,
IServerStatsPtr serverStats)
IServerStatsPtr serverStats,
NProto::TChecksumFlags checksumFlags)
{
return std::make_shared<TNbdEndpointListener>(
std::move(server),
std::move(logging),
std::move(serverStats));
std::move(serverStats),
std::move(checksumFlags));
}

} // namespace NCloud::NBlockStore::NServer
4 changes: 3 additions & 1 deletion cloud/blockstore/libs/endpoints_nbd/nbd_server.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include "public.h"

#include <cloud/blockstore/config/server.pb.h>
#include <cloud/blockstore/libs/diagnostics/public.h>
#include <cloud/blockstore/libs/endpoints/public.h>
#include <cloud/blockstore/libs/nbd/public.h>
Expand All @@ -13,6 +14,7 @@ namespace NCloud::NBlockStore::NServer {
IEndpointListenerPtr CreateNbdEndpointListener(
NBD::IServerPtr server,
ILoggingServicePtr logging,
IServerStatsPtr serverStats);
IServerStatsPtr serverStats,
NProto::TChecksumFlags checksumFlags);

} // namespace NCloud::NBlockStore::NServer
16 changes: 13 additions & 3 deletions cloud/blockstore/libs/endpoints_vhost/vhost_server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <cloud/blockstore/libs/client/session.h>
#include <cloud/blockstore/libs/endpoints/endpoint_listener.h>
#include <cloud/blockstore/libs/vhost/server.h>
#include <cloud/storage/core/libs/common/media.h>

namespace NCloud::NBlockStore::NServer {

Expand All @@ -17,10 +18,14 @@ class TVhostEndpointListener final
{
private:
const NVhost::IServerPtr Server;
const NProto::TChecksumFlags ChecksumFlags;

public:
TVhostEndpointListener(NVhost::IServerPtr server)
TVhostEndpointListener(
NVhost::IServerPtr server,
NProto::TChecksumFlags checksumFlags)
drbasic marked this conversation as resolved.
Show resolved Hide resolved
: Server(std::move(server))
, ChecksumFlags(std::move(checksumFlags))
{}

TFuture<NProto::TError> StartEndpoint(
Expand All @@ -36,6 +41,9 @@ class TVhostEndpointListener final
options.BlocksCount = volume.GetBlocksCount();
options.VhostQueuesCount = request.GetVhostQueuesCount();
options.UnalignedRequestsDisabled = request.GetUnalignedRequestsDisabled();
options.CheckBufferModificationDuringWriting =
ChecksumFlags.GetCheckBufferModificationForMirrorDisk() &&
IsReliableDiskRegistryMediaKind(volume.GetStorageMediaKind());

return Server->StartEndpoint(
request.GetUnixSocketPath(),
Expand Down Expand Up @@ -83,10 +91,12 @@ class TVhostEndpointListener final
////////////////////////////////////////////////////////////////////////////////

IEndpointListenerPtr CreateVhostEndpointListener(
NVhost::IServerPtr server)
NVhost::IServerPtr server,
const NProto::TChecksumFlags& checksumFlags)
{
return std::make_shared<TVhostEndpointListener>(
std::move(server));
std::move(server),
checksumFlags);
}

} // namespace NCloud::NBlockStore::NServer
5 changes: 4 additions & 1 deletion cloud/blockstore/libs/endpoints_vhost/vhost_server.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@

#include <cloud/blockstore/libs/endpoints/public.h>
#include <cloud/blockstore/libs/vhost/public.h>
#include <cloud/blockstore/config/server.pb.h>

namespace NCloud::NBlockStore::NServer {

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

IEndpointListenerPtr CreateVhostEndpointListener(NVhost::IServerPtr server);
IEndpointListenerPtr CreateVhostEndpointListener(
NVhost::IServerPtr server,
const NProto::TChecksumFlags& checksumFlags);

} // namespace NCloud::NBlockStore::NServer
4 changes: 3 additions & 1 deletion cloud/blockstore/libs/nbd/server_handler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -951,9 +951,11 @@ IServerHandlerFactoryPtr CreateServerHandlerFactory(
{
auto deviceHandler = deviceHandlerFactory->CreateDeviceHandler(
std::move(storage),
options.DiskId,
options.ClientId,
options.BlockSize,
options.UnalignedRequestsDisabled);
options.UnalignedRequestsDisabled,
options.CheckBufferModificationDuringWriting);

return std::make_shared<TServerHandlerFactory>(
std::move(logging),
Expand Down
1 change: 1 addition & 0 deletions cloud/blockstore/libs/nbd/server_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ struct TStorageOptions
TString CheckpointId;
bool UnalignedRequestsDisabled = false;
bool SendMinBlockSize = false;
bool CheckBufferModificationDuringWriting = false;
};

////////////////////////////////////////////////////////////////////////////////
Expand Down
29 changes: 7 additions & 22 deletions cloud/blockstore/libs/server/config.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#include "config.h"

#include <cloud/storage/core/libs/common/proto_helpers.h>

#include <library/cpp/monlib/service/pages/templates.h>

#include <util/generic/size_literals.h>
Expand Down Expand Up @@ -101,6 +103,7 @@ constexpr TDuration Seconds(int s)
xxx(NodeRegistrationToken, TString, "root@builtin" )\
xxx(EndpointStorageNotImplementedErrorIsFatal, bool, false )\
xxx(VhostServerTimeoutAfterParentExit, TDuration, Seconds(60) )\
xxx(ChecksumFlags, NProto::TChecksumFlags, {} )
// BLOCKSTORE_SERVER_CONFIG

#define BLOCKSTORE_SERVER_DECLARE_CONFIG(name, type, value) \
Expand Down Expand Up @@ -151,25 +154,6 @@ TAffinity ConvertValue<TAffinity, NProto::TAffinity>(
return TAffinity(std::move(vec));
}

template <typename T>
bool IsEmpty(const T& t)
{
return !t;
}

template <typename T>
bool IsEmpty(
const google::protobuf::RepeatedPtrField<T>& value)
{
return value.empty();
}

template <>
bool IsEmpty(const NProto::TAffinity& value)
{
return value.GetCPU().empty();
}

template <typename T>
void DumpImpl(const T& t, IOutputStream& os)
{
Expand Down Expand Up @@ -264,9 +248,10 @@ TServerAppConfig::TServerAppConfig(NProto::TServerAppConfig appConfig)
#define BLOCKSTORE_CONFIG_GETTER(name, type, ...) \
type TServerAppConfig::Get##name() const \
{ \
const auto value = ServerConfig->Get##name(); \
return !IsEmpty(value) ? ConvertValue<type>(value) : Default##name; \
} \
return NCloud::HasField(*ServerConfig, #name) \
? ConvertValue<type>(ServerConfig->Get##name()) \
: Default##name; \
}
// BLOCKSTORE_CONFIG_GETTER

BLOCKSTORE_SERVER_CONFIG(BLOCKSTORE_CONFIG_GETTER)
Expand Down
1 change: 1 addition & 0 deletions cloud/blockstore/libs/server/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ class TServerAppConfig
bool GetEndpointStorageNotImplementedErrorIsFatal() const;
TDuration GetVhostServerTimeoutAfterParentExit() const;
TString GetNodeRegistrationToken() const;
NProto::TChecksumFlags GetChecksumFlags() const;

void Dump(IOutputStream& out) const override;
void DumpHtml(IOutputStream& out) const override;
Expand Down
11 changes: 9 additions & 2 deletions cloud/blockstore/libs/service/aligned_device_handler.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "aligned_device_handler.h"

#include <cloud/blockstore/libs/service/checksum_storage_wrapper.h>
#include <cloud/blockstore/libs/service/context.h>
#include <cloud/blockstore/libs/service/storage.h>

Expand Down Expand Up @@ -130,10 +131,16 @@ TBlocksInfo TBlocksInfo::MakeAligned() const

TAlignedDeviceHandler::TAlignedDeviceHandler(
IStoragePtr storage,
TString diskId,
TString clientId,
ui32 blockSize,
ui32 maxSubRequestSize)
: Storage(std::move(storage))
ui32 maxSubRequestSize,
bool checkBufferModificationDuringWriting)
: Storage(
checkBufferModificationDuringWriting ? CreateChecksumStorageWrapper(
std::move(storage),
std::move(diskId))
: std::move(storage))
, ClientId(std::move(clientId))
, BlockSize(blockSize)
, MaxBlockCount(maxSubRequestSize / BlockSize)
Expand Down
4 changes: 3 additions & 1 deletion cloud/blockstore/libs/service/aligned_device_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,11 @@ class TAlignedDeviceHandler final
public:
TAlignedDeviceHandler(
IStoragePtr storage,
TString diskId,
TString clientId,
ui32 blockSize,
ui32 maxSubRequestSize);
ui32 maxSubRequestSize,
bool checkBufferModificationDuringWriting);

// implements IDeviceHandler
NThreading::TFuture<NProto::TReadBlocksLocalResponse> Read(
Expand Down
Loading
Loading