[infinispan-issues] [JBoss JIRA] (ISPN-3665) SingleFileStore is not thread-safe for passivation

[Paul Ferraro (JIRA)]

    [ https://issues.jboss.org/browse/ISPN-3665?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12829577#comment-12829577 ] 

Paul Ferraro edited comment on ISPN-3665 at 11/1/13 4:53 PM:
-------------------------------------------------------------

So, here are the issues I see with SingleFileStore:
* Both start() and stop() set the values of a number of non-volatile fields.  These changes are not guaranteed to be visible to threads calling operations such as write(...), delete(...), etc.  Making these methods synchronized would fix this problem.
* -write(MarshalledEntry) sets the values of a number of non-volatile fields of the FileEntry object.  These changes are not guaranteed to be visible to other threads.  Making these fields volatile would fix this problem.- Actually, this looks OK.  Visibility to these fields is ensured by the entries monitor.
* delete(...) calls free(FileEntry), which zeros out the portion of the file even though other threads might be reading the value.  Dan alluded to this issue above.


                
      was (Author: pferraro):
    So, here are the issues I see with SingleFileStore:
* Both start() and stop() set the values of a number of non-volatile fields.  These changes are not guaranteed to be visible to threads calling operations such as write(...), delete(...), etc.  Making these methods synchronized would fix this problem.
* write(MarshalledEntry) sets the values of a number of non-volatile fields of the FileEntry object.  These changes are not guaranteed to be visible to other threads.  Making these fields volatile would fix this problem.
* delete(...) calls free(FileEntry), which zeros out the portion of the file even though other threads might be reading the value.  Dan alluded to this issue above.


                  
> SingleFileStore is not thread-safe for passivation
> --------------------------------------------------
>
>                 Key: ISPN-3665
>                 URL: https://issues.jboss.org/browse/ISPN-3665
>             Project: Infinispan
>          Issue Type: Bug
>          Components: Loaders and Stores
>    Affects Versions: 6.0.0.CR1
>            Reporter: Paul Ferraro
>            Assignee: Mircea Markus
>            Priority: Blocker
>              Labels: 620
>         Attachments: Test.java
>
>
> SingleFileStore never makes use of FileChannel.force(...) to flush changes to disk.  This causes problems for the passivation use case.
> If one thread evicts a cache entry, while immediately after another thread attempts to read the same cache entry, the Cache.get(...) can return null.  This is because the entry is never flushed to disk.
> I've attached a test to reproduce the problem.
> I also ran the same test with the addition of FileChannel.force(false) to the write(...) method, and the test succeeds.
> A proper fix should probably make this a configurable property (as it was with the old file store implementation).  It would be nice if the flush could defer until just before tx commit, but, off hand, I don't know how feasible that is.
> I suspect this lack of flush also accounts for much of the bold claim of a 100x performance improvement over the old file store implementation.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira