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.

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

Fix up and test conflicting names behavior #28

Closed
jacobsa opened this issue Mar 26, 2015 · 6 comments
Closed

Fix up and test conflicting names behavior #28

jacobsa opened this issue Mar 26, 2015 · 6 comments
Assignees
Milestone

Comments

@jacobsa
Copy link
Contributor

jacobsa commented Mar 26, 2015

Currently:

  • DirInode.LookUpChild prefers directories over files if there is a conflicting name.
  • DirInode.ReadEntries makes no effort to do the same—a conflicting name will show up twice.

We don't have a good test for this, and I think files should probably be preferred over directories instead—they are more "local" in some sense. Action items:

  • Add a test for this behavior, specifying files are preferred.
  • Make it pass by reversing the behavior.
  • Figure out if we should be attempting to suppress the duplicates in ReadDir. What is the ls user experience?
  • Put this all in the semantics doc, probably under "surprising behavior".
@jacobsa jacobsa self-assigned this Mar 26, 2015
@jacobsa jacobsa added this to the alpha milestone Mar 26, 2015
jacobsa added a commit that referenced this issue Mar 26, 2015
Currently it just nails down the existing behavior. See #28.
jacobsa added a commit that referenced this issue Mar 26, 2015
Now files are preferred over directories. See #28.
jacobsa added a commit that referenced this issue Mar 26, 2015
@jacobsa
Copy link
Contributor Author

jacobsa commented Mar 26, 2015

The ls experience sucks. In a bucket with objects foo and foo/:

jacobsa@jacobsa-macpro:~/go/src/github.com/googlecloudplatform/gcsfuse (conflicting-names)% ls -l ~/tmp/mp
total 0
-rwx------  1 jacobsa  eng  0 Mar 27 10:02 foo
-rwx------  1 jacobsa  eng  0 Mar 27 10:02 foo


jacobsa@jacobsa-macpro:~/go/src/github.com/googlecloudplatform/gcsfuse (conflicting-names)% ls ~/tmp/mp
foo     foo

@jacobsa
Copy link
Contributor Author

jacobsa commented Mar 27, 2015

I talked with Michael about this for a long time. Notes:

  • We should probably go back to the previous behavior and prefer directories over files. This "minimizes the damage" if this comes up, in terms of e.g. rsync's view of the number of files/directories that appear to have disappeared or have been renamed (see below).
  • Michael convinced me that it's a bad idea to just hide things—it's very concerning from the user's point of view, and may not be discovered for awhile if e.g. the user mounts a bucket and then rsyncs from it to make a backup to some other storage mechanism.
  • So instead we should have a conflicting pair result in the file appearing to have been renamed:
    • We can play games with trailing ~ characters or the like, but that's otherwise a legal character and it's deeply confusing how this interacts with actual objects and directories that end in ~, not to mention listing consistency and so on.
    • We could just flat out forbid names that end in ~, then use that as our signal. But that kind of sucks; users might want such files.
    • We already must forbid names containing U+000A (line feed, '\n' in Go), because GCS forbids it. So we can use that as a signal. ls appears to show such characters as ?, which is actually kind of nice: it makes the user want to watch out for something weird going on if they expected a file named foo and see a filed named foo?.

The best mental model for how to implement this I've come up with so far is that this is all just a matter of directory entries, relevant only at ReadDir and LookUpInode time:

  • ReadDir must detect this situation and transform the name in the entry for the file it returns by appending '\n'.
    • The easiest way to implement this: do the entire object listing, transform to DirEnt structs, sort by name, handle adjacent duplicates. Not ideal in terms of memory usage, but much much less likely to have bugs, given the complicated interface for listing (separate objects and collapsed prefixes).
  • LookUpInode operates as usual, except when the name ends in '\n'. In that case, it should recurse into the lookup for the name without that character. If the inner lookup finds a directory, then the outer lookup should return a file inode backed by the object whose name is the stripped version. If the inner lookup finds a file or nothing, then the outer lookup should return ENOENT. In other words, the file named "foo\n" exists only while the directory named "foo" exists.
  • There is no sense in which an inode has a name, so beyond this point it doesn't matter. Things like Go's File.Name method will return the name with which the file is opened, but that's fine. If the conflicting directory disappears than it will simply appear as if the file inode was renamed within the parent directory.

@jacobsa
Copy link
Contributor Author

jacobsa commented Mar 27, 2015

To do for alpha:

  • Reverse the behavior again so that directories win. At the moment the file will be hidden.
  • Put this behavior in the semantics doc, with a callout to this issue, saying it will change soon.
  • Mark this bug as being for beta.

To do for beta:

  • Implement the trailing \n sentinel behavior described above.
  • Update the semantics doc to reflect this.

jacobsa added a commit that referenced this issue Mar 27, 2015
@jacobsa jacobsa modified the milestones: beta, alpha Mar 27, 2015
jacobsa added a commit that referenced this issue Mar 31, 2015
jacobsa added a commit that referenced this issue Mar 31, 2015
@jacobsa
Copy link
Contributor Author

jacobsa commented Mar 31, 2015

Oops, there appears to be some problem with listing the directory in this situation:

jacobsa@jacobsa-macpro:~/tmp% ls -l mp/
total 0
drwx------  1 jacobsa  eng  0 Aug 31  1754 foo
-rwx------  1 jacobsa  eng  0 Mar 31 16:28 foo?


jacobsa@jacobsa-macpro:~/tmp% ls -l mp/foo/
ls: foo: No such file or directory


jacobsa@jacobsa-macpro:~/tmp% ls -l mp/foo/
ls: foo: No such file or directory

Investigate.

@jacobsa jacobsa reopened this Mar 31, 2015
@jacobsa
Copy link
Contributor Author

jacobsa commented Mar 31, 2015

Odd, it looks like the directory is listing itself in its contents:

fuse: 2015/03/31 16:30:49 Received: Lookup [ID=0x1 Node=0x1 Uid=83333 Gid=5000 Pid=88148] "foo"
fuse: 2015/03/31 16:30:50 Responding: Lookup {Node:2 Generation:0 EntryValid:0 AttrValid:0 Attr:{Inode:2 Size:0 Blocks:0 Atime:0001-01-01 00:00:00 +0000 UTC Mtime:0001-01-01 00:00:00 +0000 UTC Ctime:0001-01-01 00:00:00 +0000 UTC Crtime:0001-01-01 00:00:00 +0000 UTC Mode:drwx------ Nlink:1 Uid:83333 Gid:5000 Rdev:0 Flags:0}}
fuse: 2015/03/31 16:30:50 Received: Getattr [ID=0x5 Node=0x2 Uid=83333 Gid=5000 Pid=88148]
fuse: 2015/03/31 16:30:50 Responding: Getattr {AttrValid:0 Attr:{Inode:2 Size:0 Blocks:0 Atime:0001-01-01 00:00:00 +0000 UTC Mtime:0001-01-01 00:00:00 +0000 UTC Ctime:0001-01-01 00:00:00 +0000 UTC Crtime:0001-01-01 00:00:00 +0000 UTC Mode:drwx------ Nlink:1 Uid:83333 Gid:5000 Rdev:0 Flags:0}}
fuse: 2015/03/31 16:30:50 Received: Open [ID=0x6 Node=0x2 Uid=83333 Gid=5000 Pid=88148] dir=true fl=OpenReadOnly
fuse: 2015/03/31 16:30:50 Responding: Open {Handle:1 Flags:0}
fuse: 2015/03/31 16:30:50 Received: Statfs [ID=0x0 Node=0x1 Uid=83333 Gid=5000 Pid=88148]
fuse: 2015/03/31 16:30:50 Responding OK to Statfs.
fuse: 2015/03/31 16:30:50 Received: Getattr [ID=0x2 Node=0x2 Uid=83333 Gid=5000 Pid=88148]
fuse: 2015/03/31 16:30:50 Responding: Getattr {AttrValid:0 Attr:{Inode:2 Size:0 Blocks:0 Atime:0001-01-01 00:00:00 +0000 UTC Mtime:0001-01-01 00:00:00 +0000 UTC Ctime:0001-01-01 00:00:00 +0000 UTC Crtime:0001-01-01 00:00:00 +0000 UTC Mode:drwx------ Nlink:1 Uid:83333 Gid:5000 Rdev:0 Flags:0}}
fuse: 2015/03/31 16:30:50 Received: Read [ID=0x3 Node=0x2 Uid=83333 Gid=5000 Pid=88148] 0x1 1536 @0x0 dir=true
fuse: 2015/03/31 16:30:51 Responding: Read 32
fuse: 2015/03/31 16:30:51 Received: Lookup [ID=0x4 Node=0x2 Uid=83333 Gid=5000 Pid=88148] "foo"
fuse: 2015/03/31 16:30:51 Responding with error to *fuseops.LookUpInodeOp: no such file or directory
fuse: 2015/03/31 16:30:51 Received: Read [ID=0x1 Node=0x2 Uid=83333 Gid=5000 Pid=88148] 0x1 1536 @0x1 dir=true
fuse: 2015/03/31 16:30:51 Responding: Read 0
fuse: 2015/03/31 16:30:51 Received: Release [ID=0x5 Node=0x2 Uid=83333 Gid=5000 Pid=88148] 0x1 fl=OpenReadOnly rfl=0 owner=0x0
fuse: 2015/03/31 16:30:51 Responding OK to ReleaseDirHandleOp

@jacobsa
Copy link
Contributor Author

jacobsa commented Mar 31, 2015

I believe this is an unrelated regression. Opened #30 for it.

@jacobsa jacobsa closed this as completed Mar 31, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant