Skip to content

Ensure that nuke_dir properly deletes the folder #33

Closed
wants to merge 1 commit into from

2 participants

@adamlofts

nuke_dir() was calling couch_file:delete() which expects to be passed a file. This pull request splits out a delete_rename() function and adds nuke_dir_sync() to actually remove the folder once it is renamed.

There is also a small performance improvement by deleting the folder asynchronously after it has been renamed.

The second commit reuses the nuke_dir_sync() function to clean up the .delete folder on startup because it can now contain deleted folders.

@dch
dch commented Oct 7, 2012

We have an issue with nuke_dir as it stands in master, in that it doesn't work on Windows as the view server may still be holding files open. I'm guessing your patch will still suffer from this issue. I think the rename should work, but there needs to be a way to handle a lazy delete. Any suggestions? Ref https://issues.apache.org/jira/browse/COUCHDB-1346 https://issues.apache.org/jira/browse/COUCHDB-780 and https://issues.apache.org/jira/browse/COUCHDB-86 for some history. My build server just died so it will be a few days before I can look at this properly. Many thanks for your contribution!

@adamlofts

I believe the new commit should fix your issue but I don't have access to a windows box. Unfortunately it is quite a lot of extra code.

@adamlofts adamlofts Change nuke_dir() to rename the directory and delete asynchronously C…
…OUCHDB-1346

First nuke_dir() creates a folder in the .delete directory and moves all files
in the delete target into this folder. This is because the files may still be open
and windows only allows moving open files and not the parent folder. Then move the
directory into the delete dir and finally call nuke_dir_sync() in the background.

We sleep 5 seconds before actually deleting the folder to give the view group indexer
a chance to close its files.

On startup use nuke_dir_sync to clean the .delete directory because it now can contain
directories.
79db1e2
@dch
dch commented Oct 10, 2012
@dch
dch commented on 79db1e2 Oct 31, 2012

Sorry this has taken so long to get back to you @adamlofts! Thanks for sending in this patch BTW - I really appreciate it.

@davisp and I discussed this on IRC last night, there's a concern that the timeout approach is going to be brittle, and that a better way to handle this will be by tracking open filehandles (more complex but more portable also). At least on Windows, antivirus software is a complicating factor.

Did you consider how this will handle sub directories in DB names - e.g. PUT /embedded%2fdbname actually ends up as $COUCH/var/lib/couch/embedded/dbname.couch on disk?

I'll continue the discussion in JIRA so we get a few more eyes on it.

@adamlofts adamlofts closed this Apr 4, 2013
@strmpnk strmpnk pushed a commit to strmpnk/couchdb that referenced this pull request Dec 9, 2014
@Vagabond Vagabond Supervisor children are not guranteed to be atoms. Fixes issue #33 dc750f5
@candeira candeira pushed a commit to candeira/couchdb that referenced this pull request Apr 6, 2015
@kxepal kxepal Define ADMIN_USER and ADMIN_CTX macros
This closes #33
8e473e1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.