ConsenSysMesh / cava

ConsenSys core libraries for Java & Kotlin
Apache License 2.0
84 stars 34 forks source link

Add RocksDB support to KVs #134

Closed atoulme closed 5 years ago

atoulme commented 5 years ago

Also have KVs implement their own CoroutineScope, similar to DiscoveryService, so we don't use GlobalScope in async methods.

cleishm commented 5 years ago

So I purposefully didn’t have KV implement the CoroutineScope, as the cancellation scope for those async methods is outside the lifetime of the service.

cleishm commented 5 years ago

But agree that using GlobalScope is wrong. It should ideally be the scope of the caller.

atoulme commented 5 years ago

Hard to get a context in Java though?

atoulme commented 5 years ago

OK, I have scaled back the changes back to just adding RocksDB. Should be good to go.

cleishm commented 5 years ago

Hard to get a context in Java though?

Right. Hence I went with global scope. If you want cancellation scoping, you should be in Kotlin using the suspend methods anyway.