[Filestore] issue-3687: implement pagination during ListNodes for local filestore #3691
Conversation
|
Note This is an automated comment that will be appended during run. 🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit ab547e6.
🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit ab547e6.
|
| @@ -123,6 +123,9 @@ message TLocalServiceConfig | |||
|
|
|||
| // Trust guest kernel to check user permissions for fuse operations | |||
| optional bool GuestOnlyPermissionsCheckEnabled = 21; | |||
|
|
|||
| // Max entries each list nodes will return | |||
| struct TListDirResult | ||
| { | ||
| TVector<std::pair<TString, TFileStat>> DirEntries; | ||
| uint64_t DirOffset; |
There was a problem hiding this comment.
давай проинициализируем (на всякий случай)
| @@ -105,6 +105,14 @@ struct TOpenOrCreateResult | |||
|
|
|||
| //////////////////////////////////////////////////////////////////////////////// | |||
|
|
|||
| struct TListDirResult | |||
| { | |||
| TVector<std::pair<TString, TFileStat>> DirEntries; | |||
| } | ||
| } | ||
|
|
||
| res.DirOffset = telldir(dir); |
There was a problem hiding this comment.
telldir может вернуть ошибку
в этом случае вылетит исключение ниже?
это не отравит нам работу с другими запросами/фсками?
| @@ -123,6 +123,9 @@ message TLocalServiceConfig | |||
|
|
|||
| // Trust guest kernel to check user permissions for fuse operations | |||
| optional bool GuestOnlyPermissionsCheckEnabled = 21; | |||
|
|
|||
| // Max entries each list nodes will return | |||
| optional uint32 MaxEntriesPerListNodes = 22; | |||
There was a problem hiding this comment.
Предлагаю консистентный нейминг этого параметра:
| uint64_t offset = 0; | ||
| auto& cookie = request.GetCookie(); | ||
| if (cookie) { | ||
| offset = FromString(cookie); |
There was a problem hiding this comment.
TryFromString + обработка ошибки?
| return NLowLevel::ListDirAt(NodeFd, ignoreErrors); | ||
| auto res = NLowLevel::ListDirAt( | ||
| NodeFd, | ||
| 0, // start at begining of dir |
| @@ -320,7 +322,11 @@ TVector<std::pair<TString, TFileStat>> ListDirAt( | |||
| } | |||
| }; | |||
|
|
|||
| TVector<std::pair<TString, TFileStat>> results; | |||
| if (offset) { | |||
| seekdir(dir, offset); | |||
There was a problem hiding this comment.
А я правильно понимаю, что opendir не переживает сейчас рестарты nfs-vhost в local режиме?
| @@ -237,6 +245,7 @@ NProto::TListNodesResponse TLocalFileSystem::ListNodes( | |||
| ConvertStats(entry.second, *response.MutableNodes()->Add()); | |||
| } | |||
|
|
|||
| response.SetCookie(ToString(listRes.DirOffset)); | |||
There was a problem hiding this comment.
https://libfuse.github.io/doxygen/structfuse__lowlevel__ops.html#a65b7d7fc14d3958d7fb7d215fda20301
However, addition or removal of entries must never cause readdir() to skip over unrelated entries or to report them more than once. This means that off_t can not be a simple index that enumerates the entries that have been returned but must contain sufficient information to uniquely determine the next directory entry to return even when the set of entries is changing.
| } | ||
| }; | ||
|
|
||
| auto res = NLowLevel::ListDirAt(rootNode, 0, 5, false); |
| @@ -58,6 +58,55 @@ Y_UNIT_TEST_SUITE(TLowlevelTest) | |||
| res = NLowLevel::OpenOrCreateAt(node, "2.txt", O_WRONLY, 0755); | |||
| UNIT_ASSERT(!res.WasCreated); | |||
| } | |||
|
|
|||
| Y_UNIT_TEST(ShouldDoIterativeListDir) | |||
There was a problem hiding this comment.
хочется запускать эти тесты с новыми флажками (фичами) https://github.com/ydb-platform/nbs/blob/main/cloud/filestore/tests/fs_posix_compliance/qemu-local-test/test.py
по аналогии с https://github.com/ydb-platform/nbs/blob/main/cloud/filestore/tests/common_configs/nfs-storage-newfeatures-patch.txt
There was a problem hiding this comment.
оказалось что тут нет тестов про ls https://github.com/ydb-platform/nbs/tree/d3f0bb19a12ff4f4de19edded26b05bc0620713c/cloud/filestore/tools/testing/fs_posix_compliance/suite/tests
@debnatkh, надо добавить
#3687