You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Dan Smith <ds...@pivotal.io> on 2015/08/22 00:59:03 UTC
Review Request 37696: Fixing contents of partially written files
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37696/
-----------------------------------------------------------
Review request for geode and xiaojian zhou.
Repository: geode
Description
-------
I added some tests of partially written files. The tests write to an
output stream but don't close it, which mimics a partially written file
because of the way the code works - it doesn't save the changes to the
File object until OutputStream is closed.
I found that the metadata was inconsistent with the file contents for
partially written files. I fixed that by deleting any chunks past the
reported length of the file.
Making renames and deletes closer to atomic
It's important that when a file is renamed, the new file appears
atomically with all of the data - according to the LuceneDirectory
interface. It's also important that during a delete the file does not
start missing chunks before it is actually deleted.
These changes address those two issues. I've introduced a UUID for the
file that is used instead of the filename as part of the chunk id. When
a file is renamed, the new file just points to the same UUID. Deletes
delete the File object first and the chunks later.
There are still some dangling issues - namely that there can be dangling
chunks for files that have been deleted. But I think the more severe
issues with a lack of atomicity are resolved now.
I've added a couple of unit tests of partial renames and partial
deletes. These tests work by wrapping the underlying "region" with a
wrapper that throws a CacheClosedException after a certain number of
operations have happened - essentially mimicking a member being closed
or killed.
Diffs
-----
gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/ChunkKey.java 5564c02b0585fb74a1311b4f4510285180494d80
gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/File.java 1f5edb746a85b22fb0d5ca479cf3ef0be64084a4
gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/FileOutputStream.java 4006be21f0568fd48f1fbdc98c9c11045c488959
gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/FileSystem.java 62b37000008860e95cc71a328b0a5a00edc8c214
gemfire-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/FileSystemJUnitTest.java b79bd1e1178b78a13bdc6750828895c68e149aaf
Diff: https://reviews.apache.org/r/37696/diff/
Testing
-------
Thanks,
Dan Smith
Re: Review Request 37696: Fixing contents of partially written files
Posted by xiaojian zhou <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37696/#review96102
-----------------------------------------------------------
Ship it!
Ship It!
- xiaojian zhou
On Aug. 21, 2015, 10:59 p.m., Dan Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37696/
> -----------------------------------------------------------
>
> (Updated Aug. 21, 2015, 10:59 p.m.)
>
>
> Review request for geode and xiaojian zhou.
>
>
> Repository: geode
>
>
> Description
> -------
>
> I added some tests of partially written files. The tests write to an
> output stream but don't close it, which mimics a partially written file
> because of the way the code works - it doesn't save the changes to the
> File object until OutputStream is closed.
>
> I found that the metadata was inconsistent with the file contents for
> partially written files. I fixed that by deleting any chunks past the
> reported length of the file.
>
> Making renames and deletes closer to atomic
>
> It's important that when a file is renamed, the new file appears
> atomically with all of the data - according to the LuceneDirectory
> interface. It's also important that during a delete the file does not
> start missing chunks before it is actually deleted.
>
> These changes address those two issues. I've introduced a UUID for the
> file that is used instead of the filename as part of the chunk id. When
> a file is renamed, the new file just points to the same UUID. Deletes
> delete the File object first and the chunks later.
>
> There are still some dangling issues - namely that there can be dangling
> chunks for files that have been deleted. But I think the more severe
> issues with a lack of atomicity are resolved now.
>
> I've added a couple of unit tests of partial renames and partial
> deletes. These tests work by wrapping the underlying "region" with a
> wrapper that throws a CacheClosedException after a certain number of
> operations have happened - essentially mimicking a member being closed
> or killed.
>
>
> Diffs
> -----
>
> gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/ChunkKey.java 5564c02b0585fb74a1311b4f4510285180494d80
> gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/File.java 1f5edb746a85b22fb0d5ca479cf3ef0be64084a4
> gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/FileOutputStream.java 4006be21f0568fd48f1fbdc98c9c11045c488959
> gemfire-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/FileSystem.java 62b37000008860e95cc71a328b0a5a00edc8c214
> gemfire-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/FileSystemJUnitTest.java b79bd1e1178b78a13bdc6750828895c68e149aaf
>
> Diff: https://reviews.apache.org/r/37696/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan Smith
>
>