Skip to content

Commit

Permalink
issue-1350: simplified node creation in follower code, added a test f…
Browse files Browse the repository at this point in the history 
…or errors received from follower in this case, fixed DupCache entry commitment in CreateHandle, made TabletProxy immortal, deleted inactive pipe tracking and closure in TabletProxy, added some comments, fixed logging a bit (#1511)

* issue-1350: simplified node creation in follower code, added a test for errors received from follower in this case, added some comments, fixed logging a bit

* issue-1350: CreateHandle - if we need to create a node in one of the followers, we still need to commit DupCache entry;

* issue-1350: TabletProxy should never die, TabletProxy should not track and close idle connections - 1. there is no need for that 2. it breaks leader<->follower session creation logic
  • Loading branch information
@qkrorlqr
qkrorlqr committed Jun 29, 2024
1 parent e88c38a commit 3b25ff5
Show file tree
Hide file tree
Showing 13 changed files with 143 additions and 123 deletions.
2 changes: 1 addition & 1 deletion cloud/filestore/libs/storage/service/service_actor_createhandle.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ void TStorageServiceActor::HandleCreateHandle(
ctx,
TFileStoreComponents::SERVICE,
"create handle in follower %s",
msg->Record.DebugString().c_str());
msg->Record.ShortDebugString().Quote().c_str());

auto [cookie, inflight] = CreateInFlightRequest(
TRequestInfo(ev->Sender, ev->Cookie, msg->CallContext),
Expand Down
2 changes: 1 addition & 1 deletion cloud/filestore/libs/storage/service/service_actor_listnodes.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ void TListNodesActor::HandleGetNodeAttrResponse(
return;
}

LOG_INFO(
LOG_DEBUG(
ctx,
TFileStoreComponents::SERVICE,
"GetNodeAttrResponse from follower: %s",
Expand Down
76 changes: 76 additions & 0 deletions cloud/filestore/libs/storage/service/service_ut.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3144,6 +3144,82 @@ Y_UNIT_TEST_SUITE(TStorageServiceTest)
data.Size())->Record;
UNIT_ASSERT_VALUES_EQUAL(data, readDataResponse.GetBuffer());
}

Y_UNIT_TEST(ShouldHandleCreateNodeErrorFromFollowerUponCreateHandleViaLeader)
{
NProto::TStorageConfig config;
config.SetMultiTabletForwardingEnabled(true);
TTestEnv env({}, config);
env.CreateSubDomain("nfs");

ui32 nodeIdx = env.CreateNode("nfs");

const TString fsId = "test";
const auto shard1Id = fsId + "-f1";
const auto shard2Id = fsId + "-f2";

TServiceClient service(env.GetRuntime(), nodeIdx);
service.CreateFileStore(fsId, 1'000);
service.CreateFileStore(shard1Id, 1'000);
service.CreateFileStore(shard2Id, 1'000);

ConfigureFollowers(service, fsId, shard1Id, shard2Id);

auto headers = service.InitSession(fsId, "client");

auto error = MakeError(E_FS_INVALID_SESSION, "bad session");

env.GetRuntime().SetEventFilter(
[&] (TTestActorRuntimeBase&, TAutoPtr<IEventHandle>& event) {
switch (event->GetTypeRewrite()) {
case TEvService::EvCreateNodeResponse: {
auto* msg =
event->Get<TEvService::TEvCreateNodeResponse>();
if (error.GetCode()) {
msg->Record.MutableError()->CopyFrom(error);
}

break;
}
}

return false;
});

const ui64 requestId = 111;

service.SendCreateHandleRequest(
headers,
fsId,
RootNodeId,
"file1",
TCreateHandleArgs::CREATE_EXL,
"",
requestId);

auto response = service.RecvCreateHandleResponse();
UNIT_ASSERT_VALUES_EQUAL_C(
error.GetCode(),
response->GetError().GetCode(),
FormatError(response->GetError()));

error = {};

service.SendCreateHandleRequest(
headers,
fsId,
RootNodeId,
"file1",
TCreateHandleArgs::CREATE_EXL,
"",
requestId);

response = service.RecvCreateHandleResponse();
UNIT_ASSERT_VALUES_EQUAL_C(
error.GetCode(),
response->GetError().GetCode(),
FormatError(response->GetError()));
}
}

} // namespace NCloud::NFileStore::NStorage
14 changes: 14 additions & 0 deletions cloud/filestore/libs/storage/tablet/tablet_actor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -836,6 +836,13 @@ STFUNC(TIndexTabletActor::StateZombie)
IgnoreFunc(TEvLocal::TEvTabletMetrics);
IgnoreFunc(TEvHiveProxy::TEvReassignTabletResponse);

HFunc(
TEvIndexTabletPrivate::TEvNodeCreatedInFollower,
HandleNodeCreatedInFollower);
HFunc(
TEvIndexTabletPrivate::TEvNodeCreatedInFollowerUponCreateHandle,
HandleNodeCreatedInFollowerUponCreateHandle);

default:
HandleUnexpectedEvent(ev, TFileStoreComponents::TABLET);
break;
Expand Down Expand Up @@ -869,6 +876,13 @@ STFUNC(TIndexTabletActor::StateBroken)

IgnoreFunc(TEvHiveProxy::TEvReassignTabletResponse);

HFunc(
TEvIndexTabletPrivate::TEvNodeCreatedInFollower,
HandleNodeCreatedInFollower);
HFunc(
TEvIndexTabletPrivate::TEvNodeCreatedInFollowerUponCreateHandle,
HandleNodeCreatedInFollowerUponCreateHandle);

default:
HandleUnexpectedEvent(ev, TFileStoreComponents::TABLET);
break;
Expand Down
25 changes: 12 additions & 13 deletions cloud/filestore/libs/storage/tablet/tablet_actor_createhandle.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ class TCreateNodeInFollowerUponCreateHandleActor final
const NProto::TNode Attrs;
NProto::THeaders Headers;
NProto::TCreateHandleResponse Response;
NProto::TCreateNodeResponse FollowerResponse;

public:
TCreateNodeInFollowerUponCreateHandleActor(
Expand Down Expand Up @@ -165,8 +164,7 @@ void TCreateNodeInFollowerUponCreateHandleActor::HandleCreateNodeResponse(
FollowerId.c_str(),
FollowerName.c_str());

FollowerResponse = std::move(msg->Record);
Response.MutableNodeAttr()->CopyFrom(FollowerResponse.GetNode());
*Response.MutableNodeAttr() = std::move(*msg->Record.MutableNode());

ReplyAndDie(ctx, {});
}
Expand All @@ -188,13 +186,15 @@ void TCreateNodeInFollowerUponCreateHandleActor::ReplyAndDie(
*Response.MutableError() = std::move(error);
}

// TODO(#1350): reply directly to the client (not to the tablet) upon
// poisoning
// or keep this RequestInfo in the ActiveTransactions list until this event
// gets processed by the tablet
using TResponse =
TEvIndexTabletPrivate::TEvNodeCreatedInFollowerUponCreateHandle;
ctx.Send(ParentId, std::make_unique<TResponse>(
std::move(RequestInfo),
std::move(FollowerResponse),
std::move(Response)
));
std::move(Response)));

Die(ctx);
}
Expand Down Expand Up @@ -500,6 +500,10 @@ void TIndexTabletActor::CompleteTx_CreateHandle(
{
RemoveTransaction(*args.RequestInfo);

if (!HasError(args.Error)) {
CommitDupCacheEntry(args.SessionId, args.RequestId);
}

if (args.IsNewFollowerNode && !HasError(args.Error)) {
LOG_INFO(ctx, TFileStoreComponents::TABLET,
"%s Creating node in follower upon CreateHandle: %s, %s",
Expand Down Expand Up @@ -527,7 +531,6 @@ void TIndexTabletActor::CompleteTx_CreateHandle(
std::make_unique<TEvService::TEvCreateHandleResponse>(args.Error);

if (!HasError(args.Error)) {
CommitDupCacheEntry(args.SessionId, args.RequestId);
response->Record = std::move(args.Response);
}

Expand All @@ -547,12 +550,8 @@ void TIndexTabletActor::HandleNodeCreatedInFollowerUponCreateHandle(
{
auto* msg = ev->Get();

auto response = std::make_unique<TEvService::TEvCreateHandleResponse>(
msg->FollowerCreateNodeResponse.GetError());

if (!HasError(response->GetError())) {
response->Record = std::move(msg->CreateHandleResponse);
}
auto response = std::make_unique<TEvService::TEvCreateHandleResponse>();
response->Record = std::move(msg->CreateHandleResponse);

CompleteResponse<TEvService::TCreateHandleMethod>(
response->Record,
Expand Down
17 changes: 4 additions & 13 deletions cloud/filestore/libs/storage/tablet/tablet_actor_createnode.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ class TCreateNodeInFollowerActor final
const TActorId ParentId;
NProto::TCreateNodeRequest Request;
NProto::TCreateNodeResponse Response;
NProto::TCreateNodeResponse FollowerResponse;

public:
TCreateNodeInFollowerActor(
Expand Down Expand Up @@ -186,7 +185,7 @@ void TCreateNodeInFollowerActor::HandleCreateNodeResponse(
FollowerId.c_str(),
FollowerName.c_str());

FollowerResponse = std::move(msg->Record);
*Response.MutableNode() = std::move(*msg->Record.MutableNode());

ReplyAndDie(ctx, {});
}
Expand All @@ -212,9 +211,7 @@ void TCreateNodeInFollowerActor::ReplyAndDie(
using TResponse = TEvIndexTabletPrivate::TEvNodeCreatedInFollower;
ctx.Send(ParentId, std::make_unique<TResponse>(
std::move(RequestInfo),
std::move(FollowerResponse),
std::move(Response)
));
std::move(Response)));

Die(ctx);
}
Expand Down Expand Up @@ -520,14 +517,8 @@ void TIndexTabletActor::HandleNodeCreatedInFollower(
{
auto* msg = ev->Get();

auto response = std::make_unique<TEvService::TEvCreateNodeResponse>(
msg->FollowerCreateNodeResponse.GetError());

if (!HasError(response->GetError())) {
response->Record = std::move(msg->CreateNodeResponse);
*response->Record.MutableNode() =
std::move(*msg->FollowerCreateNodeResponse.MutableNode());
}
auto response = std::make_unique<TEvService::TEvCreateNodeResponse>();
response->Record = std::move(msg->CreateNodeResponse);

CompleteResponse<TEvService::TCreateNodeMethod>(
response->Record,
Expand Down
2 changes: 2 additions & 0 deletions cloud/filestore/libs/storage/tablet/tablet_actor_destroysession.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ void TIndexTabletActor::CompleteTx_DestroySession(
{
RemoveTransaction(*args.RequestInfo);

// TODO(#1350): destroy follower sessions

auto response = std::make_unique<TEvIndexTablet::TEvDestroySessionResponse>();
NCloud::Reply(ctx, *args.RequestInfo, std::move(response));
}
Expand Down
1 change: 1 addition & 0 deletions cloud/filestore/libs/storage/tablet/tablet_actor_unlinknode.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ void TIndexTabletActor::ExecuteTx_UnlinkNode(

// TODO(#1350): unlink external nodes
if (args.ChildRef->FollowerId) {
// XXX ref not unlinked as well
return;
}

Expand Down
6 changes: 0 additions & 6 deletions cloud/filestore/libs/storage/tablet/tablet_private.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -502,15 +502,12 @@ struct TEvIndexTabletPrivate
struct TNodeCreatedInFollower
{
TRequestInfoPtr RequestInfo;
NProto::TCreateNodeResponse FollowerCreateNodeResponse;
NProto::TCreateNodeResponse CreateNodeResponse;

TNodeCreatedInFollower(
TRequestInfoPtr requestInfo,
NProto::TCreateNodeResponse followerCreateNodeResponse,
NProto::TCreateNodeResponse createNodeResponse)
: RequestInfo(std::move(requestInfo))
, FollowerCreateNodeResponse(std::move(followerCreateNodeResponse))
, CreateNodeResponse(std::move(createNodeResponse))
{
}
Expand All @@ -523,15 +520,12 @@ struct TEvIndexTabletPrivate
struct TNodeCreatedInFollowerUponCreateHandle
{
TRequestInfoPtr RequestInfo;
NProto::TCreateNodeResponse FollowerCreateNodeResponse;
NProto::TCreateHandleResponse CreateHandleResponse;

TNodeCreatedInFollowerUponCreateHandle(
TRequestInfoPtr requestInfo,
NProto::TCreateNodeResponse followerCreateNodeResponse,
NProto::TCreateHandleResponse createHandleResponse)
: RequestInfo(std::move(requestInfo))
, FollowerCreateNodeResponse(std::move(followerCreateNodeResponse))
, CreateHandleResponse(std::move(createHandleResponse))
{
}
Expand Down
Loading

0 comments on commit 3b25ff5

Please sign in to comment.