Review Board 1.7.22


Low throughput of FileChannel

Review Request #6329 - Created Aug. 3, 2012 and updated

Denny Ye
FLUME-1423
Reviewers
Flume
hshreedharan, pwendell
Flume
Here is the description in code changes
1. Remove the 'FileChannel.force(false)'. Each commit from Source will invoke this 'force' method. This method is too heavy for amounts of data comes. Each 'force' action will be consume 50-500ms that it confirms data stored into disk. Normally, OS will flush data from kernal buffer to disk asynchronously with ms level latency. It may useless in each commit operation. Certainly, data loss may occurs in server crash not process crash. Server crash is infrequent.
2. Do not pre-allocate disk space. Disk doesn't need the pre-allocation.
3. Use 'RandomAccessFile.write()' to replace 'FileChannel.write()'. Both in my test result and low-level instruction, the former is better than the latter

Here I posted three changes, and I would like to use thread-level cached DirectByteBuffer to replace inner-heap ByteBuffer.allocate() (reuse outer-heap memory to reduce time that copying from heap to kernal). I will test this changes in next phase.

After tuning, throughput increasing from 5MB to 30MB

 
Review request changed
Updated (Aug. 3, 2012, 9:39 a.m.)
  • Here is the description in code changes
    1. Remove the 'FileChannel.force(false)'. Each commit from Source will invoke this 'force' method. This method is too heavy for amounts of data comes. Each 'force' action will be consume 50-500ms that it confirms data stored into disk. Normally, OS will flush data from kernal buffer to disk asynchronously with ms level latency. It may useless in each commit operation. Certainly, data loss may occurs in server crash not process crash. Server crash is infrequent.
    2. Do not pre-allocate disk space. Disk doesn't need the pre-allocation.
    3. Use 'RandomAccessFile.write()' to replace 'FileChannel.write()'. Both in my test result and low-level instruction, the former is better than the latter
    
    Here I posted three changes, and I would like to use thread-level cached DirectByteBuffer to replace inner-heap ByteBuffer.allocate() (reuse outer-heap memory to reduce time that copying from heap to kernal). I will test this changes in next phase.

    Here is the description in code changes
    1. Remove the 'FileChannel.force(false)'. Each commit from Source will invoke this 'force' method. This method is too heavy for amounts of data comes. Each 'force' action will be consume 50-500ms that it confirms data stored into disk. Normally, OS will flush data from kernal buffer to disk asynchronously with ms level latency. It may useless in each commit operation. Certainly, data loss may occurs in server crash not process crash. Server crash is infrequent.
    2. Do not pre-allocate disk space. Disk doesn't need the pre-allocation.
    3. Use 'RandomAccessFile.write()' to replace 'FileChannel.write()'. Both in my test result and low-level instruction, the former is better than the latter
    
    Here I posted three changes, and I would like to use thread-level cached DirectByteBuffer to replace inner-heap ByteBuffer.allocate() (reuse outer-heap memory to reduce time that copying from heap to kernal). I will test this changes in next phase.
    
    After tuning, throughput increasing from 5MB to 30MB
Description updated
Posted (Aug. 3, 2012, 10:16 a.m.)
Thanks Denny. IMO this patch defeats the purpose of 'durable' file channel. A reason an user might choose file-channel over memory-channel is to avoid data loss and JVM/server crash can happen due to various reasons such as  JVM bug, hardware problems and it is very hard to determine the frequency of crash. I would try ByteBuffer.allocateDirect() but there may be a chance of OOM as it depends on finalizer run [1] and we need to manually clean DirectByteBuffer using reflection.

[1] http://stackoverflow.com/questions/1854398/how-to-garbage-collect-a-direct-buffer-java
  1. hi Mubarak, if we remove 'force' method, throughput can increase from 5MB/s to 20MB/s, it impact the system performance. If we use 'force' method at each commit, can it hold 100% delivery?  
Posted (Aug. 3, 2012, 12:38 p.m.)
1) This would completely eliminate the guarantee FIleChannel makes. It sounds to me like like you want a disk spooling channel which does not make strict durability guarantees. This is not what FIleChannel does today. If you want faster throughput, increase batch size.
2) This is to eliminate corruption when we reach a full disk.  Also, writing over existing data is faster than writing the first time. I think we could improve this. I think we should allocate the length but not the data itself. This eliminates the metadata update when we write. Then we could just stop writing to disk when it's nearly full.
3) Have you done tests that show it's faster? Either way it should lead to write system call.
Posted (Aug. 3, 2012, 3:29 p.m.)
I'll echo concern from others that this abandons the semantics of the channel.

How much of the improvement is related to change (1) - i.e. not sync()'ing at transaction boundaries? It would be great to get the throughput improvement broken down by change.
Posted (Aug. 3, 2012, 5:20 p.m.)
I do have maybe unrelated question - aren't you trying to achieve something like SpillableChannel that is described in FLUME-1227?
Posted (Aug. 3, 2012, 6:08 p.m.)
I have to agree with the others here. Removing the force() call is not a good way to improve performance. To quote someone I know: "Correctness before performance." Removing the force kills the durability semantics of the channel, which is what most people use it for.