hi Hari, what's mean of this bug?

I think this means group commit https://kb.askmonty.org/en/group-commit-for-the-binary-log/

Yep, that is correct. I am trying to implement a simple version of what is described in that post (sorry, I didnt post the link myself!)

Hope you guys don't mind I just updated the issue title

Looks like all the methods that write to the file are synchronized, so not sure group commits can achieve much without considerable refactoring.

I think this is going to require refactoring of code and also our approach to how to we commit. From my perspective, what we want to avoid is:

thread-1: write event1
thread-2: write event2
thread-1: write event3
thread-2: commit and sync
thread-1: commit and sync (immediately after thread-2)

That is we don't want multiple threads doing subsequent commits and then syncs. It'd be ideal if we had:

thread-1: write event1
thread-2: write event2
thread-1: write event3
thread-2: commit
thread-1: commit
thread-2: sync
thread-1: sync – noop sync we already synced our commit.

Brock - the 2nd approach is what I implemented. I will do some refactoring to remove a buch of the synchronization. This is mostly for the rolling logic, so should be fairly simple handle.

Yeah it'd be interesting to see. Offhand was thinking we'd keep the synchronization and break LogFile.commit into two calls, commit and sync(offset). The commit method would return the offset of a commit and then the sync method would track the last offset a sync was called and if the last sync was greater than or equal to the offset the writer was interested in we wouldn't have to actually do the sync.

Actually commit is already split up. I did exactly that - thought right now the commit happens from synchronized blocks, so really there is no concurrency in the commits/syncs (syncs are called from synchronized methods).

Moved the sync out of the commit calls and make those calls explicitly. sync() method will fsync only if the last commit position is higher than the last sync position.

Hey Hari,

Thanks for the patch! I think the patch will work, but I think there is a race that could cause "extra" syncs? Let me know if I am off base:

thread-1 commit
thread-2 commit
thread-2 sync
thread-3 commit
thread-1 sync <- here thread-1 will do a sync if though it's required?
thread-3 sync

I think we could eliminate this by returning the offset from put/rollback and then passing into sync?

Brock,

In your example, the thread-3 sync would end up being a no-op, right? Since the sync by thread-1 happened after thread-3 updated the lastCommitPosition. After that thread-3's sync has no effect because the lastSyncOffset is now the same as the lastCommitOffset(this was updated by thread-1 when the sync happened). In the scenario you describe above, minimum of 2 syncs are required, and it looks like only 2 would actually happen.

Reading that comment again, I guess I could make it clearer. Step-by-step, with this patch:
thread-1 commit - lastSyncOffset = 0, lastCommitOffset = t-1 cOffset
thread-2 commit - lastSyncOffset = 0, lastCommitOffset = t-2 cOffset (t-2 cOffset > t-1 cOffset)
thread-2 sync - lastSyncOffset = t-2 cOffset, lastCommitOffset = t-2 cOffset (fsync, since t-2 cOffset > 0)
thread-3 commit - lastSyncOffset = t-2 cOffset, lastCommitOffset = t-3 cOffset (now t-3 cOffset > t-2 cOffset)
thread-1 sync - lastSyncOffset = t-3 cOffset, lastCommitOffset = t-3 cOffset (fsync, since t-3 cOffset > t-2 cOffset)
thread-3 sync - lastSyncOffset = t-3 cOffset, lastCommitOffset = t-3 cOffset (no-op, since t-3 cOffset == t-3 cOffset).

So the idea of group commit works fine, just that the fsync happens in a different thread than the one that wrote to the last offset. I don't think that is an issue since the total number of fsyncs is 2 as expected.

Hey Hari,

Yeah, I should have noted that thread-3 sync is a noop. The logic is obvious when we pass in the offset into the sync method but it looks like the current patch will result in the same number of syncs.

Thanks Hari! I committed this to trunk and 1.4.

Awesome!

Integrated in flume-trunk #453 (See https://builds.apache.org/job/flume-trunk/453/)
FLUME-1917: FileChannel group commit (coalesce fsync) (Revision b8c4b003a8273b17e8641a9e5bcafd2357fbd370)

Result = FAILURE
brock : http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=b8c4b003a8273b17e8641a9e5bcafd2357fbd370
Files :

• flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Log.java
• flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFile.java
• flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestLogFile.java