diff --git a/cloud/filestore/libs/diagnostics/critical_events.h b/cloud/filestore/libs/diagnostics/critical_events.h index fa8dec6273..ac6c0486db 100644 --- a/cloud/filestore/libs/diagnostics/critical_events.h +++ b/cloud/filestore/libs/diagnostics/critical_events.h @@ -22,6 +22,13 @@ namespace NCloud::NFileStore{ #define FILESTORE_IMPOSSIBLE_EVENTS(xxx) \ xxx(CancelRoutineIsNotSet) \ + xxx(ChildNodeWithoutRef) \ + xxx(SessionNotFoundInTx) \ + xxx(InvalidNodeIdForLocalNode) \ + xxx(ChildNodeIsNull) \ + xxx(TargetNodeWithoutRef) \ + xxx(ParentNodeIsNull) \ + xxx(FailedToCreateHandle) \ // FILESTORE_IMPOSSIBLE_EVENTS //////////////////////////////////////////////////////////////////////////////// diff --git a/cloud/filestore/libs/storage/tablet/tablet_actor_createhandle.cpp b/cloud/filestore/libs/storage/tablet/tablet_actor_createhandle.cpp index 9e76e77149..e00283f2d6 100644 --- a/cloud/filestore/libs/storage/tablet/tablet_actor_createhandle.cpp +++ b/cloud/filestore/libs/storage/tablet/tablet_actor_createhandle.cpp @@ -2,6 +2,7 @@ #include "helpers.h" +#include #include #include @@ -271,7 +272,7 @@ void TIndexTabletActor::HandleCreateHandle( ExecuteTx( ctx, std::move(requestInfo), - msg->Record); + std::move(msg->Record)); } //////////////////////////////////////////////////////////////////////////////// @@ -289,7 +290,12 @@ bool TIndexTabletActor::PrepareTx_CreateHandle( args.ClientId, args.SessionId, args.SessionSeqNo); - TABLET_VERIFY(session); + if (!session) { + auto message = ReportSessionNotFoundInTx(TStringBuilder() + << "CreateHandle: " << args.Request.ShortDebugString()); + args.Error = MakeError(E_INVALID_STATE, std::move(message)); + return true; + } args.ReadCommitId = GetReadCommitId(session->GetCheckpointId()); if (args.ReadCommitId == InvalidCommitId) { @@ -377,7 +383,6 @@ bool TIndexTabletActor::PrepareTx_CreateHandle( } // TODO: AccessCheck - TABLET_VERIFY(args.TargetNode); } return true; @@ -391,7 +396,12 @@ void TIndexTabletActor::ExecuteTx_CreateHandle( FILESTORE_VALIDATE_TX_ERROR(CreateHandle, args); auto* session = FindSession(args.SessionId); - TABLET_VERIFY(session); + if (!session) { + auto message = ReportSessionNotFoundInTx(TStringBuilder() + << "CreateHandle: " << args.Request.ShortDebugString()); + args.Error = MakeError(E_INVALID_STATE, std::move(message)); + return; + } // TODO: check if session is read only args.WriteCommitId = GenerateCommitId(); @@ -404,8 +414,19 @@ void TIndexTabletActor::ExecuteTx_CreateHandle( if (args.TargetNodeId == InvalidNodeId && (args.FollowerId.Empty() || args.IsNewFollowerNode)) { - TABLET_VERIFY(!args.TargetNode); - TABLET_VERIFY(args.ParentNode); + if (args.TargetNode) { + auto message = ReportTargetNodeWithoutRef(TStringBuilder() + << "CreateHandle: " << args.Request.ShortDebugString()); + args.Error = MakeError(E_INVALID_STATE, std::move(message)); + return; + } + + if (!args.ParentNode) { + auto message = ReportParentNodeIsNull(TStringBuilder() + << "CreateHandle: " << args.Request.ShortDebugString()); + args.Error = MakeError(E_INVALID_STATE, std::move(message)); + return; + } if (args.FollowerId.Empty()) { NProto::TNode attrs = @@ -473,7 +494,13 @@ void TIndexTabletActor::ExecuteTx_CreateHandle( session->GetCheckpointId() ? args.ReadCommitId : InvalidCommitId, args.Flags); - TABLET_VERIFY(handle); + if (!handle) { + auto message = ReportFailedToCreateHandle(TStringBuilder() + << "CreateHandle: " << args.Request.ShortDebugString()); + args.Error = MakeError(E_INVALID_STATE, std::move(message)); + return; + } + args.Response.SetHandle(handle->GetHandle()); auto* node = args.Response.MutableNodeAttr(); ConvertNodeFromAttrs(*node, args.TargetNodeId, args.TargetNode->Attrs); @@ -526,7 +553,7 @@ void TIndexTabletActor::CompleteTx_CreateHandle( args.FollowerName, ctx.SelfID, CreateRegularAttrs(args.Mode, args.Uid, args.Gid), - std::move(args.Headers), + std::move(*args.Request.MutableHeaders()), std::move(args.Response)); auto actorId = NCloud::Register(ctx, std::move(actor)); diff --git a/cloud/filestore/libs/storage/tablet/tablet_actor_createnode.cpp b/cloud/filestore/libs/storage/tablet/tablet_actor_createnode.cpp index 3937bd3167..fba5e35ecd 100644 --- a/cloud/filestore/libs/storage/tablet/tablet_actor_createnode.cpp +++ b/cloud/filestore/libs/storage/tablet/tablet_actor_createnode.cpp @@ -2,6 +2,7 @@ #include "helpers.h" +#include #include namespace NCloud::NFileStore::NStorage { @@ -269,10 +270,10 @@ void TIndexTabletActor::HandleCreateNode( ExecuteTx( ctx, std::move(requestInfo), - msg->Record, + std::move(msg->Record), parentNodeId, targetNodeId, - attrs); + std::move(attrs)); } //////////////////////////////////////////////////////////////////////////////// @@ -312,7 +313,6 @@ bool TIndexTabletActor::PrepareTx_CreateNode( } // TODO: AccessCheck - TABLET_VERIFY(args.ParentNode); // validate target node doesn't exist TMaybe childRef; @@ -326,7 +326,13 @@ bool TIndexTabletActor::PrepareTx_CreateNode( return true; } - TABLET_VERIFY(!args.ChildNode); + if (args.ChildNode) { + auto message = ReportChildNodeWithoutRef(TStringBuilder() + << "CreateNode: " << args.Request.ShortDebugString()); + args.Error = MakeError(E_INVALID_STATE, std::move(message)); + return true; + } + if (args.TargetNodeId != InvalidNodeId) { // hard link: validate link target args.ChildNodeId = args.TargetNodeId; @@ -353,7 +359,6 @@ bool TIndexTabletActor::PrepareTx_CreateNode( } // TODO: AccessCheck - TABLET_VERIFY(args.ChildNode); } return true; @@ -367,7 +372,12 @@ void TIndexTabletActor::ExecuteTx_CreateNode( FILESTORE_VALIDATE_TX_ERROR(CreateNode, args); auto* session = FindSession(args.SessionId); - TABLET_VERIFY(session); + if (!session) { + auto message = ReportSessionNotFoundInTx(TStringBuilder() + << "CreateNode: " << args.Request.ShortDebugString()); + args.Error = MakeError(E_INVALID_STATE, std::move(message)); + return; + } TIndexTabletDatabase db(tx.DB); @@ -424,7 +434,12 @@ void TIndexTabletActor::ExecuteTx_CreateNode( args.FollowerName); if (args.FollowerId.Empty()) { - TABLET_VERIFY(args.ChildNodeId != InvalidNodeId); + if (args.ChildNodeId == InvalidNodeId) { + auto message = ReportInvalidNodeIdForLocalNode(TStringBuilder() + << "CreateNode: " << args.Request.ShortDebugString()); + args.Error = MakeError(E_INVALID_STATE, std::move(message)); + return; + } ConvertNodeFromAttrs( *args.Response.MutableNode(), @@ -445,7 +460,9 @@ void TIndexTabletActor::ExecuteTx_CreateNode( } } - // TODO(#1350): support DupCache for external nodes + // TODO(#1350): support DupCache for external nodes - modify DupCache entry + // upon CreateNode completion in follower - in the same tx that would + // delete the corresponding CreateNode op from the op log table } void TIndexTabletActor::CompleteTx_CreateNode( @@ -487,7 +504,12 @@ void TIndexTabletActor::CompleteTx_CreateNode( CommitDupCacheEntry(args.SessionId, args.RequestId); } - TABLET_VERIFY(args.ChildNode); + if (!args.ChildNode) { + auto message = ReportChildNodeIsNull(TStringBuilder() + << "CreateNode: " << args.Request.ShortDebugString()); + *args.Response.MutableError() = + MakeError(E_INVALID_STATE, std::move(message)); + } response->Record = std::move(args.Response); NProto::TSessionEvent sessionEvent; diff --git a/cloud/filestore/libs/storage/tablet/tablet_tx.h b/cloud/filestore/libs/storage/tablet/tablet_tx.h index 3367aebe91..8e29c3b2c7 100644 --- a/cloud/filestore/libs/storage/tablet/tablet_tx.h +++ b/cloud/filestore/libs/storage/tablet/tablet_tx.h @@ -616,22 +616,20 @@ struct TTxIndexTablet TCreateNode( TRequestInfoPtr requestInfo, - const NProto::TCreateNodeRequest request, + NProto::TCreateNodeRequest request, ui64 parentNodeId, ui64 targetNodeId, - const NProto::TNode& attrs) + NProto::TNode attrs) : TSessionAware(request) , RequestInfo(std::move(requestInfo)) , ParentNodeId(parentNodeId) , TargetNodeId(targetNodeId) , Name(request.GetName()) - , Attrs(attrs) + , Attrs(std::move(attrs)) , FollowerId(request.GetFollowerFileSystemId()) , FollowerName(CreateGuidAsString()) + , Request(std::move(request)) { - if (FollowerId) { - Request = request; - } } void Clear() @@ -1061,7 +1059,7 @@ struct TTxIndexTablet const ui32 Uid; const ui32 Gid; const TString RequestFollowerId; - NProto::THeaders Headers; + NProto::TCreateHandleRequest Request; ui64 ReadCommitId = InvalidCommitId; ui64 WriteCommitId = InvalidCommitId; @@ -1076,7 +1074,7 @@ struct TTxIndexTablet TCreateHandle( TRequestInfoPtr requestInfo, - const NProto::TCreateHandleRequest& request) + NProto::TCreateHandleRequest request) : TSessionAware(request) , RequestInfo(std::move(requestInfo)) , NodeId(request.GetNodeId()) @@ -1086,10 +1084,8 @@ struct TTxIndexTablet , Uid(request.GetUid()) , Gid(request.GetGid()) , RequestFollowerId(request.GetFollowerFileSystemId()) + , Request(std::move(request)) { - if (RequestFollowerId) { - Headers = request.GetHeaders(); - } } void Clear()