Fixes problems with large snapshots #411
Conversation
| @@ -65,7 +66,7 @@ public FileChunkReader(FileInfo info, RaftStorageDirectory directory) throws IOE | |||
| public FileChunkProto readFileChunk(int chunkMaxSize) throws IOException { | |||
| final long remaining = info.getFileSize() - offset; | |||
| final int chunkLength = remaining < chunkMaxSize ? (int) remaining : chunkMaxSize; | |||
| final ByteString data = ByteString.readFrom(in, chunkLength); | |||
There was a problem hiding this comment.
ByteString.readFrom(in, chunkLength) will read the whole file not just chunkLength
| @@ -80,6 +81,32 @@ public FileChunkProto readFileChunk(int chunkMaxSize) throws IOException { | |||
| return proto; | |||
| } | |||
|
|
|||
| /** | |||
There was a problem hiding this comment.
Code copied from ByteString class
| @@ -61,22 +60,23 @@ public void installSnapshot(StateMachine stateMachine, | |||
| final long lastIncludedIndex = snapshotChunkRequest.getTermIndex().getIndex(); | |||
| final RaftStorageDirectory dir = storage.getStorageDir(); | |||
|
|
|||
| // create a unique temporary directory | |||
| final File tmpDir = new File(dir.getTmpDir(), UUID.randomUUID().toString()); | |||
There was a problem hiding this comment.
when receiving a snapshot which does not fit in a single chunk, then multiple directories are created, each having a chunk of the snapshot
| tmpDir, dir.getStateMachineDir()); | ||
| dir.getStateMachineDir().delete(); | ||
| tmpDir.renameTo(dir.getStateMachineDir()); | ||
| LOG.info("Install snapshot is done, moving files from dir:{} to:{}", tmpDir, dir.getStateMachineDir()); |
There was a problem hiding this comment.
Handles the case where multiple files are copied
| FileUtils.createDirectories(tmpDir); | ||
| tmpDir.deleteOnExit(); | ||
|
|
||
| LOG.info("Installing snapshot:{}, to tmp dir:{}", request, tmpDir); |
There was a problem hiding this comment.
This will log the whole chunk.. Lots of bytes..
alpapad
force-pushed
the
snapshot_push_to_follower
branch
2 times, most recently
from
f90b9a0 to
8a33c84
Compare
|
@alpapad would you mind creating a JIRA for this PR? |
|
I guess this will worth an email to user@ratis to ask the problem you are facing. I don't think Ratis cannot handle larger than 8MB files transferred well. There is an example in Ratis repo named FileStore. You can use that example to save files by an application based on Ratis. I believe I saw people used GBs files on FileStore. Ratis is also used by Apache Ozone which is supposed to stress Ratis a lot in its use cases. |
|
@amaliujia The problem exists when a statemachine snapshot needs to be transfered (for example because of a node failure). The snapshot should be chunked to a specific chunk-size but instead the whole file is read and the server does not expect it to be more than X Mb (I think 8, but I will need to check). I think the FileStore statemachine does not store much and file contents are not even stored in the ratis wal. I will create a jira for this. |
|
For such optimizations, usually I think it worths a benchmark to identify the bottleneck and/or provide a baseline to convince people that such optimization is useful, and how much it is useful. |
alpapad
force-pushed
the
snapshot_push_to_follower
branch
3 times, most recently
from
d2532b6 to
76ce6e1
Compare
alpapad
force-pushed
the
snapshot_push_to_follower
branch
from
76ce6e1 to
fd60e4d
Compare
alpapad
force-pushed
the
snapshot_push_to_follower
branch
3 times, most recently
from
4718b73 to
9f3e04c
Compare
alpapad
force-pushed
the
snapshot_push_to_follower
branch
from
9f3e04c to
94f5ead
Compare
alpapad
force-pushed
the
snapshot_push_to_follower
branch
from
94f5ead to
befb2d6
Compare
What changes were proposed in this pull request?
Hi,
I have been testing ratis for a solutions which uses rather largish snapshots for the state machine. Current implementation is not able to transfer files larger than 8Mb (I think), throwing all kinds of exceptions. I guess this is something not really used by current users of ratis.
Here are the places I modified/found to be problematic (code is more precise than words):
FileChunkReader.readFileChunk uses ByteString.readFrom(in, chunkLength); which will fully load the input stream and not just a junk of size chunkLength
Grpc.proto service installSnapshot(stream ratis.common.InstallSnapshotRequestProto) should return a stream
Modifications to SnapshotManager so chunks for the same snapshot will be placed in the same directory
Not sure if I need to create a jira issue (never used apache f. jira) I can repro and create jiras with stacktraces etc.