Skip to content

Commit

Permalink
issue-95: 1. fixing a bug that caused FlushState to get stuck in the …
Browse files Browse the repository at this point in the history
…Enqueued state if FlushBytes got triggered before the complete initialization of CompactionMap 2. rebooting tablet after N backpressure errors to make it more resilient in case of similar bugs (#486)
  • Loading branch information
qkrorlqr committed Feb 19, 2024
1 parent 28103d9 commit a4fdae2
Show file tree
Hide file tree
Showing 7 changed files with 192 additions and 0 deletions.
5 changes: 5 additions & 0 deletions cloud/filestore/config/storage.proto
Original file line number Diff line number Diff line change
Expand Up @@ -205,4 +205,9 @@ message TStorageConfig

// Max inflight for out of order compaction map load requests.
optional uint32 MaxOutOfOrderCompactionMapLoadRequestsInQueue = 333;

// IndexTabletActor will suicide (and thus reboot) after observing this
// number of backpressure errors. Needed to automatically recover after
// various races that may happen during index tablet startup due to bugs.
optional uint32 MaxBackpressureErrorsBeforeSuicide = 334;
}
1 change: 1 addition & 0 deletions cloud/filestore/libs/storage/core/config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ namespace {
\
xxx(TwoStageReadEnabled, bool, false )\
xxx(MaxOutOfOrderCompactionMapLoadRequestsInQueue, ui32, 5 )\
xxx(MaxBackpressureErrorsBeforeSuicide, ui32, 1000 )\
// FILESTORE_STORAGE_CONFIG

#define FILESTORE_DECLARE_CONFIG(name, type, value) \
Expand Down
2 changes: 2 additions & 0 deletions cloud/filestore/libs/storage/core/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,8 @@ class TStorageConfig

bool GetConfigsDispatcherServiceEnabled() const;

ui32 GetMaxBackpressureErrorsBeforeSuicide() const;

void Dump(IOutputStream& out) const;
void DumpHtml(IOutputStream& out) const;
void DumpOverridesHtml(IOutputStream& out) const;
Expand Down
2 changes: 2 additions & 0 deletions cloud/filestore/libs/storage/tablet/tablet_actor.h
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@ class TIndexTabletActor final
// used on monpages
NProto::TStorageConfig StorageConfigOverride;

ui32 BackpressureErrorCount = 0;

public:
TIndexTabletActor(
const NActors::TActorId& owner,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,12 @@ void TIndexTabletActor::HandleFlushBytes(
BlobIndexOpState.Complete();
}

// Flush may have been enqueued if FlushBytes was enqueued by the
// tablet (via EnqueueBlobIndexOpIfNeeded).
if (FlushState.GetOperationState() == EOperationState::Enqueued) {
FlushState.Complete();
}

replyError(
ctx,
*ev,
Expand Down
15 changes: 15 additions & 0 deletions cloud/filestore/libs/storage/tablet/tablet_actor_writedata.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,17 @@ void TIndexTabletActor::HandleWriteData(
}

if (!IsWriteAllowed(BuildBackpressureThresholds())) {
if (++BackpressureErrorCount >=
Config->GetMaxBackpressureErrorsBeforeSuicide())
{
LOG_WARN(ctx, TFileStoreComponents::TABLET_WORKER,
"%s Suiciding after %u backpressure errors",
LogTag.c_str(),
BackpressureErrorCount);

Suicide(ctx);
}

return MakeError(E_REJECTED, "rejected due to backpressure");
}

Expand All @@ -317,6 +328,10 @@ void TIndexTabletActor::HandleWriteData(
return;
}

// this request passed the backpressure check => tablet is not stuck
// anywhere, we can reset our backpressure error counter
BackpressureErrorCount = 0;

// either rejected or put into queue
if (ThrottleIfNeeded<TEvService::TWriteDataMethod>(ev, ctx)) {
return;
Expand Down
161 changes: 161 additions & 0 deletions cloud/filestore/libs/storage/tablet/tablet_ut_data.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2305,6 +2305,74 @@ Y_UNIT_TEST_SUITE(TIndexTabletTest_Data)
tablet.DestroyHandle(handle);
}

TABLET_TEST(ShouldSuicideAfterBackpressureErrors)
{
const auto block = tabletConfig.BlockSize;

NProto::TStorageConfig storageConfig;
storageConfig.SetWriteBlobThreshold(2 * block);
storageConfig.SetFlushThreshold(1_GB);
storageConfig.SetCompactionThreshold(999'999);
storageConfig.SetCleanupThreshold(999'999);
storageConfig.SetFlushBytesThreshold(1_GB);
storageConfig.SetFlushThresholdForBackpressure(2 * block);
storageConfig.SetCompactionThresholdForBackpressure(999'999);
storageConfig.SetCleanupThresholdForBackpressure(999'999);
storageConfig.SetFlushBytesThresholdForBackpressure(1_GB);
storageConfig.SetFlushThresholdForBackpressure(block);
storageConfig.SetMaxBackpressureErrorsBeforeSuicide(2);

TTestEnv env({}, std::move(storageConfig));
env.CreateSubDomain("nfs");

ui32 nodeIdx = env.CreateNode("nfs");
ui64 tabletId = env.BootIndexTablet(nodeIdx);

TIndexTabletClient tablet(
env.GetRuntime(),
nodeIdx,
tabletId,
tabletConfig);
tablet.InitSession("client", "session");

auto id = CreateNode(tablet, TCreateNodeArgs::File(RootNodeId, "test"));
ui64 handle = CreateHandle(tablet, id);
tablet.WriteData(handle, 0, block, '0'); // 1 fresh block, 1 marker

// backpressure due to FlushThresholdForBackpressure
tablet.SendWriteDataRequest(handle, 0, block, '0');
{
auto response = tablet.RecvWriteDataResponse();
UNIT_ASSERT_VALUES_EQUAL(E_REJECTED, response->GetStatus());
}

// this filter is rather coarse so we need to enable it as late as
// possible - as close to the poison pill "trigger point" as possible
bool poisonPillObserved = false;
env.GetRuntime().SetEventFilter([&] (auto& runtime, auto& event) {
Y_UNUSED(runtime);

switch (event->GetTypeRewrite()) {
case TEvents::TSystem::Poison: {
poisonPillObserved = true;
}
}

return false;
});

// backpressure due to FlushThresholdForBackpressure
// should cause tablet reboot
tablet.SendWriteDataRequest(handle, 0, block, '0');
{
auto response = tablet.RecvWriteDataResponse();
UNIT_ASSERT_VALUES_EQUAL(E_REJECTED, response->GetStatus());
}

env.GetRuntime().DispatchEvents({}, TDuration::Seconds(1));
UNIT_ASSERT(poisonPillObserved);
}

TABLET_TEST(ShouldReadUnAligned)
{
const auto block = tabletConfig.BlockSize;
Expand Down Expand Up @@ -3253,6 +3321,99 @@ Y_UNIT_TEST_SUITE(TIndexTabletTest_Data)
tablet.DestroyHandle(handle);
}

TABLET_TEST(FlushShouldNotGetStuckBecauseOfFlushBytesDuringCompactionMapLoading)
{
const auto block = tabletConfig.BlockSize;

NProto::TStorageConfig storageConfig;
storageConfig.SetFlushThreshold(4_KB);
storageConfig.SetFlushBytesThreshold(1);
storageConfig.SetLoadedCompactionRangesPerTx(2);

TTestEnv env({}, std::move(storageConfig));

env.CreateSubDomain("nfs");

ui32 nodeIdx = env.CreateNode("nfs");
ui64 tabletId = env.BootIndexTablet(nodeIdx);

TIndexTabletClient tablet(
env.GetRuntime(),
nodeIdx,
tabletId,
tabletConfig);
tablet.InitSession("client", "session");

auto id = CreateNode(tablet, TCreateNodeArgs::File(RootNodeId, "test"));
auto handle = CreateHandle(tablet, id);

// generating at least one compaction range
tablet.WriteData(handle, 0, block, 'a');

TAutoPtr<IEventHandle> loadChunk;
ui32 loadChunkCount = 0;
ui32 flushBytesCount = 0;
env.GetRuntime().SetEventFilter([&] (auto& runtime, auto& event) {
Y_UNUSED(runtime);

switch (event->GetTypeRewrite()) {
case TEvIndexTabletPrivate::EvFlushBytesRequest: {
++flushBytesCount;
break;
}

case TEvIndexTabletPrivate::EvLoadCompactionMapChunkRequest: {
++loadChunkCount;

// catching the second chunk - first one should be loaded
// so that we are able to write (and thus trigger our
// background ops)
if (loadChunkCount == 2) {
loadChunk = event.Release();
return true;
}
}
}

return false;
});

// rebooting to trigger compaction map reloading
tablet.RebootTablet();
tablet.RecoverSession();

handle = CreateHandle(tablet, id);

env.GetRuntime().DispatchEvents({}, TDuration::Seconds(1));
UNIT_ASSERT(loadChunk);
UNIT_ASSERT_VALUES_EQUAL(2, loadChunkCount);

// this write should succeed - it targets the range that should be
// loaded at this point of time
tablet.SendWriteDataRequest(handle, 0, 1_KB, 'a');
{
auto response = tablet.RecvWriteDataResponse();
UNIT_ASSERT_VALUES_EQUAL(S_OK, response->GetStatus());
}

// FlushBytes should've been triggered and its operation state should've
// been reset to Idle, Flush state should also be idle after this
UNIT_ASSERT_VALUES_EQUAL(1, flushBytesCount);

{
auto response = tablet.GetStorageStats();
const auto& stats = response->Record.GetStats();
UNIT_ASSERT_VALUES_EQUAL(
static_cast<ui32>(EOperationState::Idle),
static_cast<ui32>(stats.GetBlobIndexOpState()));
UNIT_ASSERT_VALUES_EQUAL(
static_cast<ui32>(EOperationState::Idle),
static_cast<ui32>(stats.GetFlushState()));
}

tablet.DestroyHandle(handle);
}

TABLET_TEST_16K(ShouldDescribeData)
{
const auto block = tabletConfig.BlockSize;
Expand Down

0 comments on commit a4fdae2

Please sign in to comment.