Thread safe Crudini #6

Closed
tiamar opened this Issue Mar 24, 2014 · 6 comments

Comments

Projects
None yet
2 participants
@tiamar

tiamar commented Mar 24, 2014

Crudini is an excellent tool, because it is not thread safe it's not possible to use it multithread environments, like webapplication or when more than 1 sysadmin use it to alter the same file.

@pixelb

This comment has been minimized.

Show comment Hide comment
@pixelb

pixelb Mar 24, 2014

Owner

This is a good point. We write with first a truncation, then a write. Really we should open a tmp file and rename it in place which is an atomic operation. Now that does require write access to a directory on the same file system rather than just write access to the ini file in question. Also we have to be careful to ensure the attributes of the new and old file are the same which is tricky given extended attributes etc.

The above will ensure that the ini file is consistent for a particular run, though there are stil races possible where the complete changes for a run are overwritten by those of another overlapping run. You'd need higher level locking in place to avoid that.

Owner

pixelb commented Mar 24, 2014

This is a good point. We write with first a truncation, then a write. Really we should open a tmp file and rename it in place which is an atomic operation. Now that does require write access to a directory on the same file system rather than just write access to the ini file in question. Also we have to be careful to ensure the attributes of the new and old file are the same which is tricky given extended attributes etc.

The above will ensure that the ini file is consistent for a particular run, though there are stil races possible where the complete changes for a run are overwritten by those of another overlapping run. You'd need higher level locking in place to avoid that.

@tiamar

This comment has been minimized.

Show comment Hide comment
@tiamar

tiamar Mar 24, 2014

I thought on using lockfile:
from lockfile import LockFile
...
with LockFile(cfgfile):
with open(cfgfile, 'w') as f

or fcntl could be used, what you think?

tiamar commented Mar 24, 2014

I thought on using lockfile:
from lockfile import LockFile
...
with LockFile(cfgfile):
with open(cfgfile, 'w') as f

or fcntl could be used, what you think?

@pixelb

This comment has been minimized.

Show comment Hide comment
@pixelb

pixelb Mar 24, 2014

Owner

So that would be the "higher level locking" I mentioned to cater for all cases
For my reference: https://github.com/smontanaro/pylockfile/blob/master/doc/lockfile.rst
Now from experience lockfile uses persistent files (links/dirs) to implement, and so
is susceptible to stale links/dirs left on failures. So I may use fcntl() instead falling back
gracefully to something appropriate on windos.
For my reference: openstack/nova@2c15248

Owner

pixelb commented Mar 24, 2014

So that would be the "higher level locking" I mentioned to cater for all cases
For my reference: https://github.com/smontanaro/pylockfile/blob/master/doc/lockfile.rst
Now from experience lockfile uses persistent files (links/dirs) to implement, and so
is susceptible to stale links/dirs left on failures. So I may use fcntl() instead falling back
gracefully to something appropriate on windos.
For my reference: openstack/nova@2c15248

@pixelb

This comment has been minimized.

Show comment Hide comment
@pixelb

pixelb Jun 4, 2014

Owner

Some more notes.

To ensure data consistency, i.e. ini file readers only
ever see complete data, we can do something like:

def file_replace(name, data):
  tmp=name+'.tmp' #assue writable dir on same device as name
  shutil.copy2(name, tmp)
  with open(tmp, 'w') as f:
    f.write(data)
  if getattr(os,'replace',None):
    os.replace(tmp, file) # atomic even on windos
  elif platform.system() = 'Windows':
       if os.path.exists(dst):
         backup = tmp+'.backup'
         os.rename(name, backup)
         os.rename(tmp, name)
         os.unlink(backup)
      else:
         rename(src, dst)
  else:
    os.rename(tmp,file) # atomic on POSIX

To ensure that concurrent edits by crudini don't silently discard changes
we should add locking with fcntl if available or the equivalent windos call.
See: openstack/nova@3f0ef8e

Owner

pixelb commented Jun 4, 2014

Some more notes.

To ensure data consistency, i.e. ini file readers only
ever see complete data, we can do something like:

def file_replace(name, data):
  tmp=name+'.tmp' #assue writable dir on same device as name
  shutil.copy2(name, tmp)
  with open(tmp, 'w') as f:
    f.write(data)
  if getattr(os,'replace',None):
    os.replace(tmp, file) # atomic even on windos
  elif platform.system() = 'Windows':
       if os.path.exists(dst):
         backup = tmp+'.backup'
         os.rename(name, backup)
         os.rename(tmp, name)
         os.unlink(backup)
      else:
         rename(src, dst)
  else:
    os.rename(tmp,file) # atomic on POSIX

To ensure that concurrent edits by crudini don't silently discard changes
we should add locking with fcntl if available or the equivalent windos call.
See: openstack/nova@3f0ef8e

@pixelb

This comment has been minimized.

Show comment Hide comment
@pixelb

pixelb Jun 6, 2014

Owner

AC of ACID now implemented with 231aaaf
I'll implement I of ACID next with the locking changes discussed above

Owner

pixelb commented Jun 6, 2014

AC of ACID now implemented with 231aaaf
I'll implement I of ACID next with the locking changes discussed above

@pixelb

This comment has been minimized.

Show comment Hide comment
@pixelb

pixelb Aug 22, 2014

Owner

I of ACID now implemented with 937cc23

Owner

pixelb commented Aug 22, 2014

I of ACID now implemented with 937cc23

@pixelb pixelb closed this Aug 22, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment