You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@activemq.apache.org by ta...@apache.org on 2013/12/20 00:36:16 UTC

git commit: Fix for: https://issues.apache.org/jira/browse/AMQ-4947

Updated Branches:
  refs/heads/trunk 7656e8262 -> c50b6c39b


Fix for: https://issues.apache.org/jira/browse/AMQ-4947

Project: http://git-wip-us.apache.org/repos/asf/activemq/repo
Commit: http://git-wip-us.apache.org/repos/asf/activemq/commit/c50b6c39
Tree: http://git-wip-us.apache.org/repos/asf/activemq/tree/c50b6c39
Diff: http://git-wip-us.apache.org/repos/asf/activemq/diff/c50b6c39

Branch: refs/heads/trunk
Commit: c50b6c39babb9e0c4265e632aabf9c38f69c6730
Parents: 7656e82
Author: Timothy Bish <ta...@gmai.com>
Authored: Thu Dec 19 18:36:05 2013 -0500
Committer: Timothy Bish <ta...@gmai.com>
Committed: Thu Dec 19 18:36:05 2013 -0500

----------------------------------------------------------------------
 .../journal/CallerBufferingDataFileAppender.java  |  2 +-
 .../kahadb/disk/journal/DataFileAccessor.java     |  2 +-
 .../kahadb/disk/journal/DataFileAppender.java     |  2 +-
 .../activemq/store/kahadb/disk/page/PageFile.java | 10 +++++-----
 .../store/kahadb/disk/util/DiskBenchmark.java     |  6 +++---
 .../util/RecoverableRandomAccessFile.java         | 18 +++++++++++++++++-
 6 files changed, 28 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/activemq/blob/c50b6c39/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/CallerBufferingDataFileAppender.java
----------------------------------------------------------------------
diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/CallerBufferingDataFileAppender.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/CallerBufferingDataFileAppender.java
index ff11848..a6cce59 100644
--- a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/CallerBufferingDataFileAppender.java
+++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/CallerBufferingDataFileAppender.java
@@ -155,7 +155,7 @@ class CallerBufferingDataFileAppender extends DataFileAppender {
                 }
                 
                 if (forceToDisk) {
-                    file.getFD().sync();
+                	file.getChannel().force(false);
                 }
 
                 Journal.WriteCommand lastWrite = wb.writes.getTail();

http://git-wip-us.apache.org/repos/asf/activemq/blob/c50b6c39/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAccessor.java
----------------------------------------------------------------------
diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAccessor.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAccessor.java
index 7781b7e..11a99dc 100644
--- a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAccessor.java
+++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAccessor.java
@@ -155,7 +155,7 @@ final class DataFileAccessor {
         int size = Math.min(data.getLength(), location.getSize());
         file.write(data.getData(), data.getOffset(), size);
         if (sync) {
-            file.getFD().sync();
+        	file.getChannel().force(false);
         }
 
     }

http://git-wip-us.apache.org/repos/asf/activemq/blob/c50b6c39/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAppender.java
----------------------------------------------------------------------
diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAppender.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAppender.java
index 095db52..5f73d2a 100644
--- a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAppender.java
+++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAppender.java
@@ -365,7 +365,7 @@ class DataFileAppender implements FileAppender {
                 }
 
                 if (forceToDisk) {
-                    file.getFD().sync();
+                	file.getChannel().force(false);
                 }
 
                 Journal.WriteCommand lastWrite = wb.writes.getTail();

http://git-wip-us.apache.org/repos/asf/activemq/blob/c50b6c39/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/page/PageFile.java
----------------------------------------------------------------------
diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/page/PageFile.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/page/PageFile.java
index 3f107a6..508f698 100644
--- a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/page/PageFile.java
+++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/page/PageFile.java
@@ -610,10 +610,10 @@ public class PageFile {
         // So we don't loose it.. write it 2 times...
         writeFile.seek(0);
         writeFile.write(d);
-        writeFile.getFD().sync();
+        writeFile.getChannel().force(false);
         writeFile.seek(PAGE_FILE_HEADER_SIZE / 2);
         writeFile.write(d);
-        writeFile.getFD().sync();
+        writeFile.getChannel().force(false);
     }
 
     private void storeFreeList() throws IOException {
@@ -1081,9 +1081,9 @@ public class PageFile {
             if (enableDiskSyncs) {
                 // Sync to make sure recovery buffer writes land on disk..
                 if (enableRecoveryFile) {
-                    recoveryFile.getFD().sync();
+                	recoveryFile.getChannel().force(false);
                 }
-                writeFile.getFD().sync();
+                writeFile.getChannel().force(false);
             }
         } finally {
             synchronized (writes) {
@@ -1185,7 +1185,7 @@ public class PageFile {
         }
 
         // And sync it to disk
-        writeFile.getFD().sync();
+        writeFile.getChannel().force(false);
         return nextTxId;
     }
 

http://git-wip-us.apache.org/repos/asf/activemq/blob/c50b6c39/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/util/DiskBenchmark.java
----------------------------------------------------------------------
diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/util/DiskBenchmark.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/util/DiskBenchmark.java
index 2805f5f..4615fed 100644
--- a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/util/DiskBenchmark.java
+++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/util/DiskBenchmark.java
@@ -233,9 +233,9 @@ public class DiskBenchmark {
             }
             // Sync to disk so that the we actually write the data to disk.. otherwise 
             // OS buffering might not really do the write.
-            raf.getFD().sync();
+            raf.getChannel().force(false);
         }
-        raf.getFD().sync();
+        raf.getChannel().force(false);
         raf.close();
         now = System.currentTimeMillis();
         
@@ -254,7 +254,7 @@ public class DiskBenchmark {
             for( long i=0; i+data.length < size; i+=data.length) {
                 raf.seek(i);
                 raf.write(data);
-                raf.getFD().sync();
+                raf.getChannel().force(false);
                 ioCount++;
                 now = System.currentTimeMillis();
                 if( (now-start)>sampleInterval ) {

http://git-wip-us.apache.org/repos/asf/activemq/blob/c50b6c39/activemq-kahadb-store/src/main/java/org/apache/activemq/util/RecoverableRandomAccessFile.java
----------------------------------------------------------------------
diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/util/RecoverableRandomAccessFile.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/util/RecoverableRandomAccessFile.java
index 35c1586..5411ca0 100644
--- a/activemq-kahadb-store/src/main/java/org/apache/activemq/util/RecoverableRandomAccessFile.java
+++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/util/RecoverableRandomAccessFile.java
@@ -16,7 +16,12 @@
  */
 package org.apache.activemq.util;
 
-import java.io.*;
+import java.io.File;
+import java.io.FileDescriptor;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.io.RandomAccessFile;
+import java.nio.channels.FileChannel;
 
 public class RecoverableRandomAccessFile implements java.io.DataOutput, java.io.DataInput, java.io.Closeable {
 
@@ -388,6 +393,17 @@ public class RecoverableRandomAccessFile implements java.io.DataOutput, java.io.
             throw ioe;
         }
     }
+    
+    public FileChannel getChannel() throws IOException {
+    	
+    	try {
+    		return getRaf().getChannel();
+        } catch (IOException ioe)
+        {
+            handleException();
+            throw ioe;
+        }
+    }
 
     public int read(byte[] b, int off, int len) throws IOException {
         try {


Re: git commit: Fix for: https://issues.apache.org/jira/browse/AMQ-4947

Posted by Timothy Bish <ta...@gmail.com>.
I've reverted the change.  The underlying issue remains open.

On 01/06/2014 07:27 AM, Gary Tully wrote:
> I think we do. In particular when we check for corruption we base the
> index check on the file size of the data files so we need to ensure
> the file metadata (where the size is maintained) is in sync. Otherwise
> we can drop messages from the index due to them being outside the
> reported file size.
> In general, we can only depend on the behaviour described by the java
> api rather than any particular underlying os call used by a particular
> implementation. I think this change needs a revert.
>
> On 30 December 2013 15:51, Hiram Chirino <hi...@hiramchirino.com> wrote:
>> If all calls get switched to 'getChannel().force(false)' don't we run
>> the risk of loosing data on recovery due to the file size not being
>> consistent?  File size is part of the meta-data right?
>>
>> On Thu, Dec 19, 2013 at 6:36 PM,  <ta...@apache.org> wrote:
>>> Updated Branches:
>>>    refs/heads/trunk 7656e8262 -> c50b6c39b
>>>
>>>
>>> Fix for: https://issues.apache.org/jira/browse/AMQ-4947
>>>
>>> Project: http://git-wip-us.apache.org/repos/asf/activemq/repo
>>> Commit: http://git-wip-us.apache.org/repos/asf/activemq/commit/c50b6c39
>>> Tree: http://git-wip-us.apache.org/repos/asf/activemq/tree/c50b6c39
>>> Diff: http://git-wip-us.apache.org/repos/asf/activemq/diff/c50b6c39
>>>
>>> Branch: refs/heads/trunk
>>> Commit: c50b6c39babb9e0c4265e632aabf9c38f69c6730
>>> Parents: 7656e82
>>> Author: Timothy Bish <ta...@gmai.com>
>>> Authored: Thu Dec 19 18:36:05 2013 -0500
>>> Committer: Timothy Bish <ta...@gmai.com>
>>> Committed: Thu Dec 19 18:36:05 2013 -0500
>>>
>>> ----------------------------------------------------------------------
>>>   .../journal/CallerBufferingDataFileAppender.java  |  2 +-
>>>   .../kahadb/disk/journal/DataFileAccessor.java     |  2 +-
>>>   .../kahadb/disk/journal/DataFileAppender.java     |  2 +-
>>>   .../activemq/store/kahadb/disk/page/PageFile.java | 10 +++++-----
>>>   .../store/kahadb/disk/util/DiskBenchmark.java     |  6 +++---
>>>   .../util/RecoverableRandomAccessFile.java         | 18 +++++++++++++++++-
>>>   6 files changed, 28 insertions(+), 12 deletions(-)
>>> ----------------------------------------------------------------------
>>>
>>>
>>> http://git-wip-us.apache.org/repos/asf/activemq/blob/c50b6c39/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/CallerBufferingDataFileAppender.java
>>> ----------------------------------------------------------------------
>>> diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/CallerBufferingDataFileAppender.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/CallerBufferingDataFileAppender.java
>>> index ff11848..a6cce59 100644
>>> --- a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/CallerBufferingDataFileAppender.java
>>> +++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/CallerBufferingDataFileAppender.java
>>> @@ -155,7 +155,7 @@ class CallerBufferingDataFileAppender extends DataFileAppender {
>>>                   }
>>>
>>>                   if (forceToDisk) {
>>> -                    file.getFD().sync();
>>> +                       file.getChannel().force(false);
>>>                   }
>>>
>>>                   Journal.WriteCommand lastWrite = wb.writes.getTail();
>>>
>>> http://git-wip-us.apache.org/repos/asf/activemq/blob/c50b6c39/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAccessor.java
>>> ----------------------------------------------------------------------
>>> diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAccessor.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAccessor.java
>>> index 7781b7e..11a99dc 100644
>>> --- a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAccessor.java
>>> +++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAccessor.java
>>> @@ -155,7 +155,7 @@ final class DataFileAccessor {
>>>           int size = Math.min(data.getLength(), location.getSize());
>>>           file.write(data.getData(), data.getOffset(), size);
>>>           if (sync) {
>>> -            file.getFD().sync();
>>> +               file.getChannel().force(false);
>>>           }
>>>
>>>       }
>>>
>>> http://git-wip-us.apache.org/repos/asf/activemq/blob/c50b6c39/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAppender.java
>>> ----------------------------------------------------------------------
>>> diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAppender.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAppender.java
>>> index 095db52..5f73d2a 100644
>>> --- a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAppender.java
>>> +++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAppender.java
>>> @@ -365,7 +365,7 @@ class DataFileAppender implements FileAppender {
>>>                   }
>>>
>>>                   if (forceToDisk) {
>>> -                    file.getFD().sync();
>>> +                       file.getChannel().force(false);
>>>                   }
>>>
>>>                   Journal.WriteCommand lastWrite = wb.writes.getTail();
>>>
>>> http://git-wip-us.apache.org/repos/asf/activemq/blob/c50b6c39/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/page/PageFile.java
>>> ----------------------------------------------------------------------
>>> diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/page/PageFile.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/page/PageFile.java
>>> index 3f107a6..508f698 100644
>>> --- a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/page/PageFile.java
>>> +++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/page/PageFile.java
>>> @@ -610,10 +610,10 @@ public class PageFile {
>>>           // So we don't loose it.. write it 2 times...
>>>           writeFile.seek(0);
>>>           writeFile.write(d);
>>> -        writeFile.getFD().sync();
>>> +        writeFile.getChannel().force(false);
>>>           writeFile.seek(PAGE_FILE_HEADER_SIZE / 2);
>>>           writeFile.write(d);
>>> -        writeFile.getFD().sync();
>>> +        writeFile.getChannel().force(false);
>>>       }
>>>
>>>       private void storeFreeList() throws IOException {
>>> @@ -1081,9 +1081,9 @@ public class PageFile {
>>>               if (enableDiskSyncs) {
>>>                   // Sync to make sure recovery buffer writes land on disk..
>>>                   if (enableRecoveryFile) {
>>> -                    recoveryFile.getFD().sync();
>>> +                       recoveryFile.getChannel().force(false);
>>>                   }
>>> -                writeFile.getFD().sync();
>>> +                writeFile.getChannel().force(false);
>>>               }
>>>           } finally {
>>>               synchronized (writes) {
>>> @@ -1185,7 +1185,7 @@ public class PageFile {
>>>           }
>>>
>>>           // And sync it to disk
>>> -        writeFile.getFD().sync();
>>> +        writeFile.getChannel().force(false);
>>>           return nextTxId;
>>>       }
>>>
>>>
>>> http://git-wip-us.apache.org/repos/asf/activemq/blob/c50b6c39/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/util/DiskBenchmark.java
>>> ----------------------------------------------------------------------
>>> diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/util/DiskBenchmark.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/util/DiskBenchmark.java
>>> index 2805f5f..4615fed 100644
>>> --- a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/util/DiskBenchmark.java
>>> +++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/util/DiskBenchmark.java
>>> @@ -233,9 +233,9 @@ public class DiskBenchmark {
>>>               }
>>>               // Sync to disk so that the we actually write the data to disk.. otherwise
>>>               // OS buffering might not really do the write.
>>> -            raf.getFD().sync();
>>> +            raf.getChannel().force(false);
>>>           }
>>> -        raf.getFD().sync();
>>> +        raf.getChannel().force(false);
>>>           raf.close();
>>>           now = System.currentTimeMillis();
>>>
>>> @@ -254,7 +254,7 @@ public class DiskBenchmark {
>>>               for( long i=0; i+data.length < size; i+=data.length) {
>>>                   raf.seek(i);
>>>                   raf.write(data);
>>> -                raf.getFD().sync();
>>> +                raf.getChannel().force(false);
>>>                   ioCount++;
>>>                   now = System.currentTimeMillis();
>>>                   if( (now-start)>sampleInterval ) {
>>>
>>> http://git-wip-us.apache.org/repos/asf/activemq/blob/c50b6c39/activemq-kahadb-store/src/main/java/org/apache/activemq/util/RecoverableRandomAccessFile.java
>>> ----------------------------------------------------------------------
>>> diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/util/RecoverableRandomAccessFile.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/util/RecoverableRandomAccessFile.java
>>> index 35c1586..5411ca0 100644
>>> --- a/activemq-kahadb-store/src/main/java/org/apache/activemq/util/RecoverableRandomAccessFile.java
>>> +++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/util/RecoverableRandomAccessFile.java
>>> @@ -16,7 +16,12 @@
>>>    */
>>>   package org.apache.activemq.util;
>>>
>>> -import java.io.*;
>>> +import java.io.File;
>>> +import java.io.FileDescriptor;
>>> +import java.io.FileNotFoundException;
>>> +import java.io.IOException;
>>> +import java.io.RandomAccessFile;
>>> +import java.nio.channels.FileChannel;
>>>
>>>   public class RecoverableRandomAccessFile implements java.io.DataOutput, java.io.DataInput, java.io.Closeable {
>>>
>>> @@ -388,6 +393,17 @@ public class RecoverableRandomAccessFile implements java.io.DataOutput, java.io.
>>>               throw ioe;
>>>           }
>>>       }
>>> +
>>> +    public FileChannel getChannel() throws IOException {
>>> +
>>> +       try {
>>> +               return getRaf().getChannel();
>>> +        } catch (IOException ioe)
>>> +        {
>>> +            handleException();
>>> +            throw ioe;
>>> +        }
>>> +    }
>>>
>>>       public int read(byte[] b, int off, int len) throws IOException {
>>>           try {
>>>
>>
>>
>> --
>> Hiram Chirino
>>
>> Engineering | Red Hat, Inc.
>>
>> hchirino@redhat.com | fusesource.com | redhat.com
>>
>> skype: hiramchirino | twitter: @hiramchirino
>>
>> blog: Hiram Chirino's Bit Mojo
>
>


-- 
Tim Bish
Sr Software Engineer | RedHat Inc.
tim.bish@redhat.com | www.fusesource.com | www.redhat.com
skype: tabish121 | twitter: @tabish121
blog: http://timbish.blogspot.com/


Re: git commit: Fix for: https://issues.apache.org/jira/browse/AMQ-4947

Posted by Gary Tully <ga...@gmail.com>.
I think we do. In particular when we check for corruption we base the
index check on the file size of the data files so we need to ensure
the file metadata (where the size is maintained) is in sync. Otherwise
we can drop messages from the index due to them being outside the
reported file size.
In general, we can only depend on the behaviour described by the java
api rather than any particular underlying os call used by a particular
implementation. I think this change needs a revert.

On 30 December 2013 15:51, Hiram Chirino <hi...@hiramchirino.com> wrote:
> If all calls get switched to 'getChannel().force(false)' don't we run
> the risk of loosing data on recovery due to the file size not being
> consistent?  File size is part of the meta-data right?
>
> On Thu, Dec 19, 2013 at 6:36 PM,  <ta...@apache.org> wrote:
>> Updated Branches:
>>   refs/heads/trunk 7656e8262 -> c50b6c39b
>>
>>
>> Fix for: https://issues.apache.org/jira/browse/AMQ-4947
>>
>> Project: http://git-wip-us.apache.org/repos/asf/activemq/repo
>> Commit: http://git-wip-us.apache.org/repos/asf/activemq/commit/c50b6c39
>> Tree: http://git-wip-us.apache.org/repos/asf/activemq/tree/c50b6c39
>> Diff: http://git-wip-us.apache.org/repos/asf/activemq/diff/c50b6c39
>>
>> Branch: refs/heads/trunk
>> Commit: c50b6c39babb9e0c4265e632aabf9c38f69c6730
>> Parents: 7656e82
>> Author: Timothy Bish <ta...@gmai.com>
>> Authored: Thu Dec 19 18:36:05 2013 -0500
>> Committer: Timothy Bish <ta...@gmai.com>
>> Committed: Thu Dec 19 18:36:05 2013 -0500
>>
>> ----------------------------------------------------------------------
>>  .../journal/CallerBufferingDataFileAppender.java  |  2 +-
>>  .../kahadb/disk/journal/DataFileAccessor.java     |  2 +-
>>  .../kahadb/disk/journal/DataFileAppender.java     |  2 +-
>>  .../activemq/store/kahadb/disk/page/PageFile.java | 10 +++++-----
>>  .../store/kahadb/disk/util/DiskBenchmark.java     |  6 +++---
>>  .../util/RecoverableRandomAccessFile.java         | 18 +++++++++++++++++-
>>  6 files changed, 28 insertions(+), 12 deletions(-)
>> ----------------------------------------------------------------------
>>
>>
>> http://git-wip-us.apache.org/repos/asf/activemq/blob/c50b6c39/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/CallerBufferingDataFileAppender.java
>> ----------------------------------------------------------------------
>> diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/CallerBufferingDataFileAppender.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/CallerBufferingDataFileAppender.java
>> index ff11848..a6cce59 100644
>> --- a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/CallerBufferingDataFileAppender.java
>> +++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/CallerBufferingDataFileAppender.java
>> @@ -155,7 +155,7 @@ class CallerBufferingDataFileAppender extends DataFileAppender {
>>                  }
>>
>>                  if (forceToDisk) {
>> -                    file.getFD().sync();
>> +                       file.getChannel().force(false);
>>                  }
>>
>>                  Journal.WriteCommand lastWrite = wb.writes.getTail();
>>
>> http://git-wip-us.apache.org/repos/asf/activemq/blob/c50b6c39/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAccessor.java
>> ----------------------------------------------------------------------
>> diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAccessor.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAccessor.java
>> index 7781b7e..11a99dc 100644
>> --- a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAccessor.java
>> +++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAccessor.java
>> @@ -155,7 +155,7 @@ final class DataFileAccessor {
>>          int size = Math.min(data.getLength(), location.getSize());
>>          file.write(data.getData(), data.getOffset(), size);
>>          if (sync) {
>> -            file.getFD().sync();
>> +               file.getChannel().force(false);
>>          }
>>
>>      }
>>
>> http://git-wip-us.apache.org/repos/asf/activemq/blob/c50b6c39/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAppender.java
>> ----------------------------------------------------------------------
>> diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAppender.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAppender.java
>> index 095db52..5f73d2a 100644
>> --- a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAppender.java
>> +++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAppender.java
>> @@ -365,7 +365,7 @@ class DataFileAppender implements FileAppender {
>>                  }
>>
>>                  if (forceToDisk) {
>> -                    file.getFD().sync();
>> +                       file.getChannel().force(false);
>>                  }
>>
>>                  Journal.WriteCommand lastWrite = wb.writes.getTail();
>>
>> http://git-wip-us.apache.org/repos/asf/activemq/blob/c50b6c39/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/page/PageFile.java
>> ----------------------------------------------------------------------
>> diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/page/PageFile.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/page/PageFile.java
>> index 3f107a6..508f698 100644
>> --- a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/page/PageFile.java
>> +++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/page/PageFile.java
>> @@ -610,10 +610,10 @@ public class PageFile {
>>          // So we don't loose it.. write it 2 times...
>>          writeFile.seek(0);
>>          writeFile.write(d);
>> -        writeFile.getFD().sync();
>> +        writeFile.getChannel().force(false);
>>          writeFile.seek(PAGE_FILE_HEADER_SIZE / 2);
>>          writeFile.write(d);
>> -        writeFile.getFD().sync();
>> +        writeFile.getChannel().force(false);
>>      }
>>
>>      private void storeFreeList() throws IOException {
>> @@ -1081,9 +1081,9 @@ public class PageFile {
>>              if (enableDiskSyncs) {
>>                  // Sync to make sure recovery buffer writes land on disk..
>>                  if (enableRecoveryFile) {
>> -                    recoveryFile.getFD().sync();
>> +                       recoveryFile.getChannel().force(false);
>>                  }
>> -                writeFile.getFD().sync();
>> +                writeFile.getChannel().force(false);
>>              }
>>          } finally {
>>              synchronized (writes) {
>> @@ -1185,7 +1185,7 @@ public class PageFile {
>>          }
>>
>>          // And sync it to disk
>> -        writeFile.getFD().sync();
>> +        writeFile.getChannel().force(false);
>>          return nextTxId;
>>      }
>>
>>
>> http://git-wip-us.apache.org/repos/asf/activemq/blob/c50b6c39/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/util/DiskBenchmark.java
>> ----------------------------------------------------------------------
>> diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/util/DiskBenchmark.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/util/DiskBenchmark.java
>> index 2805f5f..4615fed 100644
>> --- a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/util/DiskBenchmark.java
>> +++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/util/DiskBenchmark.java
>> @@ -233,9 +233,9 @@ public class DiskBenchmark {
>>              }
>>              // Sync to disk so that the we actually write the data to disk.. otherwise
>>              // OS buffering might not really do the write.
>> -            raf.getFD().sync();
>> +            raf.getChannel().force(false);
>>          }
>> -        raf.getFD().sync();
>> +        raf.getChannel().force(false);
>>          raf.close();
>>          now = System.currentTimeMillis();
>>
>> @@ -254,7 +254,7 @@ public class DiskBenchmark {
>>              for( long i=0; i+data.length < size; i+=data.length) {
>>                  raf.seek(i);
>>                  raf.write(data);
>> -                raf.getFD().sync();
>> +                raf.getChannel().force(false);
>>                  ioCount++;
>>                  now = System.currentTimeMillis();
>>                  if( (now-start)>sampleInterval ) {
>>
>> http://git-wip-us.apache.org/repos/asf/activemq/blob/c50b6c39/activemq-kahadb-store/src/main/java/org/apache/activemq/util/RecoverableRandomAccessFile.java
>> ----------------------------------------------------------------------
>> diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/util/RecoverableRandomAccessFile.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/util/RecoverableRandomAccessFile.java
>> index 35c1586..5411ca0 100644
>> --- a/activemq-kahadb-store/src/main/java/org/apache/activemq/util/RecoverableRandomAccessFile.java
>> +++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/util/RecoverableRandomAccessFile.java
>> @@ -16,7 +16,12 @@
>>   */
>>  package org.apache.activemq.util;
>>
>> -import java.io.*;
>> +import java.io.File;
>> +import java.io.FileDescriptor;
>> +import java.io.FileNotFoundException;
>> +import java.io.IOException;
>> +import java.io.RandomAccessFile;
>> +import java.nio.channels.FileChannel;
>>
>>  public class RecoverableRandomAccessFile implements java.io.DataOutput, java.io.DataInput, java.io.Closeable {
>>
>> @@ -388,6 +393,17 @@ public class RecoverableRandomAccessFile implements java.io.DataOutput, java.io.
>>              throw ioe;
>>          }
>>      }
>> +
>> +    public FileChannel getChannel() throws IOException {
>> +
>> +       try {
>> +               return getRaf().getChannel();
>> +        } catch (IOException ioe)
>> +        {
>> +            handleException();
>> +            throw ioe;
>> +        }
>> +    }
>>
>>      public int read(byte[] b, int off, int len) throws IOException {
>>          try {
>>
>
>
>
> --
> Hiram Chirino
>
> Engineering | Red Hat, Inc.
>
> hchirino@redhat.com | fusesource.com | redhat.com
>
> skype: hiramchirino | twitter: @hiramchirino
>
> blog: Hiram Chirino's Bit Mojo



-- 
http://redhat.com
http://blog.garytully.com

Re: git commit: Fix for: https://issues.apache.org/jira/browse/AMQ-4947

Posted by Hiram Chirino <hi...@hiramchirino.com>.
If all calls get switched to 'getChannel().force(false)' don't we run
the risk of loosing data on recovery due to the file size not being
consistent?  File size is part of the meta-data right?

On Thu, Dec 19, 2013 at 6:36 PM,  <ta...@apache.org> wrote:
> Updated Branches:
>   refs/heads/trunk 7656e8262 -> c50b6c39b
>
>
> Fix for: https://issues.apache.org/jira/browse/AMQ-4947
>
> Project: http://git-wip-us.apache.org/repos/asf/activemq/repo
> Commit: http://git-wip-us.apache.org/repos/asf/activemq/commit/c50b6c39
> Tree: http://git-wip-us.apache.org/repos/asf/activemq/tree/c50b6c39
> Diff: http://git-wip-us.apache.org/repos/asf/activemq/diff/c50b6c39
>
> Branch: refs/heads/trunk
> Commit: c50b6c39babb9e0c4265e632aabf9c38f69c6730
> Parents: 7656e82
> Author: Timothy Bish <ta...@gmai.com>
> Authored: Thu Dec 19 18:36:05 2013 -0500
> Committer: Timothy Bish <ta...@gmai.com>
> Committed: Thu Dec 19 18:36:05 2013 -0500
>
> ----------------------------------------------------------------------
>  .../journal/CallerBufferingDataFileAppender.java  |  2 +-
>  .../kahadb/disk/journal/DataFileAccessor.java     |  2 +-
>  .../kahadb/disk/journal/DataFileAppender.java     |  2 +-
>  .../activemq/store/kahadb/disk/page/PageFile.java | 10 +++++-----
>  .../store/kahadb/disk/util/DiskBenchmark.java     |  6 +++---
>  .../util/RecoverableRandomAccessFile.java         | 18 +++++++++++++++++-
>  6 files changed, 28 insertions(+), 12 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/activemq/blob/c50b6c39/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/CallerBufferingDataFileAppender.java
> ----------------------------------------------------------------------
> diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/CallerBufferingDataFileAppender.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/CallerBufferingDataFileAppender.java
> index ff11848..a6cce59 100644
> --- a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/CallerBufferingDataFileAppender.java
> +++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/CallerBufferingDataFileAppender.java
> @@ -155,7 +155,7 @@ class CallerBufferingDataFileAppender extends DataFileAppender {
>                  }
>
>                  if (forceToDisk) {
> -                    file.getFD().sync();
> +                       file.getChannel().force(false);
>                  }
>
>                  Journal.WriteCommand lastWrite = wb.writes.getTail();
>
> http://git-wip-us.apache.org/repos/asf/activemq/blob/c50b6c39/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAccessor.java
> ----------------------------------------------------------------------
> diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAccessor.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAccessor.java
> index 7781b7e..11a99dc 100644
> --- a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAccessor.java
> +++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAccessor.java
> @@ -155,7 +155,7 @@ final class DataFileAccessor {
>          int size = Math.min(data.getLength(), location.getSize());
>          file.write(data.getData(), data.getOffset(), size);
>          if (sync) {
> -            file.getFD().sync();
> +               file.getChannel().force(false);
>          }
>
>      }
>
> http://git-wip-us.apache.org/repos/asf/activemq/blob/c50b6c39/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAppender.java
> ----------------------------------------------------------------------
> diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAppender.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAppender.java
> index 095db52..5f73d2a 100644
> --- a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAppender.java
> +++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/journal/DataFileAppender.java
> @@ -365,7 +365,7 @@ class DataFileAppender implements FileAppender {
>                  }
>
>                  if (forceToDisk) {
> -                    file.getFD().sync();
> +                       file.getChannel().force(false);
>                  }
>
>                  Journal.WriteCommand lastWrite = wb.writes.getTail();
>
> http://git-wip-us.apache.org/repos/asf/activemq/blob/c50b6c39/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/page/PageFile.java
> ----------------------------------------------------------------------
> diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/page/PageFile.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/page/PageFile.java
> index 3f107a6..508f698 100644
> --- a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/page/PageFile.java
> +++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/page/PageFile.java
> @@ -610,10 +610,10 @@ public class PageFile {
>          // So we don't loose it.. write it 2 times...
>          writeFile.seek(0);
>          writeFile.write(d);
> -        writeFile.getFD().sync();
> +        writeFile.getChannel().force(false);
>          writeFile.seek(PAGE_FILE_HEADER_SIZE / 2);
>          writeFile.write(d);
> -        writeFile.getFD().sync();
> +        writeFile.getChannel().force(false);
>      }
>
>      private void storeFreeList() throws IOException {
> @@ -1081,9 +1081,9 @@ public class PageFile {
>              if (enableDiskSyncs) {
>                  // Sync to make sure recovery buffer writes land on disk..
>                  if (enableRecoveryFile) {
> -                    recoveryFile.getFD().sync();
> +                       recoveryFile.getChannel().force(false);
>                  }
> -                writeFile.getFD().sync();
> +                writeFile.getChannel().force(false);
>              }
>          } finally {
>              synchronized (writes) {
> @@ -1185,7 +1185,7 @@ public class PageFile {
>          }
>
>          // And sync it to disk
> -        writeFile.getFD().sync();
> +        writeFile.getChannel().force(false);
>          return nextTxId;
>      }
>
>
> http://git-wip-us.apache.org/repos/asf/activemq/blob/c50b6c39/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/util/DiskBenchmark.java
> ----------------------------------------------------------------------
> diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/util/DiskBenchmark.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/util/DiskBenchmark.java
> index 2805f5f..4615fed 100644
> --- a/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/util/DiskBenchmark.java
> +++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/disk/util/DiskBenchmark.java
> @@ -233,9 +233,9 @@ public class DiskBenchmark {
>              }
>              // Sync to disk so that the we actually write the data to disk.. otherwise
>              // OS buffering might not really do the write.
> -            raf.getFD().sync();
> +            raf.getChannel().force(false);
>          }
> -        raf.getFD().sync();
> +        raf.getChannel().force(false);
>          raf.close();
>          now = System.currentTimeMillis();
>
> @@ -254,7 +254,7 @@ public class DiskBenchmark {
>              for( long i=0; i+data.length < size; i+=data.length) {
>                  raf.seek(i);
>                  raf.write(data);
> -                raf.getFD().sync();
> +                raf.getChannel().force(false);
>                  ioCount++;
>                  now = System.currentTimeMillis();
>                  if( (now-start)>sampleInterval ) {
>
> http://git-wip-us.apache.org/repos/asf/activemq/blob/c50b6c39/activemq-kahadb-store/src/main/java/org/apache/activemq/util/RecoverableRandomAccessFile.java
> ----------------------------------------------------------------------
> diff --git a/activemq-kahadb-store/src/main/java/org/apache/activemq/util/RecoverableRandomAccessFile.java b/activemq-kahadb-store/src/main/java/org/apache/activemq/util/RecoverableRandomAccessFile.java
> index 35c1586..5411ca0 100644
> --- a/activemq-kahadb-store/src/main/java/org/apache/activemq/util/RecoverableRandomAccessFile.java
> +++ b/activemq-kahadb-store/src/main/java/org/apache/activemq/util/RecoverableRandomAccessFile.java
> @@ -16,7 +16,12 @@
>   */
>  package org.apache.activemq.util;
>
> -import java.io.*;
> +import java.io.File;
> +import java.io.FileDescriptor;
> +import java.io.FileNotFoundException;
> +import java.io.IOException;
> +import java.io.RandomAccessFile;
> +import java.nio.channels.FileChannel;
>
>  public class RecoverableRandomAccessFile implements java.io.DataOutput, java.io.DataInput, java.io.Closeable {
>
> @@ -388,6 +393,17 @@ public class RecoverableRandomAccessFile implements java.io.DataOutput, java.io.
>              throw ioe;
>          }
>      }
> +
> +    public FileChannel getChannel() throws IOException {
> +
> +       try {
> +               return getRaf().getChannel();
> +        } catch (IOException ioe)
> +        {
> +            handleException();
> +            throw ioe;
> +        }
> +    }
>
>      public int read(byte[] b, int off, int len) throws IOException {
>          try {
>



-- 
Hiram Chirino

Engineering | Red Hat, Inc.

hchirino@redhat.com | fusesource.com | redhat.com

skype: hiramchirino | twitter: @hiramchirino

blog: Hiram Chirino's Bit Mojo