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 <[email protected]> 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,  <[email protected]> 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 <[email protected]>
Authored: Thu Dec 19 18:36:05 2013 -0500
Committer: Timothy Bish <[email protected]>
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.

[email protected] | fusesource.com | redhat.com

skype: hiramchirino | twitter: @hiramchirino

blog: Hiram Chirino's Bit Mojo




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

Reply via email to