Azure / azure-storage-fuse

A virtual file system adapter for Azure Blob storage
Other
659 stars 207 forks source link

Calling lseek(fd, 0, SEEK_SET) does not reset pagination token when reading directory in AzStorage #1221

Closed martin-heralecky closed 1 year ago

martin-heralecky commented 1 year ago

Which version of blobfuse was used?

d37cee6 (2.0.5)

Which OS distribution and version are you using?

Ubuntu 22.04.1

What was the issue encountered?

When reading directory contents, if a lseek(fd, 0, SEEK_SET) call occurs, it is ignored and pagination token is not reset.

Example scenario:

  1. AzStorage::StreamDir(offset 0, count 5000) is called and returns entries 0-5000.
  2. lseek(fd, 0, SEEK_SET) is called, which (I presume) is ignored by blobfuse.
  3. AzStorage::StreamDir(offset 0, count 5000) is called again, but uses the old token, so returns entries 5000-10000 instead of 0-5000.

Please share logs if available.

LOG_TRACE [libfuse_handler.go (447)]: Libfuse::libfuse_opendir : <path>
LOG_TRACE [attr_cache.go (273)]: AttrCache::StreamDir : <path>
LOG_TRACE [azstorage.go (284)]: AzStorage::StreamDir : Path <path>, offset 0, count 5000
LOG_TRACE [block_blob.go (540)]: BlockBlob::List : prefix <path>, marker
LOG_DEBUG [azstorage.go (305)]: AzStorage::StreamDir : Retrieved 5000 objects with  marker for Path <path>
LOG_DEBUG [azstorage.go (308)]: AzStorage::StreamDir : next-marker <token> for Path <path>
LOG_TRACE [attr_cache.go (273)]: AttrCache::StreamDir : <path>

# lseek(0, SEEK_SET) called here. next StreamDir has offset 0 (again)

LOG_TRACE [azstorage.go (284)]: AzStorage::StreamDir : Path <path>, offset 0, count 5000
LOG_TRACE [block_blob.go (540)]: BlockBlob::List : prefix <path>, marker <token>
LOG_DEBUG [azstorage.go (305)]: AzStorage::StreamDir : Retrieved 5000 objects with <token> marker for Path <path>
LOG_DEBUG [azstorage.go (308)]: AzStorage::StreamDir : next-marker <token> for Path <path>
LOG_TRACE [attr_cache.go (273)]: AttrCache::StreamDir : <path>
LOG_TRACE [azstorage.go (284)]: AzStorage::StreamDir : Path <path>, offset 5000, count 5000
LOG_TRACE [block_blob.go (540)]: BlockBlob::List : prefix <path>, marker <token>
LOG_DEBUG [azstorage.go (305)]: AzStorage::StreamDir : Retrieved 3996 objects with <token> marker for Path <path>
LOG_TRACE [libfuse_handler.go (473)]: Libfuse::libfuse_releasedir : <path>, handle: 7

In this case, the directory contains 13996 entries, but only 8996 was returned after the lseek.

Attached is a C program demonstrating the problem. readdir_with_lseek.txt

vibhansa-msft commented 1 year ago

Thanks for reaching out and submitting such a detailed report with the sample code. From Blobfuse end we do not support SEEK on directory handle. Reason to avoid this is that seeking to 0, you can argue that we need to restart the listing but once allowed user is free to SEEK to any offset and based on that we need to continue the listing. Blobfuse does not store the pagination tokens and hence such SEEK would mean we need to restart from offset 0, reach to that location and get the token and then continue from there the listing part. This might result into excessive calls to storage and will cost you money as the listing APIs will be charged. Also, if there is sort of a loop to SEEK at one location the cost can rise significantly. To avoid such vague scenarios, we avoid supporting the SEEK.

If you have a genuine use-case where you can prove that seek is required, we can investigate from our end. For now, this is by design and SEEK is not supported on directories. We have not seen any customer asking for any such feature as well in past.

martin-heralecky commented 1 year ago

Thank you for your reply.

The reason this bothers us is that now, the de-facto standard way of listing directory contents in PHP is not working:

$dir = new DirectoryIterator($path);
foreach ($dir as $file) {
    echo $file->getFilename();
}

For some reason, this scripts opens a directory, reads one block of entries, then seeks to 0 and then continues reading until end. I'll investigate this further, maybe the faulty behaviour lies on PHP's side.

I understand why you don't want seeking supported, but shouldn't calling lseek at least produce an error? I am not familiar with how FUSE works and if "returning an error" is even possible, but currently, lseek(fd, 0, SEEK_SET) just returns a successful 0, even though the seek is actually not performed.

vibhansa-msft commented 1 year ago

I will investigate on returning the error on lseek on directory. So far I have not seen that but I doubt any such call will end up in blobfuse, most likely in readdir we will just get the offset and nothing more. As I explained earlier honoring these offsets in readdir might be tricky and expensive so we will not go on that path. If LSEEK can be failed, I will investigate on that part.

vibhansa-msft commented 1 year ago

I was looking at libfuse traces and I do not see any callback being generated for LSEEK on dir handle.

 unique: 12, success, outsize: 312
unique: 14, opcode: READDIR (28), nodeid: 2, insize: 80, pid: 26200
readdir[824638855296] from 0
   unique: 14, success, outsize: 312
unique: 16, opcode: READDIR (28), nodeid: 2, insize: 80, pid: 26200
readdir[824638855296] from 0
   unique: 16, success, outsize: 312
unique: 18, opcode: READDIR (28), nodeid: 2, insize: 80, pid: 26200
readdir[824638855296] from 0
   unique: 18, success, outsize: 312
unique: 20, opcode: READDIR (28), nodeid: 2, insize: 80, pid: 26200
readdir[824638855296] from 0
   unique: 20, success, outsize: 312
unique: 22, opcode: READDIR (28), nodeid: 2, insize: 80, pid: 26200
readdir[824638855296] from 0
   unique: 22, success, outsize: 312
unique: 24, opcode: READDIR (28), nodeid: 2, insize: 80, pid: 26200
readdir[824638855296] from 0
   unique: 24, success, outsize: 312
unique: 26, opcode: READDIR (28), nodeid: 2, insize: 80, pid: 26200
readdir[824638855296] from 0

This means there is no way for me to return back an error when the seek operation is done. I see there is nothing we can fix from our side. As a note I can add to readme saying seek on directory handles is not supported. Closing this as there is no action item on the blobfuse end here.

vibhansa-msft commented 1 year ago

Here is a PR to call it out in our README https://github.com/Azure/azure-storage-fuse/pull/1224/files