Hdfs as plugin - extend to java jni by alanpaxton · Pull Request #4 · riversand963/rocksdb

alanpaxton · 2021-12-07T16:33:02Z

Add configuration of JNI and Java components of RocksDB to the plugin architecture. This allows us to build the hdfs plugin to the level of running the HDFS Java tests, when the companion piece (PR to the plugin itself) is applied to the plugin. See riversand963/rocksdb-hdfs-env#1 to https://github.com/riversand963/rocksdb-hdfs-env (the plugin branch).

The following symbols are added to <PLUGIN>.mk and used by the Makefile(s) to configure the build.

<PLUGIN>_JNI_NATIVE_SOURCES = a list of C/CPP files relative to the plugin, e.g. java/rocksjni/env_hdfs.cc
<PLUGIN>_NATIVE_JAVA_CLASSES = a list of Java classes in the plugin with native code, e.g. org.rocksdb.HdfsEnv
<PLUGIN>_JAVA_TESTS = a list of Java test classes in the plugin, e.g. org.rocksdb.HdfsEnvTest

adamretter · 2021-12-22T13:29:02Z

@riversand963 can we get some review on this please?

riversand963 · 2021-12-22T18:19:44Z

@adamretter @alanpaxton do you have permission to add these commits directly to the PR facebook#9170?

riversand963 · 2021-12-23T00:02:34Z

Makefile

I believe this has been fixed on the main branch.

Summary: This PR moves HDFS support from RocksDB repo to a separate repo. The new (temporary?) repo in this PR serves as an example before we finalize the decision on where and who to host hdfs support. At this point, people can start from the example repo and fork. Java/JNI is not included yet, and needs to be done later if necessary. The goal is to include this commit in RocksDB 7.0 release. Reference: https://github.com/ajkr/dedupfs by ajkr Pull Request resolved: facebook#9170 Test Plan: Follow the instructions in https://github.com/riversand963/rocksdb-hdfs-env/blob/master/README.md. Build and run db_bench and db_stress. make check Reviewed By: ajkr Differential Revision: D33751662 Pulled By: riversand963 fbshipit-source-id: 22b4db7f31762ed417a20239f5a08dcd1696244f

Summary: This PR is one proposal to resolve facebook#9382. Looking at the code, I can't think of a reason why rdb is an internal component of RocksDB: it does not require any header files NOT in `include/rocksdb`. It's a better idea to host it somewhere else. Plus, rdb requires python2 which is not supported any more. No fixes or improvements will be made, even for potential security bugs (https://www.python.org/doc/sunset-python-2/). Pull Request resolved: facebook#9399 Test Plan: make check Reviewed By: ajkr Differential Revision: D33641965 Pulled By: riversand963 fbshipit-source-id: 2a6a74693e5de36834f355e41d6865db206af48b

Summary: This PR moves RADOS support from RocksDB repo to a separate repo. The new (temporary?) repo in this PR serves as an example before we finalize the decision on where and who to host RADOS support. At this point, people can start from the example repo and fork. The goal is to include this commit in RocksDB 7.0 release. Reference: https://github.com/ajkr/dedupfs by ajkr Pull Request resolved: facebook#9206 Test Plan: Follow instructions in https://github.com/riversand963/rocksdb-rados-env/blob/main/README.md and build test binary `env_librados_test` and run it. Also, make check Reviewed By: ajkr Differential Revision: D33751690 Pulled By: riversand963 fbshipit-source-id: 30466c62afa9e4619847a48567ed158e62835e35

Java/JNI builds to include plugin java sources plugin build flags pplugin tests

riversand963 · 2022-01-25T16:49:23Z

Sorry @alanpaxton I didn't double check before deleting my hdfs-as-plugin branch when I closed facebook#9170. However, it was my intention in the comment facebook#9170 (comment) that this PR be made against https://github.com/facebook/rocksdb, not https://github.com/riversand963/rocksdb. Could you please open another PR with the actual changes here and make the PR into https://github.com/facebook/rocksdb?

riversand963 · 2022-01-25T16:54:21Z

diff --git a/Makefile b/Makefile
index 0ec9ceaa0..6366b00b6 100644
--- a/Makefile
+++ b/Makefile
@@ -237,10 +237,20 @@ ROCKSDB_PLUGIN_MKS = $(foreach plugin, $(ROCKSDB_PLUGINS), plugin/$(plugin)/*.mk
 include $(ROCKSDB_PLUGIN_MKS)
 ROCKSDB_PLUGIN_SOURCES = $(foreach plugin, $(ROCKSDB_PLUGINS), $(foreach source, $($(plugin)_SOURCES), plugin/$(plugin)/$(source)))
 ROCKSDB_PLUGIN_HEADERS = $(foreach plugin, $(ROCKSDB_PLUGINS), $(foreach header, $($(plugin)_HEADERS), plugin/$(plugin)/$(header)))
-ROCKSDB_PLUGIN_PKGCONFIG_REQUIRES = $(foreach plugin, $(ROCKSDB_PLUGINS), $($(plugin)_PKGCONFIG_REQUIRES))
-PLATFORM_LDFLAGS += $(foreach plugin, $(ROCKSDB_PLUGINS), $($(plugin)_LDFLAGS))
+ROCKSDB_PLUGIN_PKGCONFIG_REQUIRES = $(strip $(foreach plugin, $(ROCKSDB_PLUGINS), $($(plugin)_PKGCONFIG_REQUIRES)))
 CXXFLAGS += $(foreach plugin, $(ROCKSDB_PLUGINS), $($(plugin)_CXXFLAGS))
 
+# Patch up the link flags for JNI from the plugins
+ROCKSDB_PLUGIN_LDFLAGS = $(foreach plugin, $(ROCKSDB_PLUGINS), $($(plugin)_LDFLAGS))
+PLATFORM_LDFLAGS += $(ROCKSDB_PLUGIN_LDFLAGS)
+JAVA_LDFLAGS += $(ROCKSDB_PLUGIN_LDFLAGS)
+JAVA_STATIC_LDFLAGS += $(ROCKSDB_PLUGIN_LDFLAGS)
+
+# Patch up the list of java native sources with files from the plugins
+ROCKSDB_PLUGIN_JNI_NATIVE_SOURCES = $(foreach plugin, $(ROCKSDB_PLUGINS), $(foreach source, $($(plugin)_JNI_NATIVE_SOURCES), plugin/$(plugin)/$(source)))
+ALL_JNI_NATIVE_SOURCES = $(JNI_NATIVE_SOURCES) $(ROCKSDB_PLUGIN_JNI_NATIVE_SOURCES)
+ROCKSDB_PLUGIN_JNI_CXX_INCLUDEFLAGS = $(foreach plugin, $(ROCKSDB_PLUGINS), -I./plugin/$(plugin))
+
 ifneq ($(strip $(ROCKSDB_PLUGIN_PKGCONFIG_REQUIRES)),)
 LDFLAGS := $(LDFLAGS) $(shell pkg-config --libs $(ROCKSDB_PLUGIN_PKGCONFIG_REQUIRES))
 ifneq ($(.SHELLSTATUS),0)
@@ -2261,7 +2271,7 @@ rocksdbjavastatic_javalib:
 	cd java; SHA256_CMD='$(SHA256_CMD)' $(MAKE) javalib
 	rm -f java/target/$(ROCKSDBJNILIB)
 	$(CXX) $(CXXFLAGS) -I./java/. $(JAVA_INCLUDE) -shared -fPIC \
-	  -o ./java/target/$(ROCKSDBJNILIB) $(JNI_NATIVE_SOURCES) \
+	  -o ./java/target/$(ROCKSDBJNILIB) $(ALL_JNI_NATIVE_SOURCES) \
 	  $(LIB_OBJECTS) $(COVERAGEFLAGS) \
 	  $(JAVA_COMPRESSIONS) $(JAVA_STATIC_LDFLAGS)
 	cd java/target;if [ "$(DEBUG_LEVEL)" == "0" ]; then \
@@ -2370,7 +2380,7 @@ ifeq ($(JAVA_HOME),)
 endif
 	$(AM_V_GEN)cd java; SHA256_CMD='$(SHA256_CMD)' $(MAKE) javalib;
 	$(AM_V_at)rm -f ./java/target/$(ROCKSDBJNILIB)
-	$(AM_V_at)$(CXX) $(CXXFLAGS) -I./java/. $(JAVA_INCLUDE) -shared -fPIC -o ./java/target/$(ROCKSDBJNILIB) $(JNI_NATIVE_SOURCES) $(LIB_OBJECTS) $(JAVA_LDFLAGS) $(COVERAGEFLAGS)
+	$(AM_V_at)$(CXX) $(CXXFLAGS) -I./java/. -I./java/rocksjni $(JAVA_INCLUDE) $(ROCKSDB_PLUGIN_JNI_CXX_INCLUDEFLAGS) -shared -fPIC -o ./java/target/$(ROCKSDBJNILIB) $(ALL_JNI_NATIVE_SOURCES) $(LIB_OBJECTS) $(JAVA_LDFLAGS) $(COVERAGEFLAGS)
 	$(AM_V_at)cd java; $(JAR_CMD) -cf target/$(ROCKSDB_JAR) HISTORY*.md
 	$(AM_V_at)cd java/target; $(JAR_CMD) -uf $(ROCKSDB_JAR) $(ROCKSDBJNILIB)
 	$(AM_V_at)cd java/target/classes; $(JAR_CMD) -uf ../$(ROCKSDB_JAR) org/rocksdb/*.class org/rocksdb/util/*.class
diff --git a/java/Makefile b/java/Makefile
index 9b04fc466..a307d70f7 100644
--- a/java/Makefile
+++ b/java/Makefile
@@ -273,6 +273,33 @@ JAVA_ARGS ?=
 
 JAVAC_ARGS ?=
 
+# Read plugin configuration
+PLUGIN_PATH = ../plugin
+ROCKSDB_PLUGIN_MKS = $(foreach plugin, $(ROCKSDB_PLUGINS), $(PLUGIN_PATH)/$(plugin)/*.mk)
+include $(ROCKSDB_PLUGIN_MKS)
+
+# Add paths to Java sources in plugins
+ROCKSDB_PLUGIN_JAVA_ROOTS = $(foreach plugin, $(ROCKSDB_PLUGINS), $(PLUGIN_PATH)/$(plugin)/java)
+PLUGIN_SOURCES = $(foreach root, $(ROCKSDB_PLUGIN_JAVA_ROOTS), $(foreach pkg, org/rocksdb/util org/rocksdb, $(root)/$(MAIN_SRC)/$(pkg)/*.java))
+CORE_SOURCES = $(foreach pkg, org/rocksdb/util org/rocksdb, $(MAIN_SRC)/$(pkg)/*.java)
+SOURCES = $(wildcard $(CORE_SOURCES) $(PLUGIN_SOURCES))
+PLUGIN_TEST_SOURCES = $(foreach root, $(ROCKSDB_PLUGIN_JAVA_ROOTS), $(foreach pkg, org/rocksdb/test org/rocksdb/util org/rocksdb, $(root)/$(TEST_SRC)/$(pkg)/*.java))
+CORE_TEST_SOURCES = $(foreach pkg, org/rocksdb/test org/rocksdb/util org/rocksdb, $(TEST_SRC)/$(pkg)/*.java)
+TEST_SOURCES = $(wildcard $(CORE_TEST_SOURCES) $(PLUGIN_TEST_SOURCES))
+$(info ROCKSDB_PLUGIN_MKS [$(ROCKSDB_PLUGIN_MKS)])
+$(info PLUGIN_SOURCES [$(PLUGIN_SOURCES)])
+$(info PLUGIN_SOURCES [$(PLUGIN_SOURCES)])
+$(info ROCKSDB_PLUGIN_JAVA_ROOTS [$(ROCKSDB_PLUGIN_JAVA_ROOTS)])
+$(info PLUGIN_TEST_SOURCES [$(PLUGIN_TEST_SOURCES)])
+$(info CORE_TEST_SOURCES [$(CORE_TEST_SOURCES)])
+$(info TEST_SOURCES [$(TEST_SOURCES)])
+
+# Configure the plugin tests and java classes
+ROCKSDB_PLUGIN_NATIVE_JAVA_CLASSES = $(foreach plugin, $(ROCKSDB_PLUGINS), $(foreach class, $($(plugin)_NATIVE_JAVA_CLASSES), $(class)))
+NATIVE_JAVA_CLASSES = $(NATIVE_JAVA_CLASSES) $(ROCKSDB_PLUGIN_NATIVE_JAVA_CLASSES)
+ROCKSDB_PLUGIN_JAVA_TESTS = $(foreach plugin, $(ROCKSDB_PLUGINS), $(foreach testclass, $($(plugin)_JAVA_TESTS), $(testclass)))
+ALL_JAVA_TESTS = $(JAVA_TESTS) $(ROCKSDB_PLUGIN_JAVA_TESTS)
+
 # When debugging add -Xcheck:jni to the java args
 ifneq ($(DEBUG_LEVEL),0)
 	JAVA_ARGS += -ea -Xcheck:jni
@@ -303,14 +330,10 @@ javalib: java java_test javadocs
 
 java:
 	$(AM_V_GEN)mkdir -p $(MAIN_CLASSES)
-ifeq ($(shell $(JAVAC_CMD) -version 2>&1 | grep 1.7.0 > /dev/null; printf $$?), 0)
-	$(AM_V_at)$(JAVAC_CMD) $(JAVAC_ARGS) -d $(MAIN_CLASSES)\
-		$(MAIN_SRC)/org/rocksdb/util/*.java\
-		$(MAIN_SRC)/org/rocksdb/*.java
+ifeq ($(shell java -version 2>&1 | grep 1.7.0 > /dev/null; printf $$?), 0)
+	$(AM_V_at) $(JAVAC_CMD) $(JAVAC_ARGS) -d $(MAIN_CLASSES) $(SOURCES)
 else
-	$(AM_V_at)$(JAVAC_CMD) $(JAVAC_ARGS) -h $(NATIVE_INCLUDE) -d $(MAIN_CLASSES)\
-		$(MAIN_SRC)/org/rocksdb/util/*.java\
-		$(MAIN_SRC)/org/rocksdb/*.java
+	$(AM_V_at) $(JAVAC_CMD) $(JAVAC_ARGS) -h $(NATIVE_INCLUDE) -d $(MAIN_CLASSES) $(SOURCES)
 endif
 	$(AM_V_at)@cp ../HISTORY.md ./HISTORY-CPP.md
 	$(AM_V_at)@rm -f ./HISTORY-CPP.md
@@ -415,23 +438,21 @@ resolve_test_deps: $(JAVA_JUNIT_JAR_PATH) $(JAVA_HAMCREST_JAR_PATH) $(JAVA_MOCKI
 
 java_test: java resolve_test_deps
 	$(AM_V_GEN)mkdir -p $(TEST_CLASSES)
-ifeq ($(shell $(JAVAC_CMD) -version 2>&1|grep 1.7.0 >/dev/null; printf $$?),0)
-	$(AM_V_at)$(JAVAC_CMD) $(JAVAC_ARGS) -cp $(MAIN_CLASSES):$(JAVA_TESTCLASSPATH) -d $(TEST_CLASSES)\
-		$(TEST_SRC)/org/rocksdb/test/*.java\
-		$(TEST_SRC)/org/rocksdb/util/*.java\
-		$(TEST_SRC)/org/rocksdb/*.java
-	$(AM_V_at)$(JAVAH_CMD) -cp $(MAIN_CLASSES):$(TEST_CLASSES) -d $(NATIVE_INCLUDE) -jni $(NATIVE_JAVA_TEST_CLASSES)
+ifeq ($(shell java -version 2>&1|grep 1.7.0 >/dev/null; printf $$?),0)
+	$(AM_V_at) $(JAVAC_CMD) $(JAVAC_ARGS) -cp $(MAIN_CLASSES):$(JAVA_TESTCLASSPATH) -d $(TEST_CLASSES) $(TEST_SOURCES)
+	$(AM_V_at) $(JAVAH_CMD) -cp $(MAIN_CLASSES):$(TEST_CLASSES) -d $(NATIVE_INCLUDE) -jni $(NATIVE_JAVA_TEST_CLASSES)
 else
-	$(AM_V_at)$(JAVAC_CMD) $(JAVAC_ARGS) -cp $(MAIN_CLASSES):$(JAVA_TESTCLASSPATH) -h $(NATIVE_INCLUDE) -d $(TEST_CLASSES)\
-		$(TEST_SRC)/org/rocksdb/test/*.java\
-		$(TEST_SRC)/org/rocksdb/util/*.java\
-		$(TEST_SRC)/org/rocksdb/*.java
+	$(AM_V_at) $(JAVAC_CMD) $(JAVAC_ARGS) -cp $(MAIN_CLASSES):$(JAVA_TESTCLASSPATH) -h $(NATIVE_INCLUDE) -d $(TEST_CLASSES)\
+		$(TEST_SOURCES)
 endif
 
 test: java java_test run_test
 
 run_test:
-	$(JAVA_CMD) $(JAVA_ARGS) -Djava.library.path=target -cp "$(MAIN_CLASSES):$(TEST_CLASSES):$(JAVA_TESTCLASSPATH):target/*" org.rocksdb.test.RocksJunitRunner $(JAVA_TESTS)
+	$(JAVA_CMD) $(JAVA_ARGS) -Djava.library.path=target -cp "$(MAIN_CLASSES):$(TEST_CLASSES):$(JAVA_TESTCLASSPATH):target/*" org.rocksdb.test.RocksJunitRunner $(ALL_JAVA_TESTS)
+
+run_plugin_test:
+	java $(JAVA_ARGS) -Djava.library.path=target -cp "$(MAIN_CLASSES):$(TEST_CLASSES):$(JAVA_TESTCLASSPATH):target/*" org.rocksdb.test.RocksJunitRunner $(ROCKSDB_PLUGIN_JAVA_TESTS)
 
 db_bench: java
 	$(AM_V_GEN)mkdir -p $(BENCHMARK_MAIN_CLASSES)

riversand963 · 2022-01-25T16:55:03Z

@alanpaxton since I messed up the original PR branch, the above is the actual change :)

alanpaxton · 2022-01-26T08:30:11Z

Hi @riversand963 sorry, yea I misunderstood, so just so I have it clear, I need to redo my PR standalone against the facebook/main branch as your changes all merged into that ?

riversand963 · 2022-01-26T17:49:36Z

Hi @alanpaxton, yes, that's what I proposed, and thanks for the help!

alanpaxton mentioned this pull request Dec 7, 2021

Extend HDFS plugin to support Java/JNI riversand963/rocksdb-hdfs-env#1

Merged

riversand963 reviewed Dec 23, 2021

View reviewed changes

Makefile Outdated

Copy link

Owner

riversand963 Dec 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this has been fixed on the main branch.

riversand963 force-pushed the hdfs-as-plugin branch 2 times, most recently from 404f988 to b1a4618 Compare January 5, 2022 00:55

riversand963 force-pushed the hdfs-as-plugin branch from b1a4618 to 3dde51c Compare January 25, 2022 01:31

riversand963 added 2 commits January 24, 2022 20:23

riversand963 mentioned this pull request Jan 25, 2022

[RFC] Move HDFS support to separate repo facebook/rocksdb#9170

Closed

riversand963 and others added 4 commits January 24, 2022 22:50

Make Java and JNI respect plugins

d7c9bed

Java/JNI builds to include plugin java sources plugin build flags pplugin tests

Fix empty requires from plugins

602da97

Remove debugging

d95c80c

alanpaxton force-pushed the hdfs-as-plugin-java-jni branch from 3cd8527 to d95c80c Compare January 25, 2022 08:43

riversand963 deleted the branch riversand963:hdfs-as-plugin January 25, 2022 15:45

riversand963 closed this Jan 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hdfs as plugin - extend to java jni#4

Hdfs as plugin - extend to java jni#4
alanpaxton wants to merge 6 commits intoriversand963:hdfs-as-pluginfrom
alanpaxton:hdfs-as-plugin-java-jni

alanpaxton commented Dec 7, 2021 •

edited

Loading

Uh oh!

adamretter commented Dec 22, 2021

Uh oh!

riversand963 commented Dec 22, 2021

Uh oh!

riversand963 Dec 23, 2021

Uh oh!

riversand963 commented Jan 25, 2022

Uh oh!

riversand963 commented Jan 25, 2022

Uh oh!

riversand963 commented Jan 25, 2022

Uh oh!

alanpaxton commented Jan 26, 2022

Uh oh!

riversand963 commented Jan 26, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

alanpaxton commented Dec 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adamretter commented Dec 22, 2021

Uh oh!

riversand963 commented Dec 22, 2021

Uh oh!

riversand963 Dec 23, 2021

Choose a reason for hiding this comment

Uh oh!

riversand963 commented Jan 25, 2022

Uh oh!

riversand963 commented Jan 25, 2022

Uh oh!

riversand963 commented Jan 25, 2022

Uh oh!

alanpaxton commented Jan 26, 2022

Uh oh!

riversand963 commented Jan 26, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alanpaxton commented Dec 7, 2021 •

edited

Loading