Skip to content

Commit ff1c76c

Browse files
committed
issue-5432: drain async handle operations queue back to back
1 parent bdb2af8 commit ff1c76c

11 files changed

Lines changed: 125 additions & 16 deletions

File tree

cloud/filestore/libs/service_local/config.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ namespace {
2929
xxx(GuestWriteBackCacheEnabled, bool, false )\
3030
xxx(AsyncDestroyHandleEnabled, bool, false )\
3131
xxx(AsyncDestroyReadOnlyHandleEnabled, bool, false )\
32-
xxx(AsyncHandleOperationPeriod, TDuration, 50ms )\
32+
xxx(AsyncHandleOperationPeriod, TDuration, 0ms )\
3333
xxx(OpenNodeByHandleEnabled, bool, false )\
3434
xxx(NodeCleanupBatchSize, ui32, 1000 )\
3535
xxx(ZeroCopyEnabled, bool, false )\

cloud/filestore/libs/storage/core/config.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ using TAliases = NProto::TStorageConfig::TFilestoreAliases;
264264
xxx(AsyncCreateHandleEnabled, bool, false )\
265265
xxx(TabletUnsafeAsyncReadOnlyCreateHandleEnabled, bool, false )\
266266
xxx(TabletUnsafeAsyncDestroyHandleEnabled, bool, false )\
267-
xxx(AsyncHandleOperationPeriod, TDuration, TDuration::MilliSeconds(50))\
267+
xxx(AsyncHandleOperationPeriod, TDuration, TDuration::Zero() )\
268268
xxx(UnconfirmedCreateHandleGraceTimeout, \
269269
TDuration, TDuration::Minutes(2) )\
270270
\

cloud/filestore/libs/storage/tablet/tablet_ut_sessions.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1420,8 +1420,6 @@ Y_UNIT_TEST_SUITE(TIndexTabletTest_Sessions)
14201420
NProto::TFileStoreFeatures features;
14211421
features.SetThreeStageWriteThreshold(64_KB);
14221422
features.SetPreferredBlockSize(4_KB);
1423-
features.SetAsyncHandleOperationPeriod(
1424-
TDuration::MilliSeconds(50).MilliSeconds());
14251423
features.SetHasXAttrs(true);
14261424
features.SetMaxFuseLoopThreads(1);
14271425
features.SetTabletDirectRdmaEnabled(false);

cloud/filestore/libs/vfs_fuse/config.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ namespace {
3232
xxx(AsyncDestroyHandleEnabled, bool, false )\
3333
xxx(AsyncDestroyReadOnlyHandleEnabled, bool, false )\
3434
xxx(AsyncCreateHandleEnabled, bool, false )\
35-
xxx(AsyncHandleOperationPeriod, TDuration, TDuration::MilliSeconds(50) )\
35+
xxx(AsyncHandleOperationPeriod, TDuration, TDuration::Zero() )\
3636
\
3737
xxx(DirectIoEnabled, bool, false )\
3838
xxx(DirectIoAlign, ui32, 4_KB )\

cloud/filestore/libs/vfs_fuse/fs_impl.cpp

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@ ui64 GenerateCacheVersion(std::atomic<ui64>& version)
2929
return version.fetch_add(1, std::memory_order_release) + 1;
3030
}
3131

32+
// AsyncHandleOperationPeriod defaults to 0 so a non-empty queue is drained
33+
// back-to-back. When the queue is empty we back off by this hardcoded delay
34+
// instead to avoid busy looping.
35+
constexpr TDuration EmptyHandleOpsQueueBackoff = TDuration::MilliSeconds(50);
36+
3237
} // namespace
3338

3439
////////////////////////////////////////////////////////////////////////////////
@@ -88,7 +93,7 @@ TFileSystem::~TFileSystem()
8893
void TFileSystem::Init()
8994
{
9095
STORAGE_INFO("scheduling handle ops queue processing");
91-
ScheduleProcessHandleOpsQueue();
96+
ScheduleProcessHandleOpsQueue(Config->GetAsyncHandleOperationPeriod());
9297
}
9398

9499
void TFileSystem::Reset()
@@ -97,11 +102,11 @@ void TFileSystem::Reset()
97102
DirectoryHandleCache->Reset();
98103
}
99104

100-
void TFileSystem::ScheduleProcessHandleOpsQueue()
105+
void TFileSystem::ScheduleProcessHandleOpsQueue(TDuration delay)
101106
{
102107
if (HandleOpsQueue) {
103108
Scheduler->Schedule(
104-
Timer->Now() + Config->GetAsyncHandleOperationPeriod(),
109+
Timer->Now() + delay,
105110
[=, ptr = weak_from_this()]()
106111
{
107112
if (auto self = ptr.lock()) {
@@ -415,7 +420,7 @@ void TFileSystem::CompleteHandleOpsQueueEntry()
415420
HandleOpsQueue->PopFront();
416421
}
417422
ProcessDelayedRelease();
418-
ScheduleProcessHandleOpsQueue();
423+
ScheduleProcessHandleOpsQueue(Config->GetAsyncHandleOperationPeriod());
419424
}
420425

421426
void TFileSystem::ProcessDelayedRelease()
@@ -440,7 +445,7 @@ void TFileSystem::ProcessHandleOpsQueue()
440445
{
441446
TGuard g{HandleOpsQueueLock};
442447
if (HandleOpsQueue->Empty()) {
443-
ScheduleProcessHandleOpsQueue();
448+
ScheduleProcessHandleOpsQueue(EmptyHandleOpsQueueBackoff);
444449
return;
445450
}
446451

@@ -451,7 +456,7 @@ void TFileSystem::ProcessHandleOpsQueue()
451456
<< "Failed to get TQueueEntry from queue, filesystem: "
452457
<< Config->GetFileSystemId());
453458
HandleOpsQueue->PopFront();
454-
ScheduleProcessHandleOpsQueue();
459+
ScheduleProcessHandleOpsQueue(Config->GetAsyncHandleOperationPeriod());
455460
return;
456461
}
457462

@@ -513,7 +518,7 @@ void TFileSystem::ProcessHandleOpsQueue()
513518
TStringBuilder() << "Unexpected TQueueEntry in queue, filesystem: "
514519
<< Config->GetFileSystemId());
515520
HandleOpsQueue->PopFront();
516-
ScheduleProcessHandleOpsQueue();
521+
ScheduleProcessHandleOpsQueue(Config->GetAsyncHandleOperationPeriod());
517522
return;
518523
}
519524

cloud/filestore/libs/vfs_fuse/fs_impl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,7 @@ class TFileSystem final
514514

515515
void ClearDirectoryCache();
516516

517-
void ScheduleProcessHandleOpsQueue();
517+
void ScheduleProcessHandleOpsQueue(TDuration delay);
518518
void ProcessHandleOpsQueue();
519519

520520
void DoWrite(

cloud/filestore/libs/vfs_fuse/fs_ut.cpp

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3223,6 +3223,110 @@ Y_UNIT_TEST_SUITE(TFileSystemTest)
32233223
AtomicGet(counters->GetCounter("InProgress")->GetAtomic()));
32243224
}
32253225

3226+
Y_UNIT_TEST(ShouldDrainHandleOpsQueueBackToBack)
3227+
{
3228+
// AsyncHandleOperationPeriod defaults to 0, so a non-empty queue is
3229+
// drained back-to-back: every entry is rescheduled with zero delay.
3230+
// With a frozen clock, running only the tasks due "now" fires just the
3231+
// zero-delay tasks, so the whole queue must drain without advancing
3232+
// time. A non-zero period would leave every processing task in the
3233+
// future and this drain would never complete.
3234+
constexpr ui32 requestCount = 3;
3235+
3236+
NProto::TFileStoreFeatures features;
3237+
features.SetAsyncDestroyHandleEnabled(true);
3238+
3239+
auto timer = std::make_shared<TTestTimer>();
3240+
auto scheduler = std::make_shared<TTestScheduler>(timer->Now());
3241+
TBootstrap bootstrap(timer, scheduler, features);
3242+
3243+
std::atomic_uint handlerCalled = 0;
3244+
bootstrap.Service->SetHandlerDestroyHandle(
3245+
[&](auto, auto)
3246+
{
3247+
++handlerCalled;
3248+
return MakeFuture(NProto::TDestroyHandleResponse{});
3249+
});
3250+
3251+
auto inProgress =
3252+
bootstrap.Counters->FindSubgroup("component", "fs_ut")
3253+
->FindSubgroup("request", "DestroyHandle")
3254+
->GetCounter("InProgress");
3255+
3256+
bootstrap.Start();
3257+
Y_DEFER {
3258+
bootstrap.Stop();
3259+
};
3260+
3261+
for (ui32 i = 0; i < requestCount; ++i) {
3262+
auto future = bootstrap.Fuse->SendRequest<TReleaseRequest>(
3263+
10 + i,
3264+
2 + i,
3265+
O_RDONLY);
3266+
UNIT_ASSERT_NO_EXCEPTION(future.GetValue(WaitTimeout));
3267+
}
3268+
3269+
UNIT_ASSERT(WaitForCondition(
3270+
WaitTimeout,
3271+
[&]
3272+
{
3273+
scheduler->RunAllScheduledTasksUntilNow();
3274+
return handlerCalled.load() == requestCount
3275+
&& AtomicGet(inProgress->GetAtomic()) == 0;
3276+
}));
3277+
}
3278+
3279+
Y_UNIT_TEST(ShouldBackOffWhenHandleOpsQueueEmpty)
3280+
{
3281+
// On an empty queue processing is rescheduled with a non zero default
3282+
// to avoid a busy loop.
3283+
constexpr auto EmptyQueueBackoff = TDuration::MilliSeconds(50);
3284+
3285+
NProto::TFileStoreFeatures features;
3286+
features.SetAsyncDestroyHandleEnabled(true);
3287+
3288+
auto timer = std::make_shared<TTestTimer>();
3289+
auto scheduler = std::make_shared<TTestScheduler>(timer->Now());
3290+
TBootstrap bootstrap(timer, scheduler, features);
3291+
3292+
std::atomic_uint handlerCalled = 0;
3293+
bootstrap.Service->SetHandlerDestroyHandle(
3294+
[&](auto, auto)
3295+
{
3296+
++handlerCalled;
3297+
return MakeFuture(NProto::TDestroyHandleResponse{});
3298+
});
3299+
3300+
bootstrap.Start();
3301+
Y_DEFER {
3302+
bootstrap.Stop();
3303+
};
3304+
3305+
scheduler->RunAllScheduledTasksUntilNow();
3306+
3307+
auto future = bootstrap.Fuse->SendRequest<TReleaseRequest>(
3308+
10,
3309+
2,
3310+
O_RDONLY);
3311+
UNIT_ASSERT_NO_EXCEPTION(future.GetValue(WaitTimeout));
3312+
3313+
// The only scheduled poll is EmptyQueueBackoff in the future, so at the
3314+
// current (frozen) time it does not fire and the entry stays pending.
3315+
scheduler->RunAllScheduledTasksUntilNow();
3316+
UNIT_ASSERT_VALUES_EQUAL(0U, handlerCalled.load());
3317+
3318+
// Once the backoff elapses the poll fires and picks up the entry.
3319+
timer->AdvanceTime(EmptyQueueBackoff);
3320+
scheduler->AdvanceTime(EmptyQueueBackoff);
3321+
UNIT_ASSERT(WaitForCondition(
3322+
WaitTimeout,
3323+
[&]
3324+
{
3325+
scheduler->RunAllScheduledTasksUntilNow();
3326+
return handlerCalled.load() == 1;
3327+
}));
3328+
}
3329+
32263330
Y_UNIT_TEST(ShouldProcessReadOnlyDestroyHandleRequestsAsynchronously)
32273331
{
32283332
NProto::TFileStoreFeatures features;

cloud/filestore/tests/async_open_test/lib/__init__.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,8 @@ def file_system_handles():
192192
lambda counts: all(count > 0 for count in counts.values()))
193193

194194
# The queue is processed one entry per AsyncHandleOperationPeriod
195-
# (50ms by default), so draining all OPEN_HANDLE_COUNT confirmations
195+
# (set to 50ms in nfs-storage-patch.txt; the product default is now 0
196+
# for back-to-back drain), so draining all OPEN_HANDLE_COUNT confirmations
196197
# takes tens of seconds. Waiting for half of them leaves the rest
197198
# pending in the queue while the restarts below happen.
198199
wait_for(
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,5 @@
11
AsyncDestroyHandleEnabled: true
22
AsyncCreateHandleEnabled: true
3+
# Slow the queue drain (default is now 0/back-to-back) so restarts can happen
4+
# while confirmations are still pending in the queue.
5+
AsyncHandleOperationPeriod: 50

cloud/filestore/tests/client_sharded/canondata/test.test_enable_directory_creation_in_shards/results.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ FileStore {
1212
Features {
1313
ThreeStageWriteThreshold: 65536
1414
PreferredBlockSize: 4096
15-
AsyncHandleOperationPeriod: 50
1615
HasXAttrs: true
1716
MaxFuseLoopThreads: 1
1817
TabletDirectRdmaEnabled: false

0 commit comments

Comments
 (0)