Abstract merge operator in Java + Thread JNI attachment automatic management + loader management for static variables by publicocean0 · Pull Request #3432 · facebook/rocksdb

publicocean0 · 2018-01-29T15:34:42Z

This pull request add AbstractAssociativeMergeOperator and AbstractNotAssociativeMergeOperator in java. These classes are abstract permitting to implement the merge operator in java code.

This pull request introduces also 2 important performance improvements(they are generic applicable to all jni classes):

The JNI Thread attachment is a expensive operation. Doing it after every api operation can reduce performance in significant way. This patch introduces a automatic management for attachment/detachment: the attachment will be executed once and the detachment will be executed just before thread termination in automatic way.
Many static variables actually are loaded on the fly inside the jni api. This patch introduce a loader/unloader management for preloading all static/global variables for the jni modules when JVM is loaded. This permits to remove the loading time inside the jni api. For Adding a loader/unloader in a JNI class is very simple: this is a example

 funcInit(JNIEnv* env){
   load symbols , jclasses,methodids,fieldids, globalRefs
 }
 funcDestroy(JNIEnv* env){
   unload globalRefs
 }
   initialize {

rocksdb::setLoader( &funcInit);
rocksdb::setUnloader( &funcDestroy);

}

The thread attachment management and loading management are used by merge operator for optimizing the execution time

…ads+ loaders/unloaders for static vars

facebook-github-bot · 2018-01-29T15:35:02Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

publicocean0 · 2018-01-29T15:50:17Z

In the branch it is present also my transaction patch. Eventually if adamitter commited his transaction branch , i can merge( or remove mime) so we are aligned with master branch. The current patch is contained just in commits
6e73f37,6e0c351.

facebook-github-bot · 2018-01-29T16:50:27Z

@publicocean0 has updated the pull request. View: changes

facebook-github-bot · 2018-01-30T08:22:01Z

@publicocean0 has updated the pull request. View: changes

facebook-github-bot · 2018-01-30T15:30:53Z

@publicocean0 has updated the pull request. View: changes

…act calls. Rockdb secondary threads are are not joined correctly

facebook-github-bot · 2018-02-01T01:04:50Z

@publicocean0 has updated the pull request. View: changes

publicocean0 · 2018-02-01T01:19:39Z

Found a probable bug in rocksdb when db is closing: threads not joined in synchronous way with delete db instruction. #3453. i dont find any way for waiting the rocksdb threads are terminated after db deletion.

…implier to review

facebook-github-bot · 2018-02-01T16:30:37Z

@publicocean0 has updated the pull request. View: changes

benclay

Automatic thread attach / detach would be super helpful, it's something I ran into with #3338 and was hoping for solutions.

@ajkr mentioned they are working on some systemic approach to solve this, I am not sure what stage that effort is in.

@publicocean0 my personal preference would be to split the attachment management from the abstract merge operator to reduce size of the PRs. Then there's no example of using the automatic thread attach / detach but I think would make this easier to reviewers to parse.

benclay · 2018-02-07T16:16:29Z

 CMakeFiles/
 build/
-
+cmake-build-debug/


Needed? Looks like it got included anyway.

benclay · 2018-02-07T16:16:35Z

      util/fault_injection_test_env.cc
      utilities/cassandra/test_utils.cc
-  )
+          java/rocksjni/init.h java/rocksjni/init.cc)


benclay · 2018-02-07T16:16:44Z

@@ -0,0 +1,242 @@
+# CMAKE generated file: DO NOT EDIT!


Remove this entire directory, it's generated

benclay · 2018-02-07T16:17:32Z

 #define EXT4_SUPER_MAGIC 0xEF53
 #endif

+


Undo all whitespace changes

benclay · 2018-02-07T16:17:45Z

 	org.rocksdb.WriteBatchWithIndex\
-	org.rocksdb.WBWIRocksIterator
+	org.rocksdb.WBWIRocksIterator\
+    org.rocksdb.AbstractAssociativeMergeOperator\


Sort, fix indent

benclay · 2018-02-07T16:24:06Z

    RocksDB.loadLibrary();
  }

+


Undo whitespace changes

benclay · 2018-02-07T16:24:50Z

+     }
+     StringBuffer sb=new StringBuffer(new String(oldvalue));
+     sb.append(',');
+     //if (1==1)throw new IndexOutOfBoundsException();


Remove debug / commented lines in this file

benclay · 2018-02-07T16:25:00Z

          "xxxx".getBytes());
    }
  }
+ private class M extends AbstractAssociativeMergeOperator{


Can you move this and the associated tests to its own test file?

Ok tests works but there is a problem to solve for having the best performance. The performance is anyway better using initialization strategy because it preloads all what is possible(jvm variables handles loading consumes cpu).
Threads lanched by rocksdb are not terminated in synchronous way with "delete db"(pratically rocks threads are terminated after (when process is closing )so it happens jvm is closed before rocksdb threads are closed creating a memory leak. For evoiding it for now i create context as before (create-destroy cycle) but in future i will remove it solving this problem. This problem appear just with rocksdb threads not with java threads or c++ threads created on the fly if you use join in the end. ).
For solving the problem i might add a simple method in posix-env joining all threads inside thread pool.

benclay · 2018-02-07T16:25:19Z

+    }
+  }
+
+  //@Test


benclay · 2018-02-07T16:25:48Z

  utilities/write_batch_with_index/write_batch_with_index_test.cc       \

 JNI_NATIVE_SOURCES =                                          \
+  java/rocksjni/init.cc                                       \


maysamyabandeh · 2018-07-27T18:55:15Z

Closing it since there was no interest shown to have it merged.

adamretter · 2018-07-29T07:24:06Z

@maysamyabandeh Somehow I missed this. I will reopen it if you don't mind as it sounds like there are some useful additions in this branch such as the thread attach/detacth

maysamyabandeh · 2019-09-09T21:16:53Z

@adamretter any update on this?

adamretter · 2019-12-13T18:09:58Z

@publicocean0 Okay I pushed a commit to your branch to get it to compile in its current state on MacOS.

I also created a branch where I rebased it onto HEAD and stripped out the transaction stuff from the history - https://github.com/adamretter/rocksdb/tree/abstractMergeOperator-REBASED

However, when I run the testsuite on your branch via:

$ ROCKSDB_DISABLE_JEMALLOC=true DEBUG_LEVEL=2 make jtest_compile
$ ROCKSDB_DISABLE_JEMALLOC=true DEBUG_LEVEL=2 make jtest_run

I get a SIGSEGV in your new merge operator code:

Run: org.rocksdb.RocksDBTest testing now -> merge1
ReleasePrimitiveArrayCritical: release array failed bounds check, incorrect pointer returned ? array: 0x00007fdbb25360c0 carray: 0x000070000dce42e9
GuardedMemory(0x000070000dce3308) base_addr=0x000070000dce42c9 tag=0x0c000070000dce42 user_size=17654111020329586242 user_data=0x000070000dce42e9
  Header guard @0x000070000dce42c9 is BROKEN
#
# A fatal error has been detected by the Java Runtime Environment:
#
#  SIGSEGV (0xb) at pc=0x000000010de97f32, pid=87752, tid=0x0000000000007e1b
#
# JRE version: OpenJDK Runtime Environment (Zulu 8.40.0.25-CA-macosx) (8.0_222-b10) (build 1.8.0_222-b10)
# Java VM: OpenJDK 64-Bit Server VM (25.222-b10 mixed mode bsd-amd64 compressed oops)
# Problematic frame:
# V  [libjvm.dylib+0x297f32]  GuardedMemory::print_on(outputStream*) const+0xac
...

The full-stack trace from the hs_err log file then looks like:

Stack: [0x000070000dbe5000,0x000070000dce5000],  sp=0x000070000dce32c0,  free space=1016k
Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
V  [libjvm.dylib+0x297f32]  GuardedMemory::print_on(outputStream*) const+0xac
V  [libjvm.dylib+0x34ed6b]  check_wrapped_array_release(JavaThread*, char const*, void*, void*, int)+0xaa
V  [libjvm.dylib+0x34a109]  checked_jni_ReleaseByteArrayElements+0x16d
C  [librocksdbjni-osx.jnilib+0x5266b]  JNIEnv_::ReleaseByteArrayElements(_jbyteArray*, signed char*, int)+0x4b
C  [librocksdbjni-osx.jnilib+0x166019]  rocksdb::JNIAbstractAssociativeMergeOperator::JNIMergeOperator::Merge(rocksdb::Slice const&, rocksdb::Slice const*, rocksdb::Slice const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >*, rocksdb::Logger*) const+0x1d9
C  [librocksdbjni-osx.jnilib+0x4ca183]  rocksdb::AssociativeMergeOperator::FullMergeV2(rocksdb::MergeOperator::MergeOperationInput const&, rocksdb::MergeOperator::MergeOperationOutput*) const+0xd3
C  [librocksdbjni-osx.jnilib+0x4b74d2]  rocksdb::MergeHelper::TimedFullMerge(rocksdb::MergeOperator const*, rocksdb::Slice const&, rocksdb::Slice const*, std::__1::vector<rocksdb::Slice, std::__1::allocator<rocksdb::Slice> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >*, rocksdb::Logger*, rocksdb::Statistics*, rocksdb::Env*, rocksdb::Slice*, bool)+0x272
C  [librocksdbjni-osx.jnilib+0x54d636]  rocksdb::Version::Get(rocksdb::ReadOptions const&, rocksdb::LookupKey const&, rocksdb::PinnableSlice*, rocksdb::Status*, rocksdb::MergeContext*, rocksdb::RangeDelAggregator*, bool*, bool*, unsigned long long*, rocksdb::ReadCallback*, bool*)+0xb76
C  [librocksdbjni-osx.jnilib+0x2acf75]  rocksdb::DBImpl::GetImpl(rocksdb::ReadOptions const&, rocksdb::ColumnFamilyHandle*, rocksdb::Slice const&, rocksdb::PinnableSlice*, bool*, rocksdb::ReadCallback*, bool*)+0x8c5
C  [librocksdbjni-osx.jnilib+0x2ac6a0]  rocksdb::DBImpl::Get(rocksdb::ReadOptions const&, rocksdb::ColumnFamilyHandle*, rocksdb::Slice const&, rocksdb::PinnableSlice*)+0x60
C  [librocksdbjni-osx.jnilib+0x1fc512]  rocksdb::DB::Get(rocksdb::ReadOptions const&, rocksdb::ColumnFamilyHandle*, rocksdb::Slice const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >*)+0x142
C  [librocksdbjni-osx.jnilib+0x1fc6ad]  rocksdb::DB::Get(rocksdb::ReadOptions const&, rocksdb::Slice const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >*)+0x6d
C  [librocksdbjni-osx.jnilib+0xb3d41]  rocksdb_get_helper(JNIEnv_*, rocksdb::DB*, rocksdb::ReadOptions const&, rocksdb::ColumnFamilyHandle*, _jbyteArray*, int, int)+0x1a1
C  [librocksdbjni-osx.jnilib+0xb4000]  Java_org_rocksdb_RocksDB_get__J_3BII+0xb0
J 1260  org.rocksdb.RocksDB.get(J[BII)[B (0 bytes) @ 0x000000011179ab06 [0x000000011179aa40+0xc6]
J 1262 C2 org.rocksdb.RocksDB.get([B)[B (13 bytes) @ 0x000000011179a3a0 [0x000000011179a360+0x40]
j  org.rocksdb.RocksDBTest$2.run()V+89
j  java.lang.Thread.run()V+11
v  ~StubRoutines::call_stub
V  [libjvm.dylib+0x2ebe37]  JavaCalls::call_helper(JavaValue*, methodHandle*, JavaCallArguments*, Thread*)+0x6a7
V  [libjvm.dylib+0x2ec5de]  JavaCalls::call_virtual(JavaValue*, KlassHandle, Symbol*, Symbol*, JavaCallArguments*, Thread*)+0x164
V  [libjvm.dylib+0x2ec78a]  JavaCalls::call_virtual(JavaValue*, Handle, KlassHandle, Symbol*, Symbol*, Thread*)+0x4a
V  [libjvm.dylib+0x357828]  thread_entry(JavaThread*, Thread*)+0x7c
V  [libjvm.dylib+0x57f813]  JavaThread::thread_main_inner()+0x9b
V  [libjvm.dylib+0x580d69]  JavaThread::run()+0x18b
V  [libjvm.dylib+0x49e4ee]  java_start(Thread*)+0xf6
C  [libsystem_pthread.dylib+0x32eb]  _pthread_body+0x7e
C  [libsystem_pthread.dylib+0x6249]  _pthread_start+0x42
C  [libsystem_pthread.dylib+0x240d]  thread_start+0xd
C  0x0000000000000000

adamretter · 2021-04-26T10:58:23Z

Related to #2282

publicocean0 added 6 commits January 15, 2018 12:18

fix std::unique_ptr<char[]>

5ea5bc0

first attempt to merge transactions api

c2e588f

include transaction_db

dde032a

fix transactions

8f1e628

associative /not asociative merge operator + automatic detaching thre…

6e73f37

…ads+ loaders/unloaders for static vars

associative /not asociative merge operator + automatic detaching thre…

6e0c351

…ads+ loaders/unloaders for static vars

publicocean0 changed the title ~~Abstract merge operator~~ Jan 29, 2018

publicocean0 changed the title ~~Abstract merge operator in Java + Thread JNI attachment management + loader management~~ Jan 29, 2018

remove debug folder in commit

093450e

maysamyabandeh self-assigned this Jan 29, 2018

fix: adding RocksdbException in the java signature

ea6c238

fix signature in AbstractNotAssociativeMergeOperator

8fe4788

fix jni_vm_unload: it is not called, changed strategy. Improved abstr…

b36b334

…act calls. Rockdb secondary threads are are not joined correctly

publicocean0 added 2 commits February 1, 2018 16:27

remove std::cout

b63a0e4

splitted pull request with java transactions from this pull request.S…

1cecf71

…implier to review

benclay reviewed Feb 7, 2018

View reviewed changes

benclay mentioned this pull request Feb 9, 2018

[RocksJava] Callback bridge for Java-based CompactionFilter #3338

Open

maysamyabandeh closed this Jul 27, 2018

adamretter reopened this Jul 29, 2018

adamretter added the java-api label Jul 29, 2018

adamretter self-assigned this Jul 29, 2018

guillaumepitel mentioned this pull request Nov 28, 2018

Support MergeOperator in Java #2282

Open

maysamyabandeh removed their assignment Sep 9, 2019

gfouquier mentioned this pull request Sep 17, 2019

Support merge operators in java #4805

Open

Fix compilation problems

4e14bc7

Start of fixing code

4cb8946

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Abstract merge operator in Java + Thread JNI attachment automatic management + loader management for static variables#3432

Abstract merge operator in Java + Thread JNI attachment automatic management + loader management for static variables#3432
publicocean0 wants to merge 14 commits into
facebook:mainfrom
publicocean0:abstractMergeOperator

publicocean0 commented Jan 29, 2018 •

edited

Loading

facebook-github-bot commented Jan 29, 2018

publicocean0 commented Jan 29, 2018 •

edited

Loading

facebook-github-bot commented Jan 29, 2018

facebook-github-bot commented Jan 30, 2018

facebook-github-bot commented Jan 30, 2018

facebook-github-bot commented Feb 1, 2018

publicocean0 commented Feb 1, 2018 •

edited

Loading

facebook-github-bot commented Feb 1, 2018

benclay left a comment

benclay Feb 7, 2018

benclay Feb 7, 2018

benclay Feb 7, 2018

benclay Feb 7, 2018

benclay Feb 7, 2018

benclay Feb 7, 2018

benclay Feb 7, 2018

benclay Feb 7, 2018

publicocean0 Mar 3, 2018 •

edited

Loading

benclay Feb 7, 2018

benclay Feb 7, 2018

maysamyabandeh commented Jul 27, 2018

adamretter commented Jul 29, 2018

maysamyabandeh commented Sep 9, 2019

adamretter commented Dec 13, 2019

adamretter commented Apr 26, 2021

Labels

5 participants

+                  }
+                }
+                //@Test

Uh oh!

Conversation

publicocean0 commented Jan 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

facebook-github-bot commented Jan 29, 2018

publicocean0 commented Jan 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

facebook-github-bot commented Jan 29, 2018

facebook-github-bot commented Jan 30, 2018

facebook-github-bot commented Jan 30, 2018

facebook-github-bot commented Feb 1, 2018

publicocean0 commented Feb 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

facebook-github-bot commented Feb 1, 2018

benclay left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

publicocean0 Mar 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maysamyabandeh commented Jul 27, 2018

adamretter commented Jul 29, 2018

maysamyabandeh commented Sep 9, 2019

adamretter commented Dec 13, 2019

adamretter commented Apr 26, 2021

Labels

5 participants

publicocean0 commented Jan 29, 2018 •

edited

Loading

publicocean0 commented Jan 29, 2018 •

edited

Loading

publicocean0 commented Feb 1, 2018 •

edited

Loading

publicocean0 Mar 3, 2018 •

edited

Loading