[Feature request] Exposing StderrLogger via RocksJava (with POC)

73 views
Skip to first unread message

Neil Ramaswamy

unread,
Dec 8, 2023, 6:37:14 PM12/8/23
to rocksdb
Hi folks,

We'd like to write logs from RocksDB running within our Java application to stderr.

However, the only functionality there exists in RocksJava for setting the logger is via setLogger, which invokes the supplied logger over the JNI. Unfortunately, we cannot log this way due to the performance overheads of native threads attaching/detaching to the JVM (especially with the JDK 17 performance regression in these JNI functions).

We're more than happy to implement this feature ourselves and submit a patch; we have a basic Proof-of-Concept implementing this behavior here (we also include setting a `logPrefix` so that we know which RocksDB logs came from which database; currently, logged messages don't have this information).

We'd like to know if there's a path forward here. We will likely need to discuss:

1. Whether it's acceptable to expose just one existing logger (i.e. just StderrLogger) as opposed to all of them
2. Whether adding a `logPrefix` is acceptable, either to the base Logger implementation or just the StderrLogger

Again, we're happy to spend the time implementing this once we settle on the right design parameters.

Thanks!
Neil

Adam Retter

unread,
Dec 11, 2023, 7:13:31 AM12/11/23
to rocksdb
Hi Neil,

Thanks for your message. I think we would be happy to see one or more Pull Requests to enable this.

I am not quite sure about the API that you have suggested in your other PR though, it seems to me that having both setLogger and setStdErrLogger is a bit confusing. I would suggest instead that we keep just the setLogger API, we can then have a Java Object (possibly a singleton) that represents the StderrLogger. This also makes it easier to add the other Loggers (perhaps now or perhaps in future PRs) that you mentioned.
We also need to think about Ownership, if there is an existing logger, do we (Java) own it, if so we need to close it before we replace it with another object? otherwise we may leak some memory!

Happy to discuss further...

Kind regards. Adam.

Neil Ramaswamy

unread,
Dec 11, 2023, 2:50:38 PM12/11/23
to rocksdb
Hi Adam,

Thanks for the reply! Yeah, the code I posted was just a POC—I would hope to never ship it :)

Working this into setLogger seems a little tricky—how would (native) setLogger know that the logger handle it was given corresponded to a user-defined logger (to invoke over the JNI), or a singleton (that configures the native logger)? Just from the jlong it is given, I don't see a simple way to do this.

An option that makes more sense to me is to have a method setNativeLogger, which takes an enum that specifies an existing, native logger implementation to use. It seems that logger configuration is anyway done through the passed Options class (as described here), so an enum might be all that needs to be passed to setNativeLogger.

With respect to Ownership: I suppose if the user had a custom logger, set it via setLogger, and then set their logger to a native logger, the custom logger would be leaked. Is that the scenario you're referring to? If so, is that something we need to actively protect against, or do we assume that they will close their resources before doing away with them?

Best,
Neil

Neil Ramaswamy

unread,
Dec 14, 2023, 6:40:25 PM12/14/23
to rocksdb
Hi Adam,

I've implemented a POC of the `setNativeLogger` suggestion I had earlier. Totally fine if we don't move forward with this, but if it looks viable, I'll open the PR and we can continue discussion there.

Thanks!
Neil 

Reply all
Reply to author
Forward
0 new messages