Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Filestore] issue-2171: Avoid listing nodes with pending create on shards #2611

Merged
merged 1 commit into from
Dec 18, 2024
Merged

[Filestore] issue-2171: Avoid listing nodes with pending create on shards #2611

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
152 changes: 152 additions & 0 deletions cloud/filestore/libs/storage/service/service_ut_sharding.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3855,6 +3855,158 @@ Y_UNIT_TEST_SUITE(TStorageServiceShardingTest)
Sort(ids);
UNIT_ASSERT_VALUES_EQUAL(expected, ids);
}

void CheckPendingCreateNodeInShards(bool withCreateHandle, bool withTabletReboot)
{
NProto::TStorageConfig config;
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);

ConfigureShards(service, fsId, shard1Id, shard2Id);

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

bool createNodeObserved = false;
bool createHandleObserved = false;
TAutoPtr<IEventHandle> createNodeOnShard;
ui64 tabletId = -1;

auto& runtime = env.GetRuntime();
runtime.SetEventFilter(
[&](auto& runtime, TAutoPtr<IEventHandle>& event)
{
Y_UNUSED(runtime);
switch (event->GetTypeRewrite()) {
case TEvService::EvCreateHandleRequest:
createHandleObserved = true;
break;
case TEvService::EvCreateNodeRequest: {
const auto* msg =
event->Get<TEvService::TEvCreateNodeRequest>();
if (msg->Record.GetShardFileSystemId().empty()) {
createNodeOnShard = event.Release();
return true;
}
createNodeObserved = true;

break;
}
case TEvSSProxy::EvDescribeFileStoreResponse: {
using TDesc = TEvSSProxy::TEvDescribeFileStoreResponse;
const auto* msg = event->Get<TDesc>();
const auto& desc =
msg->PathDescription.GetFileStoreDescription();
if (desc.GetConfig().GetFileSystemId() == fsId) {
tabletId = desc.GetIndexTabletId();
}
break;
}
}
return false;
});

auto checkListNodes = [&](auto expectedNames) {
auto listNodesRsp =
service.ListNodes(headers, fsId, RootNodeId)->Record;

UNIT_ASSERT_VALUES_EQUAL(expectedNames.size(), listNodesRsp.NamesSize());
UNIT_ASSERT_VALUES_EQUAL(expectedNames.size(), listNodesRsp.NodesSize());

const auto& names = listNodesRsp.GetNames();
TVector<TString> listedNames(names.begin(), names.end());
Sort(listedNames);
UNIT_ASSERT_VALUES_EQUAL(expectedNames, listedNames);
};

// send create node/handle, delay response in shard and make sure node is not
// listed
if (!withCreateHandle) {
service.SendCreateNodeRequest(
headers,
TCreateNodeArgs::File(
RootNodeId,
"file1",
0, // mode
shard1Id));
} else {
service.SendCreateHandleRequest(
headers,
fsId,
RootNodeId,
"file1",
TCreateHandleArgs::CREATE,
shard1Id);
}
runtime.DispatchEvents(TDispatchOptions(), TDuration::Seconds(1));

if (withCreateHandle) {
UNIT_ASSERT(!createNodeObserved);
UNIT_ASSERT(createHandleObserved);
} else {
UNIT_ASSERT(createNodeObserved);
UNIT_ASSERT(!createHandleObserved);
}
UNIT_ASSERT(createNodeOnShard);

if (withTabletReboot) {
TIndexTabletClient tablet(env.GetRuntime(), nodeIdx, tabletId);
tablet.RebootTablet();

headers = service.InitSession(fsId, "client", {}, true);
}

checkListNodes(TVector<TString>{});

// send delayed response in shard and make sure node is listed
auto createNodeRsp =
std::make_unique<TEvService::TEvCreateNodeResponse>();
runtime.Send(
new IEventHandle(
createNodeOnShard->Sender,
createNodeOnShard->Recipient,
createNodeRsp.release(),
0, // flags
createNodeOnShard->Cookie),
nodeIdx);
runtime.DispatchEvents(TDispatchOptions(), TDuration::Seconds(1));

checkListNodes(TVector<TString>{"file1"});
}

Y_UNIT_TEST(ShouldNotListPendingCreateNodeInShards)
{
CheckPendingCreateNodeInShards(
false, // don't create handle
false // don't reboot tablet
);
CheckPendingCreateNodeInShards(
false, // don't create handle
true // reboot tablet
);
}

Y_UNIT_TEST(ShouldNotListPendingCreateHandleInShards)
{
CheckPendingCreateNodeInShards(
true, // create handle
false // don't reboot tablet
);
CheckPendingCreateNodeInShards(
true, // create handle
true // reboot tablet
);
}
}

} // namespace NCloud::NFileStore::NStorage
7 changes: 6 additions & 1 deletion cloud/filestore/libs/storage/tablet/tablet_actor_createnode.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ class TCreateNodeInShardActor final
const TString LogTag;
TRequestInfoPtr RequestInfo;
const TActorId ParentId;
const NProto::TCreateNodeRequest Request;
NProto::TCreateNodeRequest Request;
const ui64 RequestId;
const ui64 OpLogEntryId;
TCreateNodeInShardResult Result;
Expand Down Expand Up @@ -263,6 +263,7 @@ void TCreateNodeInShardActor::ReplyAndDie(
Request.GetHeaders().GetSessionId(),
RequestId,
OpLogEntryId,
std::move(*Request.MutableName()),
std::move(Result)));

Die(ctx);
Expand Down Expand Up @@ -649,6 +650,8 @@ void TIndexTabletActor::HandleNodeCreatedInShard(

WorkerActors.erase(ev->Sender);

EndNodeCreateInShard(msg->NodeName);

if (auto* x = std::get_if<NProto::TCreateNodeResponse>(&res)) {
if (msg->RequestInfo) {
auto response =
Expand Down Expand Up @@ -759,6 +762,8 @@ void TIndexTabletActor::RegisterCreateNodeInShardActor(
ui64 opLogEntryId,
TCreateNodeInShardResult result)
{
StartNodeCreateInShard(request.GetName());

auto actor = std::make_unique<TCreateNodeInShardActor>(
LogTag,
std::move(requestInfo),
Expand Down
13 changes: 8 additions & 5 deletions cloud/filestore/libs/storage/tablet/tablet_actor_listnodes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,11 +195,14 @@ void TIndexTabletActor::CompleteTx_ListNodes(
const auto& ref = args.ChildRefs[i];
requestBytes += ref.Name.size();
if (ref.ShardId) {
AddExternalNode(
record,
ref.Name,
ref.ShardId,
ref.ShardNodeName);
if (!HasPendingNodeCreateInShard(ref.ShardNodeName)) {
AddExternalNode(
record,
ref.Name,
ref.ShardId,
ref.ShardNodeName);

}

continue;
}
Expand Down
3 changes: 3 additions & 0 deletions cloud/filestore/libs/storage/tablet/tablet_private.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -576,18 +576,21 @@ struct TEvIndexTabletPrivate
const TString SessionId;
const ui64 RequestId;
const ui64 OpLogEntryId;
const TString NodeName;
TCreateNodeInShardResult Result;

TNodeCreatedInShard(
TRequestInfoPtr requestInfo,
TString sessionId,
ui64 requestId,
ui64 opLogEntryId,
TString nodeName,
TCreateNodeInShardResult result)
: RequestInfo(std::move(requestInfo))
, SessionId(std::move(sessionId))
, RequestId(requestId)
, OpLogEntryId(opLogEntryId)
, NodeName(std::move(nodeName))
, Result(std::move(result))
{
}
Expand Down
6 changes: 6 additions & 0 deletions cloud/filestore/libs/storage/tablet/tablet_state.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,12 @@ FILESTORE_FILESYSTEM_STATS(FILESTORE_DECLARE_COUNTER)
const TString& message,
ui64 nodeId);

bool HasPendingNodeCreateInShard(const TString& nodeName) const;

void StartNodeCreateInShard(const TString& nodeName);

void EndNodeCreateInShard(const TString& nodeName);

private:
void UpdateUsedBlocksCount(
TIndexTabletDatabase& db,
Expand Down
1 change: 1 addition & 0 deletions cloud/filestore/libs/storage/tablet/tablet_state_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ struct TIndexTabletState::TImpl
TNodeIndexCache NodeIndexCache;
TInMemoryIndexState InMemoryIndexState;
TSet<ui64> OrphanNodeIds;
TSet<TString> PendingNodeCreateInShardNames;

TCheckpointStore Checkpoints;
TChannels Channels;
Expand Down
16 changes: 16 additions & 0 deletions cloud/filestore/libs/storage/tablet/tablet_state_nodes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,22 @@ void TIndexTabletState::WriteOrphanNode(
Impl->OrphanNodeIds.insert(nodeId);
}

bool TIndexTabletState::HasPendingNodeCreateInShard(const TString& nodeName) const
{
return Impl->PendingNodeCreateInShardNames.contains(nodeName);
}

void TIndexTabletState::StartNodeCreateInShard(const TString& nodeName)
{
Impl->PendingNodeCreateInShardNames.insert(nodeName);
}

void TIndexTabletState::EndNodeCreateInShard(const TString& nodeName)
{
Impl->PendingNodeCreateInShardNames.erase(nodeName);
}


////////////////////////////////////////////////////////////////////////////////
// NodeAttrs

Expand Down
Loading