Skip to content

Conversation

judovana
Copy link

This PR is using new libraries created in lmdbjava/native#7 .
The usage for this "secondary architecture" is now somethng like:

  ARCH=`uname -p`
 ...
    if [ "$ARCH" == "x86_64" ] ; then
     mvn clean install -P linux-x86_64 -P \!linux
    elif  [ "$ARCH" == "aarch64" ] ; then
      mvn clean install -P linux-aarch64 -P \!linux
    elif  [ "$ARCH" == "ppc64le" ] ; then
      mvn clean install -P linux-ppc64le -P \!linux
   else 
     echo "Not building native, invalid or other upstream-supported platform."
  ...

Whole concept seems to be working, with two glitches:

  • optional for native jars do not work, so even when building solemn aarch64 you need the ppc64le (as they are not yet in maven repos) and similiarly you need aarch64 when building ppc64le, and yo need them both when you are now building gneric linux profiles (looking for advice in this topic)
    • you can easily fake them, but it is dangerous
  • Second issue is more scary. Unit tests fails on both ppc64le and aarch64. I tried both with head and the crucial commit of lmdb of 0c357cc88a00bda03aa4a982fc227a5872707df2 . On contrary intel is not failing also with head lmdb.

I have to be doing something wrong, or lmdbjava must be doing somethng I faild to discover, as lmdb itself seems to work fine on aarch64/ppc64le. Both based on linux ditributions usages and on unittests.

ERROR] Failures:
[ERROR] DbiTest.stats:460
Expected: is <4096>
but: was <32768>
[ERROR] EnvTest.stats:393
Expected: is <4096>
but: was <32768>
[ERROR] Errors:
[ERROR] CursorIterableTest.before:132 » MapFull Environment mapsize reached (-30792)
... Skipped 11 identical lines...
[ERROR] CursorIterableTest.before:132 » MapFull Environment mapsize reached (-30792)
[ERROR] EnvTest.setMapSize:341 » MapFull Environment mapsize reached (-30792)
[ERROR] TxnTest.largeKeysRejected » Unexpected exception, expected<org.lmdbjava.Dbi$B...
[ERROR] TxnTest.rangeSearch:103 » MapFull Environment mapsize reached (-30792)
[ERROR] TxnTest.readOnlyTxnAllowedInReadOnlyEnv:138 » MapFull Environment mapsize reac...
[ERROR] TxnTest.readWriteTxnDeniedInReadOnlyEnv » Unexpected exception, expected<org....
[ERROR] TxnTest.testGetId:181 » MapFull Environment mapsize reached (-30792)
[ERROR] TxnTest.zeroByteKeysRejected » Unexpected exception, expected<org.lmdbjava.Db...
[INFO]
[ERROR] Tests run: 187, Failures: 2, Errors: 31, Skipped: 0

Before i spam OpenLDAP, do you have any idea what may be going on?

@judovana
Copy link
Author

The checks are failing as expected due to the issue with missing ppc64le/aarch64 binaries:

[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 14.337 s
[INFO] Finished at: 2020-08-17T12:45:50Z
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal on project lmdbjava: Could not resolve dependencies for project org.lmdbjava:lmdbjava:jar:0.8.2-SNAPSHOT: The following artifacts could not be resolved: org.lmdbjava:lmdbjava-native-linux-aarch64:jar:0.9.24-2-SNAPSHOT, org.lmdbjava:lmdbjava-native-linux-ppc64le:jar:0.9.24-2-SNAPSHOT: Could not find artifact org.lmdbjava:lmdbjava-native-linux-aarch64:jar:0.9.24-2-SNAPSHOT in sonatype-snapshots (https://oss.sonatype.org/content/repositories/snapshots) -> [Help 1]

This can be easly fixed in pom.xml or yaml running the pom, but I would rather discusse the "secondary arches" with you first.

@Karm
Copy link

Karm commented Sep 2, 2020

Hello @judovana,

There are 2 major problem with the test suite and possibly with the Java API as such:

4096 is not the page size on all systems

The default expectation of LMDB Java is that the page size on the system is 4096. That does not hold. Ultimately my aarch64 and ppc64le (Power 9) systems running RHEL 8 report:

#  getconf PAGESIZE
65536

LMDB Java reads that as 1/2, i.e.

final Stat stat;
try (Txn<ByteBuffer> txn = env.txnRead()) {
  stat = db.stat(txn);
}

gives 32768 as the page size in stat.pageSize. Not 4096.

LMDB native API uses pages, not bytes

The LMDB Java code looks as if LIB.mdb_env_set_mapsize takes a number of bytes while in fact, the native API uses number of pages. e.g. (an example from a C app):

/** Maximum size of the DB in multiples of page size in bytes
 * e.g. on my system: getconf PAGESIZE is 4096 so setting 1048576 here gives 4GB
 * max LMDB DB size.*/
#define IPRANGER_MAX_MAP_SIZE_IN_PAGES 1048576

 E(mdb_env_set_mapsize(env, sysconf(_SC_PAGESIZE) *
                                 IPRANGER_MAX_MAP_SIZE_IN_PAGES));

So, in our case on these aarch64 and ppc64le systems, their 32768 is 8 times more than the expected default of 4096.
Introducing magic number 8 makes the test pass on these systems:

diff --git a/src/main/java/org/lmdbjava/Env.java b/src/main/java/org/lmdbjava/Env.java
index 7634a34..c9c3397 100644
--- a/src/main/java/org/lmdbjava/Env.java
+++ b/src/main/java/org/lmdbjava/Env.java
@@ -486,7 +486,7 @@ public final class Env<T> implements AutoCloseable {
       checkRc(LIB.mdb_env_create(envPtr));
       final Pointer ptr = envPtr.getValue();
       try {
-        checkRc(LIB.mdb_env_set_mapsize(ptr, mapSize));
+        checkRc(LIB.mdb_env_set_mapsize(ptr, 8 * mapSize));
         checkRc(LIB.mdb_env_set_maxdbs(ptr, maxDbs));
         checkRc(LIB.mdb_env_set_maxreaders(ptr, maxReaders));
         final int flagsMask = mask(flags);
diff --git a/src/test/java/org/lmdbjava/DbiTest.java b/src/test/java/org/lmdbjava/DbiTest.java
index b9bf453..c35c440 100644
--- a/src/test/java/org/lmdbjava/DbiTest.java
+++ b/src/test/java/org/lmdbjava/DbiTest.java
@@ -457,7 +457,6 @@ public final class DbiTest {
     assertThat(stat.entries, is(3L));
     assertThat(stat.leafPages, is(1L));
     assertThat(stat.overflowPages, is(0L));
-    assertThat(stat.pageSize, is(4_096));
   }
 
   @Test(expected = MapFullException.class)
diff --git a/src/test/java/org/lmdbjava/EnvTest.java b/src/test/java/org/lmdbjava/EnvTest.java
index 3e9ece9..811b18c 100644
--- a/src/test/java/org/lmdbjava/EnvTest.java
+++ b/src/test/java/org/lmdbjava/EnvTest.java
@@ -26,6 +26,7 @@ import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.greaterThanOrEqualTo;
 import static org.hamcrest.Matchers.greaterThan;
 import static org.lmdbjava.CopyFlags.MDB_CP_COMPACT;
 import static org.lmdbjava.DbiFlags.MDB_CREATE;
@@ -68,7 +69,8 @@ public final class EnvTest {
         .setMapSize(MEBIBYTES.toBytes(1))
         .open(path, MDB_NOSUBDIR)) {
       final EnvInfo info = env.info();
-      assertThat(info.mapSize, is(MEBIBYTES.toBytes(1)));
+      // `is' does not make sense, one API uses bytes, the other uses pages...
+      assertThat(info.mapSize, greaterThanOrEqualTo(MEBIBYTES.toBytes(1)));
     }
   }
 
@@ -277,7 +279,7 @@ public final class EnvTest {
       assertThat(info.lastPageNumber, is(1L));
       assertThat(info.lastTransactionId, is(0L));
       assertThat(info.mapAddress, is(0L));
-      assertThat(info.mapSize, is(123_456L));
+      assertThat(info.mapSize, is(8 * 123_456L));
       assertThat(info.maxReaders, is(4));
       assertThat(info.numReaders, is(0));
       assertThat(info.toString(), containsString("maxReaders="));
@@ -355,7 +357,7 @@ public final class EnvTest {
       }
       assertThat(mapFullExThrown, is(true));
 
-      env.setMapSize(500_000);
+      env.setMapSize(8 * 500_000);
 
       try (Txn<ByteBuffer> roTxn = env.txnRead()) {
         assertThat(db.get(roTxn, bb(1)).getInt(), is(42));
@@ -390,7 +392,6 @@ public final class EnvTest {
       assertThat(stat.entries, is(0L));
       assertThat(stat.leafPages, is(0L));
       assertThat(stat.overflowPages, is(0L));
-      assertThat(stat.pageSize, is(4_096));
       assertThat(stat.toString(), containsString("pageSize="));
     }
   }
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running org.lmdbjava.KeyRangeTest
[INFO] Tests run: 19, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.15 s - in org.lmdbjava.KeyRangeTest
[INFO] Running org.lmdbjava.CursorParamTest
[INFO] Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.24 s - in org.lmdbjava.CursorParamTest
[INFO] Running org.lmdbjava.TxnTest
[INFO] Tests run: 21, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.092 s - in org.lmdbjava.TxnTest
[INFO] Running org.lmdbjava.ResultCodeMapperTest
[INFO] Tests run: 9, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.023 s - in org.lmdbjava.ResultCodeMapperTest
[INFO] Running org.lmdbjava.LibraryTest
[INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 s - in org.lmdbjava.LibraryTest
[INFO] Running org.lmdbjava.MaskedFlagTest
[INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 s - in org.lmdbjava.MaskedFlagTest
[INFO] Running org.lmdbjava.DbiTest
[INFO] Tests run: 24, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 34.76 s - in org.lmdbjava.DbiTest
[INFO] Running org.lmdbjava.CursorIterableTest
[INFO] Tests run: 24, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.192 s - in org.lmdbjava.CursorIterableTest
[INFO] Running org.lmdbjava.MetaTest
[INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0 s - in org.lmdbjava.MetaTest
[INFO] Running org.lmdbjava.EnvTest
[INFO] Tests run: 24, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 36.082 s - in org.lmdbjava.EnvTest
[INFO] Running org.lmdbjava.ComparatorTest
[INFO] Tests run: 21, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.028 s - in org.lmdbjava.ComparatorTest
[INFO] Running org.lmdbjava.VerifierTest
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.022 s - in org.lmdbjava.VerifierTest
[INFO] Running org.lmdbjava.CursorTest
[INFO] Tests run: 15, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.061 s - in org.lmdbjava.CursorTest
[INFO] Running org.lmdbjava.ByteBufferProxyTest
[INFO] Tests run: 10, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.01 s - in org.lmdbjava.ByteBufferProxyTest
[INFO] Running org.lmdbjava.TutorialTest
[INFO] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 3.138 s - in org.lmdbjava.TutorialTest
[INFO] 
[INFO] Results:
[INFO] 
[INFO] Tests run: 187, Failures: 0, Errors: 0, Skipped: 0

Fix

I don't know how to solve it elegantly at the moment. I propose dropping asserts for hardcoded 4096 and introduce calculating max size dynamically according to what LMDB C lib says is the page size on the system.

It would help to get a comment from maintainers. I can implement the fix if you hint what would be a good direction.

@Karm
Copy link

Karm commented Sep 3, 2020

Perhaps @bambulin might know about the API discrepancy. Kozliku, did you use bytes or pages for setting mdb_env_set_mapsize? In my C app, using C API, I definitely used the size expressed as a number of pages. This Java API seems to be expecting number of bytes though... Ideas?

@bambulin
Copy link

bambulin commented Sep 3, 2020

@Karm we don't set the flag directly - the Java API offers Env.create().setMapSize() where the size can be set. In version we use the method expects the size in bytes, however the latest version expects megabytes.

@judovana
Copy link
Author

judovana commented Sep 8, 2020

@Karm can the magical 8 be please extracted to constant?
I will merge then, and see how lmdb java upstream reacts...

If lmdb is able to return what size it expects, it sounds as good fix. I do not htink removal od asserts is god idea. It can hide an bug later hard to find.

Until the fix is presented, the constant hsould be at least filed by if arch depndence,

@Karm
Copy link

Karm commented Sep 25, 2020

This PR can be closed as it was surpassed by #165

All tests pass on all secondary archs now (and amd64 is not broken either :) )

@benalexau
Copy link
Member

This PR can be closed as it was surpassed by #165

Closing as requested; will review in #165.

@benalexau benalexau closed this Oct 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants