mafintosh / append-tree

Model a tree structure on top off an append-only log.
MIT License
54 stars 13 forks source link

Possible bug dealing with deletions #6

Open pfrazee opened 7 years ago

pfrazee commented 7 years ago

I'm writing a recursive delete algorithm, which looks like this:

function recurseDelete (archive, targetPath, st) {
  return co(function* () {
    // fetch stat if needed
    if (!st) {
      st = yield stat(archive, targetPath)
    }
    if (st.isFile()) {
      // delete file
      return new Promise((resolve, reject) => {
        console.log('unlink', targetPath)
        archive.unlink(targetPath, (err) => {
          if (err) reject(toBeakerError(err, 'unlink'))
          else resolve()
        })
      })
    } else if (st.isDirectory()) {
      // fetch children
      var children = yield readdir(archive, targetPath)
      // delete children
      yield Promise.all(children.map(childName => 
        recurseDelete(archive, path.join(targetPath, childName)))
      )
      // delete self
      return new Promise((resolve, reject) => {
        console.log('rmdir', targetPath)
        archive.rmdir(targetPath, err => {
          if (err) reject(toBeakerError(err, 'rmdir'))
          else resolve()
        })
      })
    } else {
      throw new Error('Unexpectedly encountered an entry which is neither a file or directory at', path)
    }
  })
}

That's being run against the following file-tree:

a
b/ (dir)
b/a/ (dir)
b/b
b/c
b/d/ (dir)
b/d/a
b/d/b
b/d/c/ (dir)
b/d/c/a
b/d/d
c/ (dir)
c/b/ (dir)

Specifically against the /b folder. A working log would look like this:

unlink b/b
unlink b/c
rmdir b/a
unlink b/d/d
unlink b/d/a
unlink b/d/b
unlink b/d/c/a
rmdir b/d/c
rmdir b/d
rmdir b

But the log I get is this:

unlink b/b
unlink b/c
rmdir b/a
unlink b/d/d
unlink b/d/a
unlink b/d/b
unlink b/d/c/a
rmdir b/d/c
Error: File not found

For some reason, rmdir('/b/d/c') is failing with File not found. If I run the entire method on the '/b/d' folder, no such error occurs!

I'm wondering if the record of the '/b/d/c' folder is getting lost somehow?

mafintosh commented 7 years ago

Weird! I'll try and encapsulate that in a test. Was the file tree created in the same order as you list above?

pfrazee commented 7 years ago

It was, yeah

mafintosh commented 7 years ago

Noting from IRC that this only happens on parallel dels

pfrazee commented 7 years ago

I just got this again without parallel deletes

pfrazee commented 7 years ago

Yeah I get this consistently, now, in pauls-dat-api tests. (Should be relatively easy to setup a repeatable test for you to run.)

pfrazee commented 7 years ago

To reproduce:

You should find all tests passing, but a warning should be emitted saying rmdir issue (append-tree#6). That's a warning emitted at https://github.com/beakerbrowser/pauls-dat-api/blob/SLEEP/lib/delete.js#L121-L125, where I've disabled the full error-handling on account of this bug.

For me, this bug consistently occurs in my "recursive rename" algorithm, which does a recursive-copy and then a recursive-delete. There shoudnt be any parallelism involved; it just appears that deletes are deleting more than they should.

mafintosh commented 7 years ago

Just fixed a delete bug in latest hyperdrive (released on npm). Could try to see if that fixes this by any chance?