`ls` fails every second run | `rewinddir` support about mountpoint-s3 HOT 11 CLOSED

Hello @luiscape, thanks for reporting this. I was unable to reproduce this issue. Can you provide the full log file?

Thanks!

from mountpoint-s3.

@arsh I missed a clarification: We run mount-s3 on the host but expose the mount as a bind mount inside a container (aka "sandbox") in gVisor. gVisor will reuse and rewind dir handles. Maybe mountdir isn't handling rewinddir correctly?

I am able to reproduce the issue on the issue with the following C++ program (listdir.cpp):

#include <stdio.h>
#include <dirent.h>

int main () {
  DIR *dh = opendir("/my-mount");
  if (dh == NULL) {
    printf("failed to open dir\n");
    return -1;
  }

  struct dirent *dirent;
  while ((dirent = readdir(dh)) != NULL) {
    printf("%s\n", dirent->d_name);
  }
  
  printf("rewinding\n");
  rewinddir(dh);
  
  while ((dirent = readdir(dh)) != NULL) {
    printf("%s\n", dirent->d_name);
  }
  
  closedir(dh);
  return 0;
}

Compiling and running

$ g++ -o listdir listdir.cpp && ./listdir

will reproduce the issue:

2024-03-19T21:00:43.425308Z  WARN readdirplus{req=228 ino=1 fh=9 offset=0}: mountpoint_s3::fuse: readdirplus failed: out-of-order readdir, expected=166, actual=0

(cc @gimaker)

Also sending full log file attached.

mountpoint.log

from mountpoint-s3.

Thanks for the context. In the past, we added code in

From running the repro, in my directory I can see the following offsets are requested:

1. 0 - initial read
2. 7 - I have 8 files in my test, so this is the call that returns that there is nothing more
3. 0 - after rewind

Since there is call with offset 7, that is why we get the out-of-order error. However, I'm still unclear how that was not the case in the other cases (fixed by that code I shared).

from mountpoint-s3.

A fix for this has been merged.

#825

from mountpoint-s3.

You will need to build from main until the next release. Please let us know if you find any issues.

from mountpoint-s3.

For context, rewinddir() will result in a FUSE_READDIR/FUSE_READDIRPLUS request with offset=0 being sent to the FUSE fileserver.

from mountpoint-s3.

@luiscape is your use case exclusively rewinding (starting from offset 0) and then doing a sequential list? or does it include random seeks on the dir stream?

from mountpoint-s3.

I noticed this issue when walking the root directory twice in a row (ie calling ls). I haven't noticed this issue on random seeks like you mention.

from mountpoint-s3.

@arsh Note that rewinddir(dirp) is subtly but crucially different from seekdir(dirp, 0). Quoting from readdir specs:

If a file is removed from or added to the directory after the most recent call to opendir() or rewinddir(), whether a subsequent call to readdir() returns an entry for that file is unspecified.

(Though as far as I know, from a FUSE perspective there's no way to distinguish a rewindir(dirp) and seekdir(dirp, 0))

rewinddir() should effectively behave as calling opendir() again, resetting the directory stream. Any files that were added or removed between the opendir() and the rewinddir() should be reflect in subsequent calls to readdir().

seekdir() doesn't have the same constraints - readdir() may omit files that were added after opendir() and after you call seekdir() (and may include files that have been deleted).

I am unfamiliar with the mountpoint-s3 code base so I don't know if this is a problem in practice, but I imagine it could be. The C snippet below illustrates the issue:

#include <stdio.h>
#include <dirent.h>
#include <assert.h>
#include <string.h>

int main () {
  DIR *dh = opendir("/my-mount");
  if (dh == NULL) {
    printf("failed to open dir\n");
    return -1;
  }

  struct dirent *dirent;
  while ((dirent = readdir(dh)) != NULL) {
    printf("%s\n", dirent->d_name);
  }
  
  // Create new file
  FILE *f = fopen("/my-mount/this-is-new-file", "w");
  if (f == NULL) {
    printf("failed to open file\n");
    return -1;
  }
  fwrite("hello", 5, 1, f);
  fclose(f);

  printf("rewinding\n");
  rewinddir(dh);

  // Newly created file must be returned by readdir() after calling rewinddir()
  int hello_found = 0;
  while ((dirent = readdir(dh)) != NULL) {
    if (strcmp("this-is-new-file", dirent->d_name) == 0) {
      hello_found = 1;
    }
    printf("%s\n", dirent->d_name);
  }

  assert(hello_found == 1);
  
  closedir(dh);
  return 0;
}

from mountpoint-s3.

@arsh awesome! How can we give it a try? Would you have a build link or should we build from main?

from mountpoint-s3.

Issue is now fixed. Thanks a lot!

from mountpoint-s3.