Added missing options to RocksJava #2039
Conversation
|
@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
There was a problem hiding this comment.
This diff is too big to review properly. (It would be great if this could be split into multiple smaller pieces). I just scrolled though most of the changes and they look fine to me, and I am ok with approving and merging.
That said, I'll leave the final action to @siying here.
| * Signature: (JI)V | ||
| */ | ||
| void Java_org_rocksdb_BackupableDBOptions_setMaxBackgroundOperations( | ||
| JNIEnv* env, jobject jobj, jlong jhandle, jint max_background_operations) { |
There was a problem hiding this comment.
Nit: Should this be jmax_background_operation, to conform to the other param names in the file?
| private native void setMaxBackgroundOperations(final long handle, | ||
| final int maxBackgroundOperations); | ||
| private native int maxBackgroundOperations(final long handle); | ||
| private native void setCallbackTriggerIntervalSize(final long handle, |
There was a problem hiding this comment.
Curious ... Why are some params final whereas others are non-final (in this whole file, and in a few other places)? Is it just that all the newer additions will be final going forward?
There was a problem hiding this comment.
Indeed, params will be final moving forward
| import java.util.Properties; | ||
| import java.util.Random; | ||
| import java.nio.file.Paths; | ||
| import java.util.*; |
There was a problem hiding this comment.
Better to spell out individual classes instead of a * (unless there are too many, of course).
There was a problem hiding this comment.
Yup. However, IntelliJ automatically rewrites those to a .* for you once you get past a few.
There was a problem hiding this comment.
Ah, IntelliJ! I miss some of its features in my current setup. :)
|
@sagar0 I was careful to break the PR into distinct commits to try and make it easier to review. |
This adds almost all missing options to RocksJava