From ffdf6eee19da1a492ad44e16eb329072837bd5c5 Mon Sep 17 00:00:00 2001 From: Adam Retter Date: Mon, 22 Aug 2016 19:01:42 +0100 Subject: [PATCH] Add Status to RocksDBException so that meaningful function result Status from the C++ API isn't lost (#1273) --- include/rocksdb/status.h | 2 + java/Makefile | 4 +- java/rocksjni/portal.h | 150 ++++++++++++++++-- java/rocksjni/rocksdb_exception_test.cc | 78 +++++++++ .../java/org/rocksdb/RocksDBException.java | 23 +++ java/src/main/java/org/rocksdb/Status.java | 113 +++++++++++++ .../org/rocksdb/RocksDBExceptionTest.java | 115 ++++++++++++++ src.mk | 1 + 8 files changed, 473 insertions(+), 13 deletions(-) create mode 100644 java/rocksjni/rocksdb_exception_test.cc create mode 100644 java/src/main/java/org/rocksdb/Status.java create mode 100644 java/src/test/java/org/rocksdb/RocksDBExceptionTest.java diff --git a/include/rocksdb/status.h b/include/rocksdb/status.h index bff15ee0f..03fd8895f 100644 --- a/include/rocksdb/status.h +++ b/include/rocksdb/status.h @@ -73,6 +73,8 @@ class Status { SubCode subcode() const { return subcode_; } + const char* getState() const { return state_; } + // Return a success status. static Status OK() { return Status(); } diff --git a/java/Makefile b/java/Makefile index 8dba3a851..a6afc99d7 100644 --- a/java/Makefile +++ b/java/Makefile @@ -44,7 +44,8 @@ NATIVE_JAVA_CLASSES = org.rocksdb.AbstractCompactionFilter\ org.rocksdb.WriteBatchWithIndex\ org.rocksdb.WBWIRocksIterator -NATIVE_JAVA_TEST_CLASSES = org.rocksdb.WriteBatchTest\ +NATIVE_JAVA_TEST_CLASSES = org.rocksdb.RocksDBExceptionTest\ + org.rocksdb.WriteBatchTest\ org.rocksdb.WriteBatchTestInternalHelper ROCKSDB_MAJOR = $(shell egrep "ROCKSDB_MAJOR.[0-9]" ../include/rocksdb/version.h | cut -d ' ' -f 3) @@ -87,6 +88,7 @@ JAVA_TESTS = org.rocksdb.BackupableDBOptionsTest\ org.rocksdb.ReadOnlyTest\ org.rocksdb.ReadOptionsTest\ org.rocksdb.RocksDBTest\ + org.rocksdb.RocksDBExceptionTest\ org.rocksdb.RocksEnvTest\ org.rocksdb.RocksIteratorTest\ org.rocksdb.RocksMemEnvTest\ diff --git a/java/rocksjni/portal.h b/java/rocksjni/portal.h index 4d7e50217..344a8f405 100644 --- a/java/rocksjni/portal.h +++ b/java/rocksjni/portal.h @@ -81,23 +81,15 @@ template class RocksDBJavaException { return jclazz; } - // Create and throw a java exception by converting the input - // Status. - // - // In case s.ok() is true, then this function will not throw any - // exception. - static void ThrowNew(JNIEnv* env, Status s) { - if (s.ok()) { - return; - } - jstring msg = env->NewStringUTF(s.ToString().c_str()); + // Create and throw a java exception with the provided message + static void ThrowNew(JNIEnv* env, const std::string& msg) { + jstring jmsg = env->NewStringUTF(msg.c_str()); // get the constructor id of org.rocksdb.RocksDBException static jmethodID mid = env->GetMethodID( DERIVED::getJClass(env), "", "(Ljava/lang/String;)V"); assert(mid != nullptr); - env->Throw((jthrowable)env->NewObject(DERIVED::getJClass(env), - mid, msg)); + env->Throw((jthrowable)env->NewObject(DERIVED::getJClass(env), mid, jmsg)); } }; @@ -110,6 +102,87 @@ class RocksDBJni : public RocksDBNativeClass { } }; +// The portal class for org.rocksdb.Status +class StatusJni : public RocksDBNativeClass { + public: + // Get the java class id of org.rocksdb.Status. + static jclass getJClass(JNIEnv* env) { + return RocksDBNativeClass::getJClass(env, "org/rocksdb/Status"); + } + + // Create a new org.rocksdb.Status with the same properties as the + // provided C++ rocksdb::Status object + static jobject construct(JNIEnv* env, const Status& status) { + static jmethodID mid = + env->GetMethodID(getJClass(env), "", "(BBLjava/lang/String;)V"); + assert(mid != nullptr); + + jstring jstate = nullptr; + if (status.getState() != nullptr) { + std::string s(status.getState()); + jstate = env->NewStringUTF(s.c_str()); + } + return env->NewObject(getJClass(env), mid, toJavaStatusCode(status.code()), + toJavaStatusSubCode(status.subcode()), jstate); + } + + // Returns the equivalent org.rocksdb.Status.Code for the provided + // C++ rocksdb::Status::Code enum + static jbyte toJavaStatusCode(const rocksdb::Status::Code& code) { + switch (code) { + case rocksdb::Status::Code::kOk: + return 0x0; + case rocksdb::Status::Code::kNotFound: + return 0x1; + case rocksdb::Status::Code::kCorruption: + return 0x2; + case rocksdb::Status::Code::kNotSupported: + return 0x3; + case rocksdb::Status::Code::kInvalidArgument: + return 0x4; + case rocksdb::Status::Code::kIOError: + return 0x5; + case rocksdb::Status::Code::kMergeInProgress: + return 0x6; + case rocksdb::Status::Code::kIncomplete: + return 0x7; + case rocksdb::Status::Code::kShutdownInProgress: + return 0x8; + case rocksdb::Status::Code::kTimedOut: + return 0x9; + case rocksdb::Status::Code::kAborted: + return 0xA; + case rocksdb::Status::Code::kBusy: + return 0xB; + case rocksdb::Status::Code::kExpired: + return 0xC; + case rocksdb::Status::Code::kTryAgain: + return 0xD; + default: + return 0xFF; // undefined + } + } + + // Returns the equivalent org.rocksdb.Status.SubCode for the provided + // C++ rocksdb::Status::SubCode enum + static jbyte toJavaStatusSubCode(const rocksdb::Status::SubCode& subCode) { + switch (subCode) { + case rocksdb::Status::SubCode::kNone: + return 0x0; + case rocksdb::Status::SubCode::kMutexTimeout: + return 0x1; + case rocksdb::Status::SubCode::kLockTimeout: + return 0x2; + case rocksdb::Status::SubCode::kLockLimit: + return 0x3; + case rocksdb::Status::SubCode::kMaxSubCode: + return 0xFE; + default: + return 0xFF; // undefined + } + } +}; + // The portal class for org.rocksdb.RocksDBException class RocksDBExceptionJni : public RocksDBJavaException { @@ -119,6 +192,41 @@ class RocksDBExceptionJni : return RocksDBJavaException::getJClass(env, "org/rocksdb/RocksDBException"); } + + static void ThrowNew(JNIEnv* env, const std::string& msg) { + RocksDBJavaException::ThrowNew(env, msg); + } + + static void ThrowNew(JNIEnv* env, const Status& s) { + if (s.ok()) { + return; + } + + jobject jstatus = StatusJni::construct(env, s); + + // get the constructor id of org.rocksdb.RocksDBException + static jmethodID mid = + env->GetMethodID(getJClass(env), "", "(Lorg/rocksdb/Status;)V"); + assert(mid != nullptr); + + env->Throw((jthrowable)env->NewObject(getJClass(env), mid, jstatus)); + } + + static void ThrowNew(JNIEnv* env, const std::string& msg, const Status& s) { + if (s.ok()) { + return; + } + + jstring jmsg = env->NewStringUTF(msg.c_str()); + jobject jstatus = StatusJni::construct(env, s); + + // get the constructor id of org.rocksdb.RocksDBException + static jmethodID mid = env->GetMethodID( + getJClass(env), "", "(Ljava/lang/String;Lorg/rocksdb/Status;)V"); + assert(mid != nullptr); + + env->Throw((jthrowable)env->NewObject(getJClass(env), mid, jmsg, jstatus)); + } }; // The portal class for java.lang.IllegalArgumentException @@ -130,6 +238,24 @@ class IllegalArgumentExceptionJni : return RocksDBJavaException::getJClass(env, "java/lang/IllegalArgumentException"); } + + // Create and throw a IllegalArgumentException by converting the input + // Status. + // + // In case s.ok() is true, then this function will not throw any + // exception. + static void ThrowNew(JNIEnv* env, const Status& s) { + if (s.ok()) { + return; + } + jstring msg = env->NewStringUTF(s.ToString().c_str()); + // get the constructor id of org.rocksdb.RocksDBException + static jmethodID mid = + env->GetMethodID(getJClass(env), "", "(Ljava/lang/String;)V"); + assert(mid != nullptr); + + env->Throw((jthrowable)env->NewObject(getJClass(env), mid, msg)); + } }; diff --git a/java/rocksjni/rocksdb_exception_test.cc b/java/rocksjni/rocksdb_exception_test.cc new file mode 100644 index 000000000..87900d3c1 --- /dev/null +++ b/java/rocksjni/rocksdb_exception_test.cc @@ -0,0 +1,78 @@ +// Copyright (c) 2011-present, Facebook, Inc. All rights reserved. +// This source code is licensed under the BSD-style license found in the +// LICENSE file in the root directory of this source tree. An additional grant +// of patent rights can be found in the PATENTS file in the same directory. + +#include + +#include "include/org_rocksdb_RocksDBExceptionTest.h" + +#include "rocksdb/slice.h" +#include "rocksdb/status.h" +#include "rocksjni/portal.h" + +/* + * Class: org_rocksdb_RocksDBExceptionTest + * Method: raiseException + * Signature: ()V + */ +void Java_org_rocksdb_RocksDBExceptionTest_raiseException(JNIEnv* env, + jobject jobj) { + rocksdb::RocksDBExceptionJni::ThrowNew(env, std::string("test message")); +} + +/* + * Class: org_rocksdb_RocksDBExceptionTest + * Method: raiseExceptionWithStatusCode + * Signature: ()V + */ +void Java_org_rocksdb_RocksDBExceptionTest_raiseExceptionWithStatusCode( + JNIEnv* env, jobject jobj) { + rocksdb::RocksDBExceptionJni::ThrowNew(env, "test message", + rocksdb::Status::NotSupported()); +} + +/* + * Class: org_rocksdb_RocksDBExceptionTest + * Method: raiseExceptionNoMsgWithStatusCode + * Signature: ()V + */ +void Java_org_rocksdb_RocksDBExceptionTest_raiseExceptionNoMsgWithStatusCode( + JNIEnv* env, jobject jobj) { + rocksdb::RocksDBExceptionJni::ThrowNew(env, rocksdb::Status::NotSupported()); +} + +/* + * Class: org_rocksdb_RocksDBExceptionTest + * Method: raiseExceptionWithStatusCodeSubCode + * Signature: ()V + */ +void Java_org_rocksdb_RocksDBExceptionTest_raiseExceptionWithStatusCodeSubCode( + JNIEnv* env, jobject jobj) { + rocksdb::RocksDBExceptionJni::ThrowNew( + env, "test message", + rocksdb::Status::TimedOut(rocksdb::Status::SubCode::kLockTimeout)); +} + +/* + * Class: org_rocksdb_RocksDBExceptionTest + * Method: raiseExceptionNoMsgWithStatusCodeSubCode + * Signature: ()V + */ +void Java_org_rocksdb_RocksDBExceptionTest_raiseExceptionNoMsgWithStatusCodeSubCode( + JNIEnv* env, jobject jobj) { + rocksdb::RocksDBExceptionJni::ThrowNew( + env, rocksdb::Status::TimedOut(rocksdb::Status::SubCode::kLockTimeout)); +} + +/* + * Class: org_rocksdb_RocksDBExceptionTest + * Method: raiseExceptionWithStatusCodeState + * Signature: ()V + */ +void Java_org_rocksdb_RocksDBExceptionTest_raiseExceptionWithStatusCodeState( + JNIEnv* env, jobject jobj) { + rocksdb::Slice state("test state"); + rocksdb::RocksDBExceptionJni::ThrowNew(env, "test message", + rocksdb::Status::NotSupported(state)); +} diff --git a/java/src/main/java/org/rocksdb/RocksDBException.java b/java/src/main/java/org/rocksdb/RocksDBException.java index ee869f20f..25aadad8f 100644 --- a/java/src/main/java/org/rocksdb/RocksDBException.java +++ b/java/src/main/java/org/rocksdb/RocksDBException.java @@ -10,12 +10,35 @@ package org.rocksdb; * type is used to describe an internal error from the c++ rocksdb library. */ public class RocksDBException extends Exception { + + /* @Nullable */ private final Status status; + /** * The private construct used by a set of public static factory method. * * @param msg the specified error message. */ public RocksDBException(final String msg) { + this(msg, null); + } + + public RocksDBException(final String msg, final Status status) { super(msg); + this.status = status; + } + + public RocksDBException(final Status status) { + super(status.getState() != null ? status.getState() + : status.getCodeString()); + this.status = status; + } + + /** + * Get the status returned from RocksDB + * + * @return The status reported by RocksDB, or null if no status is available + */ + public Status getStatus() { + return status; } } diff --git a/java/src/main/java/org/rocksdb/Status.java b/java/src/main/java/org/rocksdb/Status.java new file mode 100644 index 000000000..79d2e1321 --- /dev/null +++ b/java/src/main/java/org/rocksdb/Status.java @@ -0,0 +1,113 @@ +// Copyright (c) 2011-present, Facebook, Inc. All rights reserved. +// This source code is licensed under the BSD-style license found in the +// LICENSE file in the root directory of this source tree. An additional grant +// of patent rights can be found in the PATENTS file in the same directory. + +package org.rocksdb; + +/** + * Represents the status returned by a function call in RocksDB. + * + * Currently only used with {@link RocksDBException} when the + * status is not {@link Code#Ok} + */ +public class Status { + private final Code code; + /* @Nullable */ private final SubCode subCode; + /* @Nullable */ private final String state; + + public Status(final Code code, final SubCode subCode, final String state) { + this.code = code; + this.subCode = subCode; + this.state = state; + } + + /** + * Intentionally private as this will be called from JNI + */ + private Status(final byte code, final byte subCode, final String state) { + this.code = Code.getCode(code); + this.subCode = SubCode.getSubCode(subCode); + this.state = state; + } + + public Code getCode() { + return code; + } + + public SubCode getSubCode() { + return subCode; + } + + public String getState() { + return state; + } + + public String getCodeString() { + final StringBuilder builder = new StringBuilder() + .append(code.name()); + if(subCode != null && subCode != SubCode.None) { + builder.append("(") + .append(subCode.name()) + .append(")"); + } + return builder.toString(); + } + + public enum Code { + Ok( (byte)0x0), + NotFound( (byte)0x1), + Corruption( (byte)0x2), + NotSupported( (byte)0x3), + InvalidArgument( (byte)0x4), + IOError( (byte)0x5), + MergeInProgress( (byte)0x6), + Incomplete( (byte)0x7), + ShutdownInProgress( (byte)0x8), + TimedOut( (byte)0x9), + Aborted( (byte)0xA), + Busy( (byte)0xB), + Expired( (byte)0xC), + TryAgain( (byte)0xD); + + private final byte value; + + Code(final byte value) { + this.value = value; + } + + public static Code getCode(final byte value) { + for (final Code code : Code.values()) { + if (code.value == value){ + return code; + } + } + throw new IllegalArgumentException( + "Illegal value provided for Code."); + } + } + + public enum SubCode { + None( (byte)0x0), + MutexTimeout( (byte)0x1), + LockTimeout( (byte)0x2), + LockLimit( (byte)0x3), + MaxSubCode( (byte)0xFE); + + private final byte value; + + SubCode(final byte value) { + this.value = value; + } + + public static SubCode getSubCode(final byte value) { + for (final SubCode subCode : SubCode.values()) { + if (subCode.value == value){ + return subCode; + } + } + throw new IllegalArgumentException( + "Illegal value provided for SubCode."); + } + } +} diff --git a/java/src/test/java/org/rocksdb/RocksDBExceptionTest.java b/java/src/test/java/org/rocksdb/RocksDBExceptionTest.java new file mode 100644 index 000000000..162b62e55 --- /dev/null +++ b/java/src/test/java/org/rocksdb/RocksDBExceptionTest.java @@ -0,0 +1,115 @@ +// Copyright (c) 2011-present, Facebook, Inc. All rights reserved. +// This source code is licensed under the BSD-style license found in the +// LICENSE file in the root directory of this source tree. An additional grant +// of patent rights can be found in the PATENTS file in the same directory. + +package org.rocksdb; + +import org.junit.Test; + +import org.rocksdb.Status.Code; +import org.rocksdb.Status.SubCode; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.fail; + +public class RocksDBExceptionTest { + + @Test + public void exception() { + try { + raiseException(); + } catch(final RocksDBException e) { + assertThat(e.getStatus()).isNull(); + assertThat(e.getMessage()).isEqualTo("test message"); + return; + } + fail(); + } + + @Test + public void exceptionWithStatusCode() { + try { + raiseExceptionWithStatusCode(); + } catch(final RocksDBException e) { + assertThat(e.getStatus()).isNotNull(); + assertThat(e.getStatus().getCode()).isEqualTo(Code.NotSupported); + assertThat(e.getStatus().getSubCode()).isEqualTo(SubCode.None); + assertThat(e.getStatus().getState()).isNull(); + assertThat(e.getMessage()).isEqualTo("test message"); + return; + } + fail(); + } + + @Test + public void exceptionNoMsgWithStatusCode() { + try { + raiseExceptionNoMsgWithStatusCode(); + } catch(final RocksDBException e) { + assertThat(e.getStatus()).isNotNull(); + assertThat(e.getStatus().getCode()).isEqualTo(Code.NotSupported); + assertThat(e.getStatus().getSubCode()).isEqualTo(SubCode.None); + assertThat(e.getStatus().getState()).isNull(); + assertThat(e.getMessage()).isEqualTo(Code.NotSupported.name()); + return; + } + fail(); + } + + @Test + public void exceptionWithStatusCodeSubCode() { + try { + raiseExceptionWithStatusCodeSubCode(); + } catch(final RocksDBException e) { + assertThat(e.getStatus()).isNotNull(); + assertThat(e.getStatus().getCode()).isEqualTo(Code.TimedOut); + assertThat(e.getStatus().getSubCode()) + .isEqualTo(Status.SubCode.LockTimeout); + assertThat(e.getStatus().getState()).isNull(); + assertThat(e.getMessage()).isEqualTo("test message"); + return; + } + fail(); + } + + @Test + public void exceptionNoMsgWithStatusCodeSubCode() { + try { + raiseExceptionNoMsgWithStatusCodeSubCode(); + } catch(final RocksDBException e) { + assertThat(e.getStatus()).isNotNull(); + assertThat(e.getStatus().getCode()).isEqualTo(Code.TimedOut); + assertThat(e.getStatus().getSubCode()).isEqualTo(SubCode.LockTimeout); + assertThat(e.getStatus().getState()).isNull(); + assertThat(e.getMessage()).isEqualTo(Code.TimedOut.name() + + "(" + SubCode.LockTimeout.name() + ")"); + return; + } + fail(); + } + + @Test + public void exceptionWithStatusCodeState() { + try { + raiseExceptionWithStatusCodeState(); + } catch(final RocksDBException e) { + assertThat(e.getStatus()).isNotNull(); + assertThat(e.getStatus().getCode()).isEqualTo(Code.NotSupported); + assertThat(e.getStatus().getSubCode()).isEqualTo(SubCode.None); + assertThat(e.getStatus().getState()).isNotNull(); + assertThat(e.getMessage()).isEqualTo("test message"); + return; + } + fail(); + } + + private native void raiseException() throws RocksDBException; + private native void raiseExceptionWithStatusCode() throws RocksDBException; + private native void raiseExceptionNoMsgWithStatusCode() throws RocksDBException; + private native void raiseExceptionWithStatusCodeSubCode() + throws RocksDBException; + private native void raiseExceptionNoMsgWithStatusCodeSubCode() + throws RocksDBException; + private native void raiseExceptionWithStatusCodeState() + throws RocksDBException; +} diff --git a/src.mk b/src.mk index b8a9e3132..4d7a4cf7d 100644 --- a/src.mk +++ b/src.mk @@ -339,6 +339,7 @@ JNI_NATIVE_SOURCES = \ java/rocksjni/remove_emptyvalue_compactionfilterjni.cc \ java/rocksjni/restorejni.cc \ java/rocksjni/rocksjni.cc \ + java/rocksjni/rocksdb_exception_test.cc \ java/rocksjni/slice.cc \ java/rocksjni/snapshot.cc \ java/rocksjni/statistics.cc \