BTM
  1. BTM
  2. BTM-113

A patch to use FileChannel insteadof RandomAccessFile to write log journal.

    Details

    • Type: Improvement Improvement
    • Status: Resolved Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.1.2
    • Fix Version/s: 3.0.0
    • Environment:
      Mac osx 10.7, java version "1.6.0_29"
    • Patch Submitted:
      Yes
    • Number of attachments :
      1

      Description

      A patch to great improve DiskJournal's performance.I used java.nio.channels.FileChannel to write log in batch with ByteBuffer,insteadof using java.io.RandomAccessFile.Also i have reduced the lock range in some codes,please check the pacth in attachment.

      In my test,it could get about 2x improvement in performance.

      You can patch it with "git apply use_nio_channel.patch" and to diff the codes.

        Activity

        Hide
        Ludovic Orban added a comment -

        Interesting changes. There already were some similar things done in the btm-2.1.x branch but you went a little bit further. I merged the 2.1.3-SNAPSHOT changes back to the master branch then applied a modified version of your patch on top.

        There were some serious race conditions in your patch which forced me to modify the scope of the synchronized blocks so this may reduce the perf improvement you measured, while at the same time concurrency has generally been improved in the core TM code so YMMV.

        I'd be glad if you could try out this all-integrated version and post your feedback here.

        Thanks!

        Show
        Ludovic Orban added a comment - Interesting changes. There already were some similar things done in the btm-2.1.x branch but you went a little bit further. I merged the 2.1.3-SNAPSHOT changes back to the master branch then applied a modified version of your patch on top. There were some serious race conditions in your patch which forced me to modify the scope of the synchronized blocks so this may reduce the perf improvement you measured, while at the same time concurrency has generally been improved in the core TM code so YMMV. I'd be glad if you could try out this all-integrated version and post your feedback here. Thanks!
        Hide
        dennis zhuang added a comment -

        Great.
        I will do much more test next week.Thanks for your great work.

        Show
        dennis zhuang added a comment - Great. I will do much more test next week.Thanks for your great work.
        Hide
        dennis zhuang added a comment -

        Hi
        But i have a question about the writeLog method in TransactionLogAppender.Why did you synchronize the whole method with 'fc'.
        I thought that put data to ByteBuffer is not nessary to be synchronized.

        Show
        dennis zhuang added a comment - Hi But i have a question about the writeLog method in TransactionLogAppender.Why did you synchronize the whole method with 'fc'. I thought that put data to ByteBuffer is not nessary to be synchronized.
        Hide
        Ludovic Orban added a comment -

        There are 3 things happening in TransactionLogAppender.writeLog():

        • a check to make sure there's enough space left in the file
        • writing the TransactionLogRecord
        • updating the position header

        Those must be done atomically or you may mistakenly assume you have enough space left in the file while another thread took that space or you may corrupt the file by not updating the position header correctly because another thread changed it while you were still writing the TransactionLogRecord.

        Show
        Ludovic Orban added a comment - There are 3 things happening in TransactionLogAppender.writeLog(): a check to make sure there's enough space left in the file writing the TransactionLogRecord updating the position header Those must be done atomically or you may mistakenly assume you have enough space left in the file while another thread took that space or you may corrupt the file by not updating the position header correctly because another thread changed it while you were still writing the TransactionLogRecord.
        Hide
        dennis zhuang added a comment -

        I have a question,if the BTM can use "group commit" to reduce the times of invoking sync to flush?
        About "Group commit":
        http://kristiannielsen.livejournal.com/12254.html

        Show
        dennis zhuang added a comment - I have a question,if the BTM can use "group commit" to reduce the times of invoking sync to flush? About "Group commit": http://kristiannielsen.livejournal.com/12254.html
        Hide
        Andreas Hartmann-Schneevoigt added a comment -

        this sounds interesting. any news on that?

        Show
        Andreas Hartmann-Schneevoigt added a comment - this sounds interesting. any news on that?
        Hide
        Ludovic Orban added a comment -

        There are race conditions in the proposed patch and I never found time to look at them very closely. There is a similar effort in the 2.2 experimental branch with a new disk journal written with NIO from scratch. You may want to have a look at that instead.

        Show
        Ludovic Orban added a comment - There are race conditions in the proposed patch and I never found time to look at them very closely. There is a similar effort in the 2.2 experimental branch with a new disk journal written with NIO from scratch. You may want to have a look at that instead.
        Hide
        Brett Wooldridge added a comment -

        I believe the new concurrent disk journal in 3.0 addresses this enhancement request. It follows a similar approach, using NIO and ByteBuffer writes as well as concurrent journal writing and in-memory tracked in-flight transactions for fast journal rollover.

        Show
        Brett Wooldridge added a comment - I believe the new concurrent disk journal in 3.0 addresses this enhancement request. It follows a similar approach, using NIO and ByteBuffer writes as well as concurrent journal writing and in-memory tracked in-flight transactions for fast journal rollover.
        Hide
        Brett Wooldridge added a comment -

        Closing as fixed. Though this patch was not used, a similar scheme was implemented.

        Show
        Brett Wooldridge added a comment - Closing as fixed. Though this patch was not used, a similar scheme was implemented.

          People

          • Assignee:
            Ludovic Orban
            Reporter:
            dennis zhuang
          • Votes:
            1 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: