search

blaze / castra

Partitioned storage system based on blosc. **No longer actively maintained.**
BSD 3-Clause "New" or "Revised" License
153 stars 21 forks source link

__del__ method causes race condition between processes/threads #44

Closed jcrist closed 9 years ago

jcrist commented 9 years ago

The __del__ method causes a race condition between processes/threads reading from the same castra if it's initialized multiple times (different Castra objects, same filepath). Other workers can read from the file mid flush, which causes periodic failures (seen both locally, and on travis here).

A few solutions:

  1. Add a readonly mode that doesn't do anything on __del__, and doesn't allow extend/extend_sequence. Could either be a mode='r' kwarg, or a readonly=True kwarg. Default would be writable, so no change in default behavior.
  2. Remove the automatic flush/delete behavior. I'm unsure if removing automatic flush as a default is worth it, but I'm definitely against automatic delete. IMO temporary files/directories should be managed externally to castra. This is a separate issue entirely though.
  3. File based locks to ensure only one writer to each file/no reading while writing to same file.

My preference is option 1.

jcrist commented 9 years ago

I really want to get rid of automatic temp files for castra, as having them complicates option 1 as well. Tempfiles/directories can be created by a separate contextmanager, and simplify the implementation of castra. I need this to be settled to fix https://github.com/blaze/dask/pull/644, so would like a group decision quickly. Ping @mrocklin , @esc, @cpcloud for thoughts.

jcrist commented 9 years ago

I added a readonly kwarg in #45. If we decide we want more fine-grained open modes (like hdf5 has 'a', 'w', 'r+'...) this can be switched out later. I still would like to get rid of the tempfile functionality, but this PR covers my immediate need.