issue-506: improve tests, report critical event when request info doe… (

#633) * issue-506: improve tests, report critical event when request info does not have cancel routine but CancelRequest is called, make error messages when tablet restarts the same * align with nbs * update * update
ydb-platform · Jul 14, 2024 · 4d0ebbf · 4d0ebbf
1 parent 3ecd795
commit 4d0ebbf
Show file tree

Hide file tree

Showing 17 changed files with 86 additions and 23 deletions.
diff --git a/cloud/filestore/libs/diagnostics/critical_events.cpp b/cloud/filestore/libs/diagnostics/critical_events.cpp
@@ -19,19 +19,46 @@ void InitCriticalEventsCounter(NMonitoring::TDynamicCountersPtr counters)
 // FILESTORE_INIT_CRITICAL_EVENT_COUNTER
 
     FILESTORE_CRITICAL_EVENTS(FILESTORE_INIT_CRITICAL_EVENT_COUNTER)
+    FILESTORE_IMPOSSIBLE_EVENTS(FILESTORE_INIT_CRITICAL_EVENT_COUNTER)
 #undef FILESTORE_INIT_CRITICAL_EVENT_COUNTER
 
     NCloud::InitCriticalEventsCounter(std::move(counters));
 }
 
 #define FILESTORE_DEFINE_CRITICAL_EVENT_ROUTINE(name)                          \
-    void Report##name()                                                        \
+    TString Report##name(const TString& message)                               \
     {                                                                          \
-        ReportCriticalEvent("AppCriticalEvents/"#name, "", false);             \
+        return ReportCriticalEvent(                                            \
+            GetCriticalEventFor##name(),                                       \
+            message,                                                           \
+            false);                                                            \
     }                                                                          \
+                                                                               \
+    const TString GetCriticalEventFor##name()                                  \
+    {                                                                          \
+        return "AppCriticalEvents/"#name;                                      \
+    }                                                                        \
 // FILESTORE_DEFINE_CRITICAL_EVENT_ROUTINE
 
     FILESTORE_CRITICAL_EVENTS(FILESTORE_DEFINE_CRITICAL_EVENT_ROUTINE)
 #undef FILESTORE_DEFINE_CRITICAL_EVENT_ROUTINE
 
+#define FILESTORE_DEFINE_IMPOSSIBLE_EVENT_ROUTINE(name)                        \
+    TString Report##name(const TString& message)                               \
+    {                                                                          \
+        return ReportCriticalEvent(                                            \
+            GetCriticalEventFor##name(),                                       \
+            message,                                                           \
+            true);                                                             \
+    }                                                                          \
+                                                                               \
+    const TString GetCriticalEventFor##name()                                  \
+    {                                                                          \
+        return "AppCriticalEvents/"#name;                                      \
+    }                                                                          \
+// FILESTORE_DEFINE_IMPOSSIBLE_EVENT_ROUTINE
+
+    FILESTORE_IMPOSSIBLE_EVENTS(FILESTORE_DEFINE_IMPOSSIBLE_EVENT_ROUTINE)
+#undef FILESTORE_DEFINE_IMPOSSIBLE_EVENT_ROUTINE
+
 }   // namespace NCloud::NFileStore
diff --git a/cloud/filestore/libs/diagnostics/critical_events.h b/cloud/filestore/libs/diagnostics/critical_events.h
@@ -20,15 +20,28 @@ namespace NCloud::NFileStore{
     xxx(NodeNotFoundInFollower)                                                \
 // FILESTORE_CRITICAL_EVENTS
 
+#define FILESTORE_IMPOSSIBLE_EVENTS(xxx)                                       \
+    xxx(CancelRoutineIsNotSet)                                                 \
+// FILESTORE_IMPOSSIBLE_EVENTS
+
 ////////////////////////////////////////////////////////////////////////////////
 
 void InitCriticalEventsCounter(NMonitoring::TDynamicCountersPtr counters);
 
 #define FILESTORE_DECLARE_CRITICAL_EVENT_ROUTINE(name)                         \
-    void Report##name();                                                       \
+    TString Report##name(const TString& message = "");                         \
+    const TString GetCriticalEventFor##name();                                 \
 // FILESTORE_DECLARE_CRITICAL_EVENT_ROUTINE
 
     FILESTORE_CRITICAL_EVENTS(FILESTORE_DECLARE_CRITICAL_EVENT_ROUTINE)
 #undef FILESTORE_DECLARE_CRITICAL_EVENT_ROUTINE
 
+#define FILESTORE_DECLARE_IMPOSSIBLE_EVENT_ROUTINE(name)                       \
+    TString Report##name(const TString& message = "");                         \
+    const TString GetCriticalEventFor##name();                                 \
+// FILESTORE_DECLARE_IMPOSSIBLE_EVENT_ROUTINE
+
+    FILESTORE_IMPOSSIBLE_EVENTS(FILESTORE_DECLARE_IMPOSSIBLE_EVENT_ROUTINE)
+#undef FILESTORE_DECLARE_IMPOSSIBLE_EVENT_ROUTINE
+
 }   // namespace NCloud::NFileStore
diff --git a/cloud/filestore/libs/storage/core/request_info.h b/cloud/filestore/libs/storage/core/request_info.h
@@ -2,6 +2,7 @@
 
 #include "public.h"
 
+#include <cloud/filestore/libs/diagnostics/critical_events.h>
 #include <cloud/filestore/libs/service/context.h>
 #include <cloud/filestore/libs/storage/api/events.h>
 
@@ -48,6 +49,15 @@ struct TRequestInfo
         , CallContext(std::move(callContext))
     {}
 
+    void CancelRequest(const NActors::TActorContext& ctx)
+    {
+        if (!CancelRoutine) {
+            ReportCancelRoutineIsNotSet();
+            return;
+        };
+        CancelRoutine(ctx, *this);
+    }
+
     void AddExecCycles(ui64 cycles)
     {
         AtomicAdd(ExecCycles, cycles);
@@ -121,7 +131,7 @@ TRequestInfoPtr CreateRequestInfo(
         TRequestInfo& requestInfo)
     {
         auto response = std::make_unique<typename TMethod::TResponse>(
-            MakeError(E_REJECTED, "tablet is dead"));
+            MakeError(E_REJECTED, "tablet is shutting down"));
 
         NCloud::Reply(ctx, requestInfo, std::move(response));
     };

diff --git a/cloud/filestore/libs/storage/tablet/tablet_actor.cpp b/cloud/filestore/libs/storage/tablet/tablet_actor.cpp
@@ -277,7 +277,7 @@ void TIndexTabletActor::TerminateTransactions(const TActorContext& ctx)
         TRequestInfo* requestInfo = ActiveTransactions.PopFront();
         TABLET_VERIFY(requestInfo->RefCount() >= 1);
 
-        requestInfo->CancelRoutine(ctx, *requestInfo);
+        requestInfo->CancelRequest(ctx);
         requestInfo->UnRef();
     }
 }

diff --git a/cloud/filestore/libs/storage/tablet/tablet_actor.h b/cloud/filestore/libs/storage/tablet/tablet_actor.h
@@ -288,7 +288,7 @@ class TIndexTabletActor final
             TRequestInfo& requestInfo)
         {
             auto response = std::make_unique<typename TMethod::TResponse>(
-                MakeError(E_REJECTED, "tablet is dead"));
+                MakeError(E_REJECTED, "tablet is shutting down"));
 
             NCloud::Reply(ctx, requestInfo, std::move(response));
         };
@@ -321,7 +321,7 @@ class TIndexTabletActor final
             TRequestInfo& requestInfo)
         {
             auto response = std::make_unique<typename TMethod::TResponse>(
-                MakeError(E_REJECTED, "request cancelled"));
+                MakeError(E_REJECTED, "tablet is shutting down"));
 
             NCloud::Reply(ctx, requestInfo, std::move(response));
         };

diff --git a/cloud/filestore/libs/storage/tablet/tablet_actor_cleanupsessions.cpp b/cloud/filestore/libs/storage/tablet/tablet_actor_cleanupsessions.cpp
@@ -114,7 +114,7 @@ void TCleanupSessionsActor::HandlePoisonPill(
     const TActorContext& ctx)
 {
     Y_UNUSED(ev);
-    ReplyAndDie(ctx, MakeError(E_REJECTED, "request cancelled"));
+    ReplyAndDie(ctx, MakeError(E_REJECTED, "tablet is shutting down"));
 }
 
 void TCleanupSessionsActor::ReplyAndDie(

diff --git a/cloud/filestore/libs/storage/tablet/tablet_actor_collectgarbage.cpp b/cloud/filestore/libs/storage/tablet/tablet_actor_collectgarbage.cpp
@@ -257,7 +257,7 @@ void TCollectGarbageActor::HandlePoisonPill(
     const TActorContext& ctx)
 {
     Y_UNUSED(ev);
-    ReplyAndDie(ctx, MakeError(E_REJECTED, "request cancelled"));
+    ReplyAndDie(ctx, MakeError(E_REJECTED, "tablet is shutting down"));
 }
 
 void TCollectGarbageActor::HandleError(NProto::TError error)

diff --git a/cloud/filestore/libs/storage/tablet/tablet_actor_compaction.cpp b/cloud/filestore/libs/storage/tablet/tablet_actor_compaction.cpp
@@ -256,7 +256,7 @@ void TCompactionActor::HandlePoisonPill(
     const TActorContext& ctx)
 {
     Y_UNUSED(ev);
-    ReplyAndDie(ctx, MakeError(E_REJECTED, "request cancelled"));
+    ReplyAndDie(ctx, MakeError(E_REJECTED, "tablet is shutting down"));
 }
 
 void TCompactionActor::ReplyAndDie(

diff --git a/cloud/filestore/libs/storage/tablet/tablet_actor_destroycheckpoint.cpp b/cloud/filestore/libs/storage/tablet/tablet_actor_destroycheckpoint.cpp
@@ -158,7 +158,7 @@ void TDestroyCheckpointActor::HandlePoisonPill(
     const TActorContext& ctx)
 {
     Y_UNUSED(ev);
-    ReplyAndDie(ctx, MakeError(E_REJECTED, "request cancelled"));
+    ReplyAndDie(ctx, MakeError(E_REJECTED, "tablet is shutting down"));
 }
 
 void TDestroyCheckpointActor::ReplyAndDie(

diff --git a/cloud/filestore/libs/storage/tablet/tablet_actor_flush.cpp b/cloud/filestore/libs/storage/tablet/tablet_actor_flush.cpp
@@ -162,7 +162,7 @@ void TFlushActor::HandlePoisonPill(
     const TActorContext& ctx)
 {
     Y_UNUSED(ev);
-    ReplyAndDie(ctx, MakeError(E_REJECTED, "request cancelled"));
+    ReplyAndDie(ctx, MakeError(E_REJECTED, "tablet is shutting down"));
 }
 
 void TFlushActor::ReplyAndDie(

diff --git a/cloud/filestore/libs/storage/tablet/tablet_actor_flush_bytes.cpp b/cloud/filestore/libs/storage/tablet/tablet_actor_flush_bytes.cpp
@@ -404,7 +404,7 @@ void TFlushBytesActor::HandlePoisonPill(
     const TActorContext& ctx)
 {
     Y_UNUSED(ev);
-    ReplyAndDie(ctx, MakeError(E_REJECTED, "request cancelled"));
+    ReplyAndDie(ctx, MakeError(E_REJECTED, "tablet is shutting down"));
 }
 
 void TFlushBytesActor::ReplyAndDie(

diff --git a/cloud/filestore/libs/storage/tablet/tablet_actor_monitoring.cpp b/cloud/filestore/libs/storage/tablet/tablet_actor_monitoring.cpp
@@ -859,7 +859,7 @@ struct TIndexTabletMonitoringActor
         const TActorContext& ctx)
     {
         Y_UNUSED(ev);
-        ReplyAndDie(ctx, MakeError(E_REJECTED, "request cancelled"), {});
+        ReplyAndDie(ctx, MakeError(E_REJECTED, "tablet is shutting down"), {});
     }
 
     void ReplyAndDie(

diff --git a/cloud/filestore/libs/storage/tablet/tablet_actor_readblob.cpp b/cloud/filestore/libs/storage/tablet/tablet_actor_readblob.cpp
@@ -245,7 +245,7 @@ void TReadBlobActor::HandlePoisonPill(
     const TActorContext& ctx)
 {
     Y_UNUSED(ev);
-    ReplyAndDie(ctx, MakeError(E_REJECTED, "request cancelled"));
+    ReplyAndDie(ctx, MakeError(E_REJECTED, "tablet is shutting down"));
 }
 
 void TReadBlobActor::ReplyError(

diff --git a/cloud/filestore/libs/storage/tablet/tablet_actor_readdata.cpp b/cloud/filestore/libs/storage/tablet/tablet_actor_readdata.cpp
@@ -429,7 +429,7 @@ void TReadDataActor::HandlePoisonPill(
     const TActorContext& ctx)
 {
     Y_UNUSED(ev);
-    ReplyAndDie(ctx, MakeError(E_REJECTED, "request cancelled"));
+    ReplyAndDie(ctx, MakeError(E_REJECTED, "tablet is shutting down"));
 }
 
 void TReadDataActor::ReplyAndDie(

diff --git a/cloud/filestore/libs/storage/tablet/tablet_actor_writebatch.cpp b/cloud/filestore/libs/storage/tablet/tablet_actor_writebatch.cpp
@@ -175,7 +175,7 @@ void TWriteBatchActor::HandlePoisonPill(
     const TActorContext& ctx)
 {
     Y_UNUSED(ev);
-    ReplyAndDie(ctx, MakeError(E_REJECTED, "request cancelled"));
+    ReplyAndDie(ctx, MakeError(E_REJECTED, "tablet is shutting down"));
 }
 
 void TWriteBatchActor::ReplyAndDie(

diff --git a/cloud/filestore/libs/storage/tablet/tablet_actor_writeblob.cpp b/cloud/filestore/libs/storage/tablet/tablet_actor_writeblob.cpp
@@ -180,7 +180,7 @@ void TWriteBlobActor::HandlePoisonPill(
     const TActorContext& ctx)
 {
     Y_UNUSED(ev);
-    ReplyAndDie(ctx, MakeError(E_REJECTED, "request cancelled"));
+    ReplyAndDie(ctx, MakeError(E_REJECTED, "tablet is shutting down"));
 }
 
 void TWriteBlobActor::ReplyError(

diff --git a/cloud/filestore/libs/storage/tablet/tablet_ut_data.cpp b/cloud/filestore/libs/storage/tablet/tablet_ut_data.cpp
@@ -4377,9 +4377,11 @@ Y_UNIT_TEST_SUITE(TIndexTabletTest_Data)
             return false;
         });
 
-        env.GetRuntime().DispatchEvents(TDispatchOptions(), TDuration::Seconds(1));
-
-        UNIT_ASSERT(putSeen);
+        TDispatchOptions options;
+        options.CustomFinalCondition = [&] {
+            return putSeen;
+        };
+        env.GetRuntime().DispatchEvents(options);
 
         tablet.RebootTablet();
 
@@ -4388,6 +4390,10 @@ Y_UNIT_TEST_SUITE(TIndexTabletTest_Data)
             E_REJECTED,
             response->GetStatus(),
             response->GetErrorReason());
+
+        UNIT_ASSERT_VALUES_EQUAL(
+            "tablet is shutting down",
+            response->GetErrorReason());
     }
 
     TABLET_TEST(ShouldCancelWriteRequestsIfTabletIsRebooted)
@@ -4440,9 +4446,12 @@ Y_UNIT_TEST_SUITE(TIndexTabletTest_Data)
             return false;
         });
 
-        env.GetRuntime().DispatchEvents(TDispatchOptions(), TDuration::Seconds(1));
+        TDispatchOptions options;
+        options.CustomFinalCondition = [&] {
+            return getSeen;
+        };
 
-        UNIT_ASSERT(getSeen);
+        env.GetRuntime().DispatchEvents(options);
 
         failGet = false;
         tablet.RebootTablet();
@@ -4452,6 +4461,10 @@ Y_UNIT_TEST_SUITE(TIndexTabletTest_Data)
             E_REJECTED,
             response->GetStatus(),
             response->GetErrorReason());
+
+        UNIT_ASSERT_VALUES_EQUAL(
+            "tablet is shutting down",
+            response->GetErrorReason());
     }
 
 #define CHECK_GENERATED_BLOB(offset, length, expected)                       \