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.

Sign up for GitHub

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

Jump to bottom

23-3. Harmonize MaxRequestSize with minimum value (#2082) #2104

Merged
merged 1 commit into from
Sep 23, 2024
Merged

23-3. Harmonize MaxRequestSize with minimum value (#2082) #2104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 8 additions & 5 deletions cloud/blockstore/libs/endpoints_grpc/socket_endpoint_listener.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,16 @@ class TEndpointService final
const std::shared_ptr<NProto::TMountVolumeResponse> LastMountResponsePtr;

public:
TEndpointService(
TString diskId,
ui32 blockSize,
ISessionPtr session)
TEndpointService(TString diskId, ui32 blockSize, ISessionPtr session)
: DiskId(std::move(diskId))
, BlockSize(blockSize)
, StorageAdapter(session, blockSize, true)
, StorageAdapter(
session,
blockSize,
true, // normalize,
TDuration::Zero(), // maxRequestDuration
TDuration::Zero() // shutdownTimeout
)
, Session(std::move(session))
, LastMountResponsePtr(std::make_shared<NProto::TMountVolumeResponse>())
{
Expand Down
1 change: 0 additions & 1 deletion cloud/blockstore/libs/nbd/server_handler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -939,7 +939,6 @@ IServerHandlerFactoryPtr CreateServerHandlerFactory(
std::move(storage),
options.ClientId,
options.BlockSize,
NBD_MAX_BUFFER_SIZE / options.BlockSize,
options.UnalignedRequestsDisabled);

return std::make_shared<TServerHandlerFactory>(
Expand Down
8 changes: 7 additions & 1 deletion cloud/blockstore/libs/rdma/iface/client.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,13 @@ namespace NCloud::NBlockStore::NRdma {
struct TClientConfig
{
ui32 QueueSize = 10;
ui32 MaxBufferSize = 1024*1024;
// Keep the value greater then MaxSubRequestSize, ProcessingRangeSize,
// ResyncRangeSize in cloud/blockstore/libs/service/device_handler.cpp
// cloud/blockstore/libs/storage/partition_nonrepl/model/processing_blocks.h
// cloud/blockstore/libs/storage/partition_nonrepl/part_mirror_resync_util.h
// Keep sync with MaxBufferSize in cloud/blockstore/vhost-server/options.h
// and cloud/blockstore/libs/rdma/iface/server.h
ui32 MaxBufferSize = 4_MB + 4_KB;
EWaitMode WaitMode = EWaitMode::Poll;
ui32 PollerThreads = 1;
TDuration MaxReconnectDelay = TDuration::Seconds(60);
Expand Down
3 changes: 2 additions & 1 deletion cloud/blockstore/libs/rdma/iface/server.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ struct TServerConfig
{
ui32 Backlog = 10;
ui32 QueueSize = 10;
ui32 MaxBufferSize = 1024*1024;
// Keep sync with MaxBufferSize in cloud/blockstore/libs/rdma/iface/client.h
ui32 MaxBufferSize = 4_MB + 4_KB;
TDuration KeepAliveTimeout = TDuration::Seconds(10);
EWaitMode WaitMode = EWaitMode::Poll;
ui32 PollerThreads = 1;
Expand Down
9 changes: 7 additions & 2 deletions cloud/blockstore/libs/rdma_test/rdma_test_environment.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,13 @@ TRdmaTestEnvironment::TRdmaTestEnvironment(size_t deviceSize, ui32 poolSize)
: Storage(std::make_shared<TMemoryTestStorage>(deviceSize))
{
THashMap<TString, TStorageAdapterPtr> devices;
devices[Device_1] =
std::make_shared<TStorageAdapter>(Storage, 4_KB, true, 0);
devices[Device_1] = std::make_shared<TStorageAdapter>(
Storage,
4_KB, // storageBlockSize
true, // normalize,
TDuration::Zero(), // maxRequestDuration
TDuration::Zero() // shutdownTimeout
);
TVector<TString> uuids;
for (const auto& [key, value]: devices) {
uuids.push_back(key);
Expand Down
4 changes: 2 additions & 2 deletions cloud/blockstore/libs/service/aligned_device_handler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,11 @@ TAlignedDeviceHandler::TAlignedDeviceHandler(
IStoragePtr storage,
TString clientId,
ui32 blockSize,
ui32 maxBlockCount)
ui32 maxSubRequestSize)
: Storage(std::move(storage))
, ClientId(std::move(clientId))
, BlockSize(blockSize)
, MaxBlockCount(maxBlockCount)
, MaxBlockCount(maxSubRequestSize / BlockSize)
{
Y_ABORT_UNLESS(MaxBlockCount > 0);
}
Expand Down
2 changes: 1 addition & 1 deletion cloud/blockstore/libs/service/aligned_device_handler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class TAlignedDeviceHandler final
IStoragePtr storage,
TString clientId,
ui32 blockSize,
ui32 maxBlockCount);
ui32 maxSubRequestSize);

// implements IDeviceHandler
NThreading::TFuture<NProto::TReadBlocksLocalResponse> Read(
Expand Down
22 changes: 18 additions & 4 deletions cloud/blockstore/libs/service/device_handler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,31 +13,40 @@ namespace {

constexpr ui32 MaxUnalignedRequestSize = 32_MB;

// Keep the value less than MaxBufferSize in
// cloud/blockstore/libs/rdma/iface/client.h
constexpr ui32 MaxSubRequestSize = 4_MB;

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

struct TDefaultDeviceHandlerFactory final
: public IDeviceHandlerFactory
{
const ui32 MaxSubRequestSize;

explicit TDefaultDeviceHandlerFactory(ui32 maxSubRequestSize)
: MaxSubRequestSize(maxSubRequestSize)
{}

IDeviceHandlerPtr CreateDeviceHandler(
IStoragePtr storage,
TString clientId,
ui32 blockSize,
ui32 maxBlockCount,
bool unalignedRequestsDisabled) override
{
if (unalignedRequestsDisabled) {
return std::make_shared<TAlignedDeviceHandler>(
std::move(storage),
std::move(clientId),
blockSize,
maxBlockCount);
MaxSubRequestSize);
}

return std::make_shared<TUnalignedDeviceHandler>(
std::move(storage),
std::move(clientId),
blockSize,
maxBlockCount,
MaxSubRequestSize,
MaxUnalignedRequestSize);
}
};
Expand All @@ -48,7 +57,12 @@ struct TDefaultDeviceHandlerFactory final

IDeviceHandlerFactoryPtr CreateDefaultDeviceHandlerFactory()
{
return std::make_shared<TDefaultDeviceHandlerFactory>();
return std::make_shared<TDefaultDeviceHandlerFactory>(MaxSubRequestSize);
}

IDeviceHandlerFactoryPtr CreateDeviceHandlerFactory(ui32 maxSubRequestSize)
{
return std::make_shared<TDefaultDeviceHandlerFactory>(maxSubRequestSize);
}

} // namespace NCloud::NBlockStore
2 changes: 1 addition & 1 deletion cloud/blockstore/libs/service/device_handler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,12 @@ struct IDeviceHandlerFactory
IStoragePtr storage,
TString clientId,
ui32 blockSize,
ui32 maxBlockCount,
bool unalignedRequestsDisabled) = 0;
};

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

IDeviceHandlerFactoryPtr CreateDefaultDeviceHandlerFactory();
IDeviceHandlerFactoryPtr CreateDeviceHandlerFactory(ui32 maxSubRequestSize);

} // namespace NCloud::NBlockStore
16 changes: 6 additions & 10 deletions cloud/blockstore/libs/service/device_handler_ut.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,11 +123,11 @@ class TTestEnvironment
});
};

DeviceHandler = CreateDefaultDeviceHandlerFactory()->CreateDeviceHandler(
auto factory = CreateDeviceHandlerFactory(maxBlockCount * BlockSize);
DeviceHandler = factory->CreateDeviceHandler(
std::move(testStorage),
"testClientId",
BlockSize,
maxBlockCount,
unalignedRequestsDisabled);
}

Expand Down Expand Up @@ -335,11 +335,11 @@ Y_UNIT_TEST_SUITE(TDeviceHandlerTest)

auto storage = std::make_shared<TTestStorage>();

auto deviceHandler = CreateDefaultDeviceHandlerFactory()->CreateDeviceHandler(
auto factory = CreateDeviceHandlerFactory(blocksCountLimit * blockSize);
auto deviceHandler = factory->CreateDeviceHandler(
storage,
clientId,
blockSize,
blocksCountLimit,
false);

std::array<bool, deviceBlocksCount> zeroBlocks;
Expand Down Expand Up @@ -402,7 +402,6 @@ Y_UNIT_TEST_SUITE(TDeviceHandlerTest)
storage,
clientId,
blockSize,
1024,
true);

ui32 startIndex = 42;
Expand Down Expand Up @@ -489,7 +488,6 @@ Y_UNIT_TEST_SUITE(TDeviceHandlerTest)
storage,
clientId,
blockSize,
1024,
true);

{
Expand Down Expand Up @@ -598,11 +596,11 @@ Y_UNIT_TEST_SUITE(TDeviceHandlerTest)

auto storage = std::make_shared<TTestStorage>();

auto deviceHandler = CreateDefaultDeviceHandlerFactory()->CreateDeviceHandler(
auto factory = CreateDeviceHandlerFactory(blocksCountLimit * blockSize);
auto deviceHandler = factory->CreateDeviceHandler(
storage,
clientId,
blockSize,
blocksCountLimit,
unalignedRequestDisabled);

storage->ZeroBlocksHandler = [&] (
Expand Down Expand Up @@ -717,7 +715,6 @@ Y_UNIT_TEST_SUITE(TDeviceHandlerTest)
storage,
clientId,
blockSize,
1024,
false);

storage->WriteBlocksLocalHandler = [&] (
Expand Down Expand Up @@ -781,7 +778,6 @@ Y_UNIT_TEST_SUITE(TDeviceHandlerTest)
storage,
clientId,
blockSize,
1024,
false);

storage->WriteBlocksLocalHandler = [&] (
Expand Down
10 changes: 4 additions & 6 deletions cloud/blockstore/libs/service/storage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ namespace {

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

constexpr ui32 MaxRequestSize = 32_MB;

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

class TStorageStub final
: public IStorage
{
Expand Down Expand Up @@ -173,7 +177,6 @@ class TStorageAdapter::TImpl
const IStoragePtr Storage;
const ui32 StorageBlockSize;
const bool Normalize;
const ui32 MaxRequestSize;
const TDuration MaxRequestDuration;

std::shared_ptr<TInflightReads> InflightReads{
Expand All @@ -190,7 +193,6 @@ class TStorageAdapter::TImpl
IStoragePtr storage,
ui32 storageBlockSize,
bool normalize,
ui32 maxRequestSize,
TDuration maxRequestDuration);

TFuture<NProto::TReadBlocksResponse> ReadBlocks(
Expand Down Expand Up @@ -240,12 +242,10 @@ TStorageAdapter::TImpl::TImpl(
IStoragePtr storage,
ui32 storageBlockSize,
bool normalize,
ui32 maxRequestSize,
TDuration maxRequestDuration)
: Storage(std::move(storage))
, StorageBlockSize(storageBlockSize)
, Normalize(normalize)
, MaxRequestSize(maxRequestSize)
, MaxRequestDuration(maxRequestDuration)
{}

Expand Down Expand Up @@ -666,14 +666,12 @@ TStorageAdapter::TStorageAdapter(
IStoragePtr storage,
ui32 storageBlockSize,
bool normalize,
ui32 maxRequestSize,
TDuration maxRequestDuration,
TDuration shutdownTimeout)
: Impl(std::make_unique<TImpl>(
std::move(storage),
storageBlockSize,
normalize,
maxRequestSize,
maxRequestDuration))
, ShutdownTimeout(shutdownTimeout)
{}
Expand Down
5 changes: 2 additions & 3 deletions cloud/blockstore/libs/service/storage.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,8 @@ class TStorageAdapter
ui32 storageBlockSize,
bool normalize, // When true, we use StorageBlockSize when making
// requests to the underlying storage
ui32 maxRequestSize = 0,
TDuration maxRequestDuration = TDuration::Zero(),
TDuration shutdownTimeout = TDuration::Zero());
TDuration maxRequestDuration,
TDuration shutdownTimeout);

~TStorageAdapter();

Expand Down
37 changes: 22 additions & 15 deletions cloud/blockstore/libs/service/storage_ut.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,11 @@ Y_UNIT_TEST_SUITE(TStorageTest)

auto adapter = std::make_shared<TStorageAdapter>(
storage,
4_KB, // storageBlockSize
false, // normalize
32_MB); // maxRequestSize
4_KB, // storageBlockSize
false, // normalize,
TDuration::Zero(), // maxRequestDuration
TDuration::Zero() // shutdownTimeout
);

auto request = std::make_shared<NProto::TWriteBlocksRequest>();
request->SetStartIndex(1000);
Expand Down Expand Up @@ -143,9 +145,11 @@ Y_UNIT_TEST_SUITE(TStorageTest)

auto adapter = std::make_shared<TStorageAdapter>(
storage,
4_KB, // storageBlockSize
true, // normalize
32_MB); // maxRequestSize
4_KB, // storageBlockSize
true, // normalize,
TDuration::Zero(), // maxRequestDuration
TDuration::Zero() // shutdownTimeout
);

auto request = std::make_shared<NProto::TWriteBlocksRequest>();
request->SetStartIndex(1000);
Expand Down Expand Up @@ -215,10 +219,10 @@ Y_UNIT_TEST_SUITE(TStorageTest)

auto adapter = std::make_shared<TStorageAdapter>(
storage,
4_KB, // storageBlockSize
false, // normalize
32_MB, // maxRequestSize
TDuration::Seconds(1) // timeout
4_KB, // storageBlockSize
false, // normalize
TDuration::Seconds(1), // maxRequestDuration
TDuration::Zero() // shutdownTimeout
);

auto now = TInstant::Seconds(1);
Expand Down Expand Up @@ -337,7 +341,6 @@ Y_UNIT_TEST_SUITE(TStorageTest)
storage,
4_KB, // storageBlockSize
false, // normalize
32_MB, // maxRequestSize
TDuration::Seconds(1), // request timeout
shutdownTimeout // shutdown timeout
);
Expand Down Expand Up @@ -463,9 +466,11 @@ Y_UNIT_TEST_SUITE(TStorageTest)

auto adapter = std::make_shared<TStorageAdapter>(
storage,
4096, // storageBlockSize
4_KB, // storageBlockSize
normalize,
32_MB); // maxRequestSize
TDuration::Zero(), // request timeout
TDuration::Zero() // shutdown timeout
);

{ // Requesting the reading of two blocks. Since the first block will
// be filled with zeros, it should be optimized and will return an
Expand Down Expand Up @@ -542,9 +547,11 @@ Y_UNIT_TEST_SUITE(TStorageTest)

auto adapter = std::make_shared<TStorageAdapter>(
storage,
4096, // storageBlockSize
4_KB, // storageBlockSize
normalize,
32_MB); // maxRequestSize
TDuration::Zero(), // request timeout
TDuration::Zero() // shutdown timeout
);

{
auto request = std::make_shared<NProto::TReadBlocksRequest>();
Expand Down
Loading
Loading