Django

Code

Ticket #8403 (closed: fixed)

Opened 3 months ago

Last modified 3 weeks ago

Current file locking does not work on NFS mounts

Reported by: mtredinnick Assigned to: mtredinnick
Milestone: 1.0 Component: File uploads/storage
Version: SVN Keywords: file lock
Cc: Triage Stage: Accepted
Has patch: 0 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

The code in django.core.files.locks assumes that flock() is safe to use on every POSIX system to lock files. This isn't the case when the filesystem in question is a relatively recent NFS system. In that case, flock() actively raises an error (rather than potentially silently failing to lock things, which was the older behaviour). For those situations fnctl() should be used to do file locking.

From a bit of a search around and some source reading (plus some comments on the Mailman list, although I've avoided reading the relevant code there for the moment, since it's GPL and I might end up being the person who fixes this ticket), it looks like posixfile.lock() in the Python source is an appropriate way to do this.

I realise the locks.py file isn't our source, but it is broken in this respect and we do include it. So we have to fix it, since, right now, you can't save files to an NFS-mounted filesystem.

This effect in Django was initially noticed in this django-users thread.

Attachments

test.py (244 bytes) - added by roman on 08/25/08 13:27:26.
Test using locking, run in two separate terminals, but in the same directory.

Change History

08/18/08 19:22:36 changed by mtredinnick

  • needs_better_patch changed.
  • stage changed from Unreviewed to Accepted.
  • needs_tests changed.
  • needs_docs changed.

08/22/08 12:38:25 changed by roman

  • owner changed from nobody to roman.

08/22/08 23:39:33 changed by roman

  • keywords set to file lock.

The file of interest is django/core/files/locks.py.

Imported in django/core/files/storage.py django/core/files/move.py

Pointlessly imported in django/db/models/base.py

Trying to change to fcntl.fcntl() from fcntl.flock() in the lock and unlock. Using posfixfile.lock() looks like the best way forward, but I get an error 

   File "/usr/lib/python2.5/site-packages/django/core/files/locks.py", line 70, in lock
    rv = fcntl.fcntl(file, fcntl.F_SETLKW, lockdata)
IOError: [Errno 22] Invalid argument

This guy had the same NFS locking problem and says he got it to work. It did not work for me. Then again, my NFS mount seemed to support flock() even with subtree_check. http://coding.derkeiler.com/Archive/Python/comp.lang.python/2007-06/msg00801.html

08/25/08 13:27:26 changed by roman

  • attachment test.py added.

Test using locking, run in two separate terminals, but in the same directory.

08/28/08 12:28:19 changed by mtredinnick

  • owner changed from roman to mtredinnick.
  • status changed from new to assigned.

08/28/08 13:49:26 changed by mtredinnick

Okay, this turns out to be trickier to replicate than it looks at first sight. It's pretty dependent on the OS, and the NFS client and server versions, and the export options for the NFS mount. In fact, I wasn't able to replicate it on my laptop or two other systems with proper remote NFS mounts that I had access to.

With a bit of reading of the Python source and some thinking about what's going on, I think I've solved it in a portable fashion. So the following commit will close this ticket off. If somebody can subsequently repeat the problem, please do reopen this, but we'll need all of the above information: OS, export options, nfsd and client nfs tools versions (where appropriate). I'd also like to know if you're able to lock a file opened for writing on your particular system using Python's fcntl module in any fashion, because we'd be down to writing workarounds at that level.

By the way, the use of the posixfile module doesn't seem appropriate, after looking a bit more closely. It's pretty clearly marked as deprecated and would be bad to introduce a dependency on it now.

08/28/08 14:29:02 changed by mtredinnick

  • status changed from assigned to closed.
  • resolution set to fixed.

(In [8675]) Fixed #8403 -- Changed the use of fcntl.flock() to fcntl.lockf(). On some systems, this will ensure fnctl-based file locking is always used, which means locking of NFS-mounted files should work.

09/10/08 15:32:42 changed by jarrow

  • status changed from closed to reopened.
  • resolution deleted.

After updating to Django 1.0 I got this traceback where the admin tries to write stuff to a NFS mount. I'm not 100% sure it's the the same issue that this ticket was about, but it fails at the line of the fix ...

  • OS: Debian Etch
  • export options: (rw,sync,no_subtree_check)
  • nfsd and client nfs tools versions: 1.0.10-6+etch.1
  • options from the mount: rw,vers=3,rsize=32768,wsize=32768,hard,intr,proto=tcp,timeo=600,retrans=2,sec=sys,addr=vm1 0 0

Traceback: 

Traceback:
File "/usr/lib/python2.4/site-packages/django/core/handlers/base.py" in get_response
  86.                 response = callback(request, *callback_args, **callback_kwargs)
File "/usr/lib/python2.4/site-packages/django/contrib/admin/sites.py" in root
  158.                 return self.model_page(request, *url.split('/', 2))
File "/usr/lib/python2.4/site-packages/django/views/decorators/cache.py" in _wrapped_view_func
  44.         response = view_func(request, *args, **kwargs)
File "/usr/lib/python2.4/site-packages/django/contrib/admin/sites.py" in model_page
  177.         return admin_obj(request, rest_of_url)
File "/usr/lib/python2.4/site-packages/django/contrib/admin/options.py" in __call__
  197.             return self.change_view(request, unquote(url))
File "/usr/lib/python2.4/site-packages/django/db/transaction.py" in _commit_on_success
  238.                 res = func(*args, **kw)
File "/usr/lib/python2.4/site-packages/django/contrib/admin/options.py" in change_view
  579.                 new_object = self.save_form(request, form, change=True)
File "/usr/lib/python2.4/site-packages/django/contrib/admin/options.py" in save_form
  370.         return form.save(commit=False)
File "/usr/lib/python2.4/site-packages/django/forms/models.py" in save
  302.         return save_instance(self, self.instance, self._meta.fields, fail_message, commit)
File "/usr/lib/python2.4/site-packages/django/forms/models.py" in save_instance
  47.         f.save_form_data(instance, cleaned_data[f.name])
File "/usr/lib/python2.4/site-packages/django/db/models/fields/files.py" in save_form_data
  192.             getattr(instance, self.name).save(data.name, data, save=False)
File "/usr/lib/python2.4/site-packages/django/db/models/fields/files.py" in save
  217.         super(ImageFieldFile, self).save(name, content, save)
File "/usr/lib/python2.4/site-packages/django/db/models/fields/files.py" in save
  74.         self._name = self.storage.save(name, content)
File "/usr/lib/python2.4/site-packages/django/core/files/storage.py" in save
  45.         name = self._save(name, content)
File "/usr/lib/python2.4/site-packages/django/core/files/storage.py" in _save
  150.                     file_move_safe(content.temporary_file_path(), full_path)
File "/usr/lib/python2.4/site-packages/django/core/files/move.py" in file_move_safe
  70.             locks.lock(fd, locks.LOCK_EX)
File "/usr/lib/python2.4/site-packages/django/core/files/locks.py" in lock
  57.         fcntl.lockf(fd(file), flags)

Exception Type: IOError at /program/event/1109/
Exception Value: [Errno 5] Input/output error

Let me know if I should post more info ...

09/15/08 19:26:05 changed by mtredinnick

  • status changed from reopened to closed.
  • resolution set to fixed.

comment:7 looks like a slightly different problem. Since we've fixed the reason for the first failure, I've created a new ticket for the second version (#9099), rather than having this one reopened with every different instance of NFS problems. That will help keep the history straight.

10/06/08 11:42:03 changed by rndblnch

  • status changed from closed to reopened.
  • resolution deleted.

I reopen this bug because the fix introduces a regression for my specific case : - system : macosx - django : 1.0 - filesystem accessed : afp (it works other with on the local file system, no idea over nfs) Reverting Changeset 8675 solves the problem ...

(follow-up: ↓ 11 ) 10/06/08 12:00:46 changed by kmtracey

  • status changed from reopened to closed.
  • resolution set to fixed.

Changeset [8675] fixed a real problem and isn't simply going to be reverted. If it's causing a problem on your setup, please open a new ticket to address that. In the new ticket please include specifics of the "regression" -- what exactly happens? You have not provided enough information here for anyone to solve "the problem" you are seeing, please provide more specifics of the problem you are seeing in the new ticket.

(in reply to: ↑ 10 ) 11/13/08 03:51:07 changed by rndblnch

Replying to kmtracey:

Changeset [8675] fixed a real problem and isn't simply going to be reverted. If it's causing a problem on your setup, please open a new ticket to address that. In the new ticket please include specifics of the "regression" -- what exactly happens? You have not provided enough information here for anyone to solve "the problem" you are seeing, please provide more specifics of the problem you are seeing in the new ticket.

done here: #9433

Add/Change #8403 (Current file locking does not work on NFS mounts)




Change Properties
Action