ISSUE-614: use native fallocate & sync_file_range to improve journal allocation #110

sijie · 2020-01-15T01:08:21Z

Original Issue: apache#614

JIRA: https://issues.apache.org/jira/browse/BOOKKEEPER-816

Reporter: Sijie Guo @sijie

it'd better to leverage filesystem fallocate & sync_file_range for journal performance.

Comments from JIRA

ASF GitHub Bot 2016-11-22T22:09:46.574+0000

GitHub user sijie opened a pull request:

https://github.com/apache/bookkeeper/pull/80

BOOKKEEPER-816: use native fallocate & sync_file_range to improve journal allocation

- leverage filesystem fallocate for better journal allocation
- update marker on journal replaying
- more stats

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/sijie/bookkeeper sijie/bookkeeper_fallocate

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/bookkeeper/pull/80.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #80

commit d980296
Author: Sijie Guo sijieg@twitter.com
Date: 2014-01-28T07:03:07Z

bookie: fallocate & sync_file_range

- introduce fallocate & sync_file_range in NativeIO to provide better preallocation & file sync logic.
- if journalAdaptiveGroupWrites is disabled, use sync_file_range to sync range in better granularity
- add more stats on journal flush & creation.

RB_ID=260795

commit 014512c
Author: Sijie Guo sijie@apache.org
Date: 2016-11-18T01:32:15Z

fix merge conflicts

commit 6077ef8
Author: Sijie Guo sijie@apache.org
Date: 2016-11-18T01:42:36Z

Fix

ASF GitHub Bot 2017-02-23T15:03:56.898+0000

Github user jvrao commented on the issue:

https://github.com/apache/bookkeeper/pull/80

@sijie  any perf numbers/stats to show how much improvement this achieved?

ASF GitHub Bot 2017-03-22T23:52:59.829+0000

Github user sijie commented on the issue:

https://github.com/apache/bookkeeper/pull/80

@jvrao we used to have the benchmark number. but I don't have it with me anymore.

ASF GitHub Bot 2017-03-22T23:53:40.319+0000

Github user sijie commented on the issue:

https://github.com/apache/bookkeeper/pull/80

this function is to use native system calls if possible. so if you don't use the jni library, the feature won't be enabled. so I think it is a safe change.

ASF GitHub Bot 2017-04-13T18:24:12.955+0000

Github user eolivelli commented on the issue:

https://github.com/apache/bookkeeper/pull/80

@jvrao are you ok with this patch?

ASF GitHub Bot 2017-04-13T18:49:34.143+0000

Github user athanatos commented on the issue:

https://github.com/apache/bookkeeper/pull/80

@sijie @eolivelli If I'm reading this right, you are trying to use sync_file_range to avoid calling FileChannel.force, which presumably calls fsync.  You actually can't use sync_file_range to ensure durability -- all it does it write out the dirty pages, it won't write out allocation information and (crucially) it won't issue the barriers required to flush volatile caches.

See http://man7.org/linux/man-pages/man2/sync_file_range.2.html, particularly:
"This system call does not flush disk write caches and thus does not provide any data integrity on systems with volatile disk write caches."

See also http://www.spinics.net/lists/ceph-devel/msg03995.html wherein Christoph Hellwig removes the same behavior in Ceph.

ASF GitHub Bot 2017-04-13T19:10:09.068+0000

Github user sijie commented on the issue:

https://github.com/apache/bookkeeper/pull/80

@athanatos there are two changes here - one is fallocate, while the other one is sync_range. we can remove sync_range part as we didn't use that.

ASF GitHub Bot 2017-04-18T18:22:04.588+0000

Github user sijie commented on the issue:

https://github.com/apache/bookkeeper/pull/80

rebased to latest master and remove 'sync_file_range'

ASF GitHub Bot 2017-04-18T18:22:16.690+0000

Github user sijie commented on the issue:

https://github.com/apache/bookkeeper/pull/80

/cc @jvrao and @merlimat for reviews

ASF GitHub Bot 2017-04-20T14:31:43.669+0000

Github user eolivelli commented on the issue:

https://github.com/apache/bookkeeper/pull/80

@sijie on the logs of tests (I'm running the build on Linux x64 Fedora) now I have plenty of these errors:

> 2017-04-20 16:07:01,815 - INFO  - [BookieJournal-15001:NativeIO@41] - Loading bookkeeper native library.
> 2017-04-20 16:07:01,815 - INFO  - [BookieJournal-15001:NativeIO@46] - Unable to load bookkeeper native library. Native methods will be disabled : 
> java.lang.UnsatisfiedLinkError: no bookkeeper in java.library.path
>         at java.lang.ClassLoader.loadLibrary(ClassLoader.java:1867)
>         at java.lang.Runtime.loadLibrary0(Runtime.java:870)
>         at java.lang.System.loadLibrary(System.java:1122)
>         at org.apache.bookkeeper.util.NativeIO.<clinit>(NativeIO.java:42)

Is there any way to:
- create less logs
- run the tests with the native library automatically in the library path ?

ASF GitHub Bot 2017-04-20T14:33:19.375+0000

Github user eolivelli commented on the issue:

https://github.com/apache/bookkeeper/pull/80

@sijie a question about packaging: are we going to bundle some kind of pre-built native library ?

Maybe we can create another JIRA in order to bundle a precompiled version and distribute with the Maven dependency so that by default clients will be able to leverage this new feature

ASF GitHub Bot 2017-04-20T17:36:47.257+0000

Github user sijie commented on the issue:

https://github.com/apache/bookkeeper/pull/80

updated the branch with missing license header

ASF GitHub Bot 2017-04-20T19:46:54.566+0000

Github user eolivelli commented on the issue:

https://github.com/apache/bookkeeper/pull/80

@sijie thanks for the update. I will check again tomorrow.

I see I was not clear about the maven central distribution and packaging. I will explain better.
I am not using the standard distribution for the bookie side but I only import BookKeeper-server dependency. For me it is not a problem I will include the native lib in a custom way. I will create a jira for an automatic unzip of the native library for the future, but it is not a blocker problem for me.

ASF GitHub Bot 2017-04-21T06:57:31.361+0000

Github user eolivelli commented on the issue:

https://github.com/apache/bookkeeper/pull/80

@sijie 
in order to load the native library during JUnit tests I have added these lines to bookkeeper-server/pom.xml

```
<plugin>
          <artifactId>maven-surefire-plugin</artifactId>
          <configuration>        
            <argLine>-Xmx2G -Djava.net.preferIPv4Stack=true -Djava.library.path=${project.build.directory}/native/target/usr/local/lib</argLine>
        </configuration>
 </plugin>
```
Now:
- no error in logs
- tests run like in "production mode"

new logs:

> 2017-04-21 08:53:51,595 - INFO  - [BookieJournal-15001:NativeIO@41] - Loading bookkeeper native library.
> 2017-04-21 08:53:51,596 - INFO  - [BookieJournal-15001:NativeIO@44] - Loaded bookkeeper native library. Enabled Native IO.

Do you think we can add that configuration to the pom.xml ?

ASF GitHub Bot 2017-04-21T08:49:26.924+0000

Github user eolivelli commented on the issue:

https://github.com/apache/bookkeeper/pull/80

@sijie 
I'm sorry but the compilation error is still here for me, I'm running Fedora 24

> Linux DNA101PC193.diennea.lan 4.10.8-100.fc24.x86_64 #1 SMP Fri Mar 31 13:20:57 UTC 2017 
> x86_64 x86_64 x86_64 GNU/Linux

This is the error:

>  [exec] -- Build files have been written to: /home/diennea.lan/enrico.olivelli/dev/eolivelli/bookkeeper/bookkeeper-server/target/native
>      [exec] /usr/bin/cmake -H/home/diennea.lan/enrico.olivelli/dev/eolivelli/bookkeeper/bookkeeper-server/src -B/home/diennea.lan/enrico.oli/home/diennea.lan/enrico.olivelli/dev/eolivelli/bookkeeper/bookkeeper-server/src/main/native/src/org/apache/bookkeeper/util/Nativelli/dev/eolivelli/bookkeeper/bookkeeper-server/target/native --check-build-system CMakeFiles/Makefile.cmake 0
>      [exec] /usr/bin/cmake -E cmake_progress_start /home/diennea.lan/enrico.olivelli/dev/eolivelli/bookkeeper/bookkeeper-server/target/native/CMakeFiles /home/diennea.lan/enrico.olivelli/dev/eolivelli/bookkeeper/bookkeeper-server/target/native/CMakeFiles/progress.marks
>      [exec] make -f CMakeFiles/Makefile2 all
>      [exec] make[1]: ingresso nella directory "/home/diennea.lan/enrico.olivelli/dev/eolivelli/bookkeeper/bookkeeper-server/target/native"
>      [exec] make -f CMakeFiles/bookkeeper_static.dir/build.make CMakeFiles/bookkeeper_static.dir/depend
>      [exec] make[2]: ingresso nella directory "/home/diennea.lan/enrico.olivelli/dev/eolivelli/bookkeeper/bookkeeper-server/target/native"
>      [exec] cd /home/diennea.lan/enrico.olivelli/dev/eolivelli/bookkeeper/bookkeeper-server/target/native && /usr/bin/cmake -E cmake_depends "Unix Makefiles" /veIO.c:29:35: fatal error: asm-x86_64/unistd.h: No such file or directory
>      [exec]      #include <asm-x86_64/unistd.h>
>      [exec]                                    ^
>      [exec] compilation terminated.
>      [exec] make[2]: *** [CMakeFiles/bookkeeper_static.dir/main/native/src/org/apache/bookkeeper/util/NativeIO.c.o] Errore 1
>      [exec] make[1]: *** [CMakeFiles/bookkeeper_static.dir/all] Errore 2

ASF GitHub Bot 2017-05-03T07:55:28.877+0000

Github user jiazhai commented on the issue:

https://github.com/apache/bookkeeper/pull/80

build success on ubuntu 16.04.
```
[INFO] bookkeeper ......................................... SUCCESS [  0.836 s]
[INFO] compability dependencies ........................... SUCCESS [  0.014 s]
[INFO] bookkeeper-server-compat400 ........................ SUCCESS [  9.552 s]
[INFO] bookkeeper-server-compat410 ........................ SUCCESS [  7.361 s]
[INFO] bookkeeper-server-compat420 ........................ SUCCESS [  8.026 s]
[INFO] Stats API for bookkeeper ........................... SUCCESS [  0.189 s]
[INFO] bookkeeper-server .................................. SUCCESS [ 12.366 s]
[INFO] bookkeeper-benchmark ............................... SUCCESS [  1.108 s]
[INFO] Stats provider for twitter-stats package ........... SUCCESS [  0.627 s]
[INFO] Stats provider for twitter-ostrich package ......... SUCCESS [  1.415 s]
[INFO] Stats provider for codahale metrics ................ SUCCESS [ 17.862 s]
[INFO] bookkeeper-stats-providers ......................... SUCCESS [  0.011 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 59.685 s
[INFO] Finished at: 2017-05-03T15:49:41+08:00
[INFO] Final Memory: 48M/715M
[INFO] ------------------------------------------------------------------------

jia@x-ubuntu:~/ws/code/bookkeeper-95028b9d85f6e0dbc932f44b440798ea4fb49d02$ uname -a
Linux x-ubuntu 4.8.0-49-generic #52~16.04.1-Ubuntu SMP Thu Apr 20 10:55:59 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
```

@eolivelli Is the build error related to this change?
It seems  not finding file <asm-x86_64/unistd.h> while make.
```
[exec] #include <asm-x86_64/unistd.h>
[exec] ^
[exec] compilation terminated.
```

ASF GitHub Bot 2017-05-03T08:12:45.784+0000

Github user eolivelli commented on the issue:

https://github.com/apache/bookkeeper/pull/80

@jiazhai on my Fedora laptop I do not have "asm-x86_64/unistd.h" but only "asm/unistd.h"

I'm investigating to check if by default Fedora is broken, and I am looking for the correct package to install.

ASF GitHub Bot 2017-05-03T08:13:45.060+0000

Github user eolivelli commented on the issue:

https://github.com/apache/bookkeeper/pull/80

@jiazhai are you using the -Pnative profile ? so that cmake runs ?

ASF GitHub Bot 2017-05-03T09:02:09.146+0000

Github user jiazhai commented on the issue:

https://github.com/apache/bookkeeper/pull/80

Oh. Sorry.  I did not use Pnative.

ASF GitHub Bot 2017-05-03T15:14:02.326+0000

Github user eolivelli commented on the issue:

https://github.com/apache/bookkeeper/pull/80

I get the same error on Centos 7

```
uname -a
Linux xxx.xxx.xxx 3.10.0-514.10.2.el7.x86_64 #1 SMP Fri Mar 3 00:04:05 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
```

```
   [exec] /bin/cc  -Dbookkeeper_EXPORTS -g -Wall -O2 -D_REENTRANT -D_GNU_SOURCE -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -fPIC -I/data2/dev/bookkeeper/bookkeeper-server/target/native/javah -I/data2/dev/bookkeeper/bookkeeper-server/src/main/native/src -I/data2/dev/bookkeeper/bookkeeper-server/src -I/data2/dev/bookkeeper/bookkeeper-server/src/src -I/data2/dev/bookkeeper/bookkeeper-server/target/native -I/usr/java/current/include -I/usr/java/current/include/linux    -o CMakeFiles/bookkeeper.dir/main/native/src/org/apache/bookkeeper/util/NativeIO.c.o   -c /data2/dev/bookkeeper/bookkeeper-server/src/main/native/src/org/apache/bookkeeper/util/NativeIO.c
     [exec] /data2/dev/bookkeeper/bookkeeper-server/src/main/native/src/org/apache/bookkeeper/util/NativeIO.c:29:35: fatal error: asm-x86_64/unistd.h: No such file or directory
     [exec]      #include <asm-x86_64/unistd.h>
     [exec]                                    ^
     [exec] compilation terminated.
     [exec] make[2]: *** [CMakeFiles/bookkeeper.dir/main/native/src/org/apache/bookkeeper/util/NativeIO.c.o] Error 1
     [exec] make[1]: *** [CMakeFiles/bookkeeper.dir/all] Error 2

```

ASF GitHub Bot 2017-05-09T10:08:37.822+0000

Github user eolivelli commented on the issue:

https://github.com/apache/bookkeeper/pull/80

I am absolutely a newbie in linux native code, but I think this is the problem
https://access.redhat.com/solutions/408503

As far as I can understand basically on RedHat based platforms (Fedora, CentOS...) all the contents of "asm-x86_64" have been merged in "asm"

I do not know a workaround, we should detect that we are in a redhat system (like checking /etc/redhat-release)

@athanatos  @sijie do you have any cycle to check this problem ?

ASF GitHub Bot 2017-05-18T11:07:38.761+0000

Github user eolivelli commented on the issue:

https://github.com/apache/bookkeeper/pull/80

@sijie 
I have pushed here a fix for builds on Fedora and Centos

can you pick up this commit too (if it sounds look good to you) ?
https://github.com/eolivelli/bookkeeper/tree/bookkeeper-816-native

maybe you could also add this property to the surefire configuration in order to use native libraries in JUnit tests if present

`-Djava.library.path=${project.build.directory}/native/target/usr/local/lib`

ASF GitHub Bot 2017-05-18T11:10:22.352+0000

Github user eolivelli commented on the issue:

https://github.com/apache/bookkeeper/pull/80

@jiazhai try to install package cmake then try again, maybe you will get new errors, and you will need to install locally additional libraries

ASF GitHub Bot 2017-05-28T20:00:54.646+0000

Github user eolivelli commented on the issue:

https://github.com/apache/bookkeeper/pull/80

@sijie we can merge this patch and I will create a follow up jira for redhat build support or we can merge my patch
Ok?

ASF GitHub Bot 2017-05-30T17:33:34.274+0000

Github user sijie commented on the issue:

https://github.com/apache/bookkeeper/pull/80

@eolivelli I will pick up your change. I will try to do it today.

ASF GitHub Bot 2017-06-04T13:18:22.312+0000

Github user eolivelli commented on the issue:

https://github.com/apache/bookkeeper/pull/80

I just got into this interesting reading about fallocate
https://groups.google.com/forum/m/#!topic/mechanical-sympathy/UMrKt75yOmg

I am ok with the current patch and the jni approach

ASF GitHub Bot 2017-06-09T08:32:01.505+0000

Github user eolivelli commented on the issue:

https://github.com/apache/bookkeeper/pull/80

@sijie did you have time to take a look to this ?

ASF GitHub Bot 2017-06-09T16:22:07.911+0000

Github user sijie commented on the issue:

https://github.com/apache/bookkeeper/pull/80

@eolivelli I haven't yet will do in the weekend.

ASF GitHub Bot 2017-06-27T20:41:02.336+0000

Github user eolivelli commented on the issue:

https://github.com/apache/bookkeeper/pull/80

@sijie I am going to use this improvement in order to make a necessary change in order to support java9. I would like to make 4.5 runnable on java9 as we are starting to test all of our apps and bookies cannot run on java9 without  hacks

ASF GitHub Bot 2017-06-27T20:52:17.444+0000

Github user sijie commented on the issue:

https://github.com/apache/bookkeeper/pull/80

@eolivelli thank you

ASF GitHub Bot 2017-06-30T08:08:13.063+0000

Github user eolivelli commented on the issue:

https://github.com/apache/bookkeeper/pull/80

@sijie do you have some cycle to go on with this issue ?
I apologize for being "annoying" on this topic

Sijie Guo 2017-08-01T21:36:26.184+0000

moved this to 4.6.0

The text was updated successfully, but these errors were encountered:

sijie added jiramigrated triage/week-33 type/task labels Jan 15, 2020

sijie added triage/week-33 and removed triage/week-33 labels Aug 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ISSUE-614: use native fallocate & sync_file_range to improve journal allocation #110

ISSUE-614: use native fallocate & sync_file_range to improve journal allocation #110

sijie commented Jan 15, 2020