Skip to content

[Filestore] issue-3687: implement pagination during ListNodes for local filestore #3691

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.

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions cloud/filestore/config/server.proto
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,9 @@ message TLocalServiceConfig

// Trust guest kernel to check user permissions for fuse operations
optional bool GuestOnlyPermissionsCheckEnabled = 21;

// Max entries each ListNodes will return
optional uint32 MaxResponseEntries = 22;
}

////////////////////////////////////////////////////////////////////////////////
Expand Down
1 change: 1 addition & 0 deletions cloud/filestore/libs/service_local/config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ constexpr TDuration AsyncHandleOpsPeriod = TDuration::MilliSeconds(50);
xxx(ServerWriteBackCacheEnabled, bool, false )\
xxx(DontPopulateNodeCacheWhenListingNodes, bool, false )\
xxx(GuestOnlyPermissionsCheckEnabled, bool, false )\
xxx(MaxResponseEntries, ui32, 10000 )\
// FILESTORE_SERVICE_CONFIG

#define FILESTORE_SERVICE_DECLARE_CONFIG(name, type, value) \
Expand Down
2 changes: 2 additions & 0 deletions cloud/filestore/libs/service_local/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ class TLocalFileStoreConfig
bool GetDontPopulateNodeCacheWhenListingNodes() const;

bool GetGuestOnlyPermissionsCheckEnabled() const;

ui32 GetMaxResponseEntries() const;
};

} // namespace NCloud::NFileStore
13 changes: 12 additions & 1 deletion cloud/filestore/libs/service_local/fs_node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <cloud/filestore/libs/diagnostics/critical_events.h>

#include <util/generic/guid.h>
#include <util/stream/format.h>

namespace NCloud::NFileStore {

Expand Down Expand Up @@ -205,7 +206,16 @@ NProto::TListNodesResponse TLocalFileSystem::ListNodes(
return TErrorResponse(ErrorInvalidParent(request.GetNodeId()));
}

auto entries = parent->List();
uint64_t offset = 0;
auto& cookie = request.GetCookie();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

почему не  const?

if (cookie) {
if (!TryFromString(cookie, offset)) {
return TErrorResponse(ErrorInvalidArgument());
}
}

auto listRes = parent->List(offset, Config->GetMaxResponseEntries());
auto& entries = listRes.DirEntries;

NProto::TListNodesResponse response;
response.MutableNames()->Reserve(entries.size());
Expand Down Expand Up @@ -237,6 +247,7 @@ NProto::TListNodesResponse TLocalFileSystem::ListNodes(
ConvertStats(entry.second, *response.MutableNodes()->Add());
}

response.SetCookie(ToString(listRes.DirOffset));
return response;
}

Expand Down
16 changes: 14 additions & 2 deletions cloud/filestore/libs/service_local/index.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,21 @@ TString TIndexNode::ReadLink() const
return NLowLevel::ReadLink(NodeFd);
}

TVector<std::pair<TString, TFileStat>> TIndexNode::List(bool ignoreErrors)
TVector<NLowLevel::TDirEntry> TIndexNode::List(bool ignoreErrors)
{
return NLowLevel::ListDirAt(NodeFd, ignoreErrors);
auto res = NLowLevel::ListDirAt(
NodeFd,
0, // start at beginning of dir
0, // don't limit number of entries
ignoreErrors);

return std::move(res.DirEntries);
}

NLowLevel::TListDirResult
TIndexNode::List(uint64_t offset, size_t entriesLimit, bool ignoreErrors)
{
return NLowLevel::ListDirAt(NodeFd, offset, entriesLimit, ignoreErrors);
}

TFileStat TIndexNode::Stat()
Expand Down
4 changes: 3 additions & 1 deletion cloud/filestore/libs/service_local/index.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ class TIndexNode
TIndexNodePtr CreateSymlink(const TString& name, const TString& target);
TIndexNodePtr CreateSocket(const TString& name, int flags);

TVector<std::pair<TString, TFileStat>> List(bool ignoreErrors = false);
TVector<NLowLevel::TDirEntry> List(bool ignoreErrors = false);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а может не делать дефолты в параметрах?

NLowLevel::TListDirResult
List(uint64_t offset, size_t entriesLimit, bool ignoreErrors = false);

void Rename(
const TString& name,
Expand Down
28 changes: 23 additions & 5 deletions cloud/filestore/libs/service_local/lowlevel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -297,8 +297,10 @@ TFileSystemStat StatFs(const TFileHandle& handle)
return GetFileSystemStat(fs);
}

TVector<std::pair<TString, TFileStat>> ListDirAt(
TListDirResult ListDirAt(
const TFileHandle& handle,
uint64_t offset,
size_t entriesLimit,
bool ignoreErrors)
{
auto fd = openat(Fd(handle), ".", O_RDONLY);
Expand All @@ -320,7 +322,11 @@ TVector<std::pair<TString, TFileStat>> ListDirAt(
}
};

TVector<std::pair<TString, TFileStat>> results;
if (offset) {
seekdir(dir, offset);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А я правильно понимаю, что opendir не переживает сейчас рестарты nfs-vhost в local режиме?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

да у нас есть issue на эту тему #2390

}

TListDirResult res;

errno = 0;
while (auto* entry = readdir(dir)) {
Expand All @@ -332,22 +338,34 @@ TVector<std::pair<TString, TFileStat>> ListDirAt(
if (ignoreErrors) {
try {
auto stat = StatAt(handle, name);
results.emplace_back(std::move(name), stat);
res.DirEntries.emplace_back(std::move(name), stat);
} catch (const TServiceError& err) {
errno = 0;
continue;
}
} else {
auto stat = StatAt(handle, name);
results.emplace_back(std::move(name), stat);
res.DirEntries.emplace_back(std::move(name), stat);
}

if (entriesLimit && --entriesLimit == 0) {
break;
}
}

Y_ENSURE_EX(errno == 0, TServiceError(GetSystemErrorCode())
<< "failed to list: "
<< LastSystemErrorText());

return results;
long loc = telldir(dir);

Y_ENSURE_EX(loc != -1, TServiceError(GetSystemErrorCode())
<< "failed to telldir: "
<< LastSystemErrorText());

res.DirOffset = loc;

return res;
}

////////////////////////////////////////////////////////////////////////////////
Expand Down
14 changes: 13 additions & 1 deletion cloud/filestore/libs/service_local/lowlevel.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,16 @@ struct TOpenOrCreateResult

////////////////////////////////////////////////////////////////////////////////

using TDirEntry = std::pair<TString, TFileStat>;

struct TListDirResult
{
TVector<TDirEntry> DirEntries;
uint64_t DirOffset = 0;
};

////////////////////////////////////////////////////////////////////////////////

TFileHandle Open(const TString& path, int flags, int mode);
TFileHandle Open(const TFileHandle& handle, int flags, int mode);
TFileHandle OpenAt(
Expand Down Expand Up @@ -144,8 +154,10 @@ TFileStat Stat(const TFileHandle& handle);
TFileStat StatAt(const TFileHandle& handle, const TString& name);
TFileSystemStat StatFs(const TFileHandle& handle);

TVector<std::pair<TString, TFileStat>> ListDirAt(
TListDirResult ListDirAt(
const TFileHandle& handle,
uint64_t offset,
size_t entriesLimit,
bool ignoreErrors);

//
Expand Down
56 changes: 56 additions & 0 deletions cloud/filestore/libs/service_local/lowlevel_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,62 @@ Y_UNIT_TEST_SUITE(TLowlevelTest)
res = NLowLevel::OpenOrCreateAt(node, "2.txt", O_WRONLY, 0755);
UNIT_ASSERT(!res.WasCreated);
}

Y_UNIT_TEST(ShouldDoIterativeListDir)
{
const TTempDir TempDir;
auto rootNode = NLowLevel::Open(TempDir.Name(), O_PATH, 0);

int nodesCount = 10;
TSet<TString> entryNames;
for (int i = 0; i < nodesCount; i++) {
if (i == 0) {
auto name = "dir_" + ToString(i);
entryNames.insert(name);
NLowLevel::MkDirAt(rootNode, name, 0755);
} else {
auto name = "file_" + ToString(i);
entryNames.insert(name);
NLowLevel::OpenAt(
rootNode,
name,
O_CREAT | O_WRONLY,
0755);
}
}

auto savedEntryNames = entryNames;
auto checkListDirResult =
[&](NLowLevel::TListDirResult& res, size_t expectedEntriesCount)
{
UNIT_ASSERT_EQUAL(res.DirEntries.size(), expectedEntriesCount);
for (auto& entry: res.DirEntries) {
UNIT_ASSERT_EQUAL_C(
1,
entryNames.count(entry.first),
TStringBuilder() << entry.first << " missing");
entryNames.erase(entry.first);
}
};

auto res = NLowLevel::ListDirAt(rootNode, 0, 5, false);
checkListDirResult(res, 5);

res = NLowLevel::ListDirAt(rootNode, res.DirOffset, 3, false);
checkListDirResult(res, 3);

res = NLowLevel::ListDirAt(rootNode, res.DirOffset, 2, false);
checkListDirResult(res, 2);

res = NLowLevel::ListDirAt(rootNode, res.DirOffset, 2, false);
checkListDirResult(res, 0);

// with limit 0 all nodes should be read
entryNames = savedEntryNames;
res = NLowLevel::ListDirAt(rootNode, 0, 0, false);
checkListDirResult(res, 10);

}
};

} // namespace NCloud::NFileStore
2 changes: 1 addition & 1 deletion cloud/filestore/libs/service_local/session.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ class TSession
} catch (...) {
STORAGE_ERROR(
"Failed to open Handle, HandleId=" << it->HandleId <<
", NodeId" << it->NodeId <<
", NodeId=" << it->NodeId <<
", Exception=" << CurrentExceptionMessage());
HandleTable->DeleteRecord(it.GetIndex());
continue;
Expand Down
Loading