Add drive.rename(oldPath, newPath)
#376
Conversation
|
Addresses #364. Should this renaming happen as a batch append? I had assumed that if batching was required, it could be done at the const b = drive.batch()
await b.rename(oldPath, newPath)
await b.flush() |
|
@HDegroote any chance this could be merged? Maybe even soon? :-) |
index.js
Outdated
| async rename (oldPath, newPath, opts) { | ||
| const existingEntry = await this.entry(oldPath, opts) | ||
| await this.db.del(std(oldPath, false), { keyEncoding }) | ||
| return this.db.put(std(newPath, false), existingEntry.value, { keyEncoding }) |
There was a problem hiding this comment.
This logic is prone to race conditions:
- If the process crashes/stops after doing the del, but before the put runs, the entry's metadata no longer exists
- if rename runs twice for the same oldPath and newPath, and the second one starts after the first one finished its del operation (but before it finished its put), then
existingEntrywill be null, so the second run will override the result of the first one and make it point to null
The first issue can be resolved by using a batch, and the second with some sort of lock or debounce, but I'm not sure it's worth the complexity for this feature
There was a problem hiding this comment.
Great catch! I should have thought of this.
Added drive.batch() to lock the drive while renaming which should address the race condition and the mid process crash concern.
It's @mafintosh who decides on merging PR's for Hyperdrive, so best to ping him. I reviewed the PR though. I think it's a less trivial feature than you initially thought, because of the potential for race conditions. |
|
Needs a batch, no need for extra lock. The read should be on the batch as well. Docs need to specify that its for a single entry also |
Only updates the file entry in the hyperbee.
Previously renaming an entry twice at the same time could result in a race condition where the two entries were created for the same `oldPath` etc.
lejeunerenard
force-pushed
the
feat-rename
branch
from
375fa07
to
51582cb
Compare
|
Specified that |
|
just wanted to come back and wonder if rename could be done using The idea was to create a method |
Only updates the file entry in the hyperbee.
This PR was inspired by someone wanting to rename the file without having to re-append the blobstore entry.