-
Notifications
You must be signed in to change notification settings - Fork 6.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support MergeOperator in Java #2282
Comments
@sagar0 So I have a work in progress for this. However, trying to figure out an efficient API is very hard as their is substantial cost for transferring the |
I do understand that this is a hard one and I +1 the idea of publishing an experimental API and seeking community feedback, if any. |
is there any progress on this issue? Maybe something like map.put(key,oldValue,newValue):boolean could be implemented for atomic updates, I know it would not be as efficient but if there is another way to do atomic updates for RockDb in java without using counters i'm all ears. |
@adamretter Also interested if there is any ETA for this? I also noticed that our workaround for some merge performance problems mentioned in #1988 (setting |
+1 on this issue. @adamretter please provide some feedback on the current status. I would love to test and/or help out if possible. |
@adamretter do you have any news? |
@GalRogozinski My plan is to implement this in the later part of November and December. |
That would be a very lovely feature to have. For now, we are avoiding Gets for updates by using key suffix + a background sweep on the base which does the merge + put + delete. That's nasty, but still has significantly better performance than pure get/put. The PR here #3432 seems to have proposed some interesting ideas, and it feels like apart from requiring cleanup & straightening the code, it was doing the job. |
We've started working with @gfouquier on a Java-side MergeOperator based on PR #3432 by @publicocean0 however there seems to be quite a few redunduncy between the init.[cc|h] and the portal.[cc|h] and a lot of thing are not working anymore probably due to evolutions in the master. I'm not sure why @GalRogozinski unassigned @adamretter from this issue, since he seems by far the most experienced in this matter. @adamretter, can you confirm you're working on it ? We'd love to participate if you have a banch we could test. |
It was a github bug. I just mentioned @adamretter and it unassigned him. I am not even supposed to have permissions here. I just mentioned him again because maybe it will assign him back |
Is the ask to have MergeOperators written in Java or to have custom MergeOperators written in C++ available to Java? I am working on something that I hope opens up more of the extensions written in C++ available to Java (the latter of the two). |
I may speak only for me, but I understand that this issue is about the first case. And by the way, we have a working version here, based on #3432 from @publicocean0 - it's not yet ready for a proper PR though, and we're first testing it heavily. |
@adamretter you seem to worry about JNI performance, so I think I could share some stuff I'm working on.
|
The ultimate aim of this issue is to allow RocksJava API users to allow accessing custom merge operators without modifying RocksDB library code. The merge operator code should lie in the application's code repository.
|
@sagar0: To add a MergeOperator written in C++ from a custom shared library to the rocks code, you would do something like: I plan to push something out in the next few weeks that does exactly this for Cassandra. To implement a Java-based MergeOperator, I would assume someone would write a C++ one that delegates back to the Java layer. Then the "SetMergeOperator" would use the name of the C++ one and pass in properties for the Java class. If there is interest on this thread, I will add a comment here when I generate the PR for Cassandra using this model. |
On Mon, Dec 17, 2018 at 7:36 PM mrambacher ***@***.***> wrote:
To implement a Java-based MergeOperator, I would assume someone would
write a C++ one that delegates back to the Java layer. Then the
"SetMergeOperator" would use the name of the C++ one and pass in properties
for the Java class.
|
I submitted a pull request to support merge operators in java here :
#4805
On Mon, Dec 17, 2018 at 9:01 PM Geoffroy Fouquier <g.fouquier@gmail.com>
wrote:
…
On Mon, Dec 17, 2018 at 7:36 PM mrambacher ***@***.***>
wrote:
> To implement a Java-based MergeOperator, I would assume someone would
> write a C++ one that delegates back to the Java layer. Then the
> "SetMergeOperator" would use the name of the C++ one and pass in properties
> for the Java class.
>
we are currently working on this part, based on #3432
<#3432>but more integrated into
rocksdb. Our current approach is to modify the java MergeOperator class
which inherit from RocksCallbackObject and allows to instantiate
JniCallback onthe c++ side.
|
Hello, is there a reason why the pull request by @gfouquier hasn't been merged ? It's a pretty big deal for us to have this functionality upstream. We use it in production in a modified Rocksdb and we think this would enable lots of people to use merge functionnality of rocksdb in java |
Hi @adamretter , I would also want this PR merged. Is there any plan for it? Also, I see there is options.setMergeOperator() available in rocks java, isn't its role to use merge operator in java? |
There are a couple different things going on in this thread. Are you asking for access to more of the C++ MergeOperators in Java or asking for the ability to write MergeOperators in Java? I have been working on making the former easier to do but not the ability to write Java MergeOperators. |
Oh, so you mean the ability to write MergeOperators in Java is already available by options.setMergeOperator() and this PR is meant to access more of the C++ MergeOperators in Java? |
No I think this is the opposite, the ability to use C++ merge operators is already available and this PR is to ba able to write java merge operators. We use them heavily and rely currently on a patched version of RocksDB. We also we really like for our PR to be merged. AFAIK, there has been no news about any plan since my colleague @gfouquier has submitted the last version. |
Thanks @guillaumepitel for information. Can you clarify what StringAppendOperator.java does then? Looks like that is exposed in java, which can be set using options.setMergeOperator(). I just started exploring merge operator and looks like it would be helpful for our use-case where we have a read-modify-write (key:[v1,v2,v3]) and we want to convert it into an append using merge operator, so all values v1, v2 and v3 can be merged during key reads. |
@brary this is precisely how C MergeOperators are currently supported from the Java : StringAppendOperator is just a wrapper around a native JNI call that encapsulate a Java Strings concat operation written in C/JNI. That's probably what @mrambacher is trying to improve. Look at the code of StringAppendOperator, it calls a native method to create a new instance. |
@guillaumepitel @brary Right. I am trying to make any and all MergeOperators (and other "plugin classes") accessible to Java and C++ via simple APIs. See #6533 for more information. |
We found that performance was quite bad with out initial implementation, we are exploring new routes to working around the performance issues and hope to send a PR in the next month or two. |
Hi there! Any work in this area lately? We're also really interested in this, but at the same time, at the moment, we don't really need a full-blown API exposing all the features of merge operators in Java, but rather want to overcome a single limitation of |
I am working on making MergeOperators into Customizable objects, meaning I can configure and load them more generically and through options file. I have considered changing the StringAppend operator to use a string (instead of char) separator when I complete this work. Having an empty string would allow for the behavior you are looking for. From there it would become a matter of getting that interface/method exposed to Java... |
@mrambacher This level of configuration definitely sounds really cool, but, on the other hand, if it's still WIP, it seems not that difficult to add an extra |
@east825 We have a similar use case; were you ever able to move forward with this? |
MergeOperator
functionality is not currently available in the Java API.StringAppendOperator
is the only implementation that is exposed as part of Java API today. There has been considerable interest from the community recently to expose full functionality of MergeOperator so that they can take advantage of it in Java as well (Ex: #1988 , #2289)cc: @adamretter
The text was updated successfully, but these errors were encountered: