In our investigation of https://github.com/spring-projects/spring-boot/issues/14685 we have concluded that there's no API that we can use to determine Couchbase's health in a timely manner. CouchbaseReactiveHealthIndicator was added earlier in 2.1. We should remove it before GA.
Closing in favor of PR #14817
The Couchbase team have advised us to use Couchbase's DiagnosticsReport. In a reactive context, that means that we need to use AsyncCluster#diagnostics. As things stand, we only auto-configure a Cluster. I'm not sure what the implications of also auto-configuring an AsyncCluster might be.
@simonbasle, can you help us with your expertise here please?
When we move over, we need to remove CouchbaseHealthIndicatorProperties as they're only used by the reactive indicator.
@wilkinsona I've used the timeout there for consistency but I think we should remove timeout right away on master. If the new implementation doesn't need it, then the reactive impl shouldn't use it either.
Ok. I'll take care of that as part of merging the fix for #14685 forwards.
I don't think we'd want to configure a Cluster and an AsyncCluster as they won't share resources.
A CouchbaseCluster (implementation of Cluster) is a blocking wrapper around CouchbaseAsyncCluster (implementation of AsyncCluster). Unfortunately, there's no public API for creating a CouchbaseCluster from CouchbaseAsyncCluster or for getting the CouchbaseAsyncCluster from a CouchbaseCluster. Short of resorting to reflection, I'm not sure what we can do here. We might be back to dropping the reactive health indicator after all.
I'm actually surprised not to see an async() method on the Cluster like there is on the Bucket 馃
I guess using the Cluster#diagnostics _could_ be good enough _iff_ the underlying core response doesn't trigger IO every time, but I'm not sure of that. We need @daschl expertise there again 馃槃
@simonbasle I think we can add the async() in the next bugfix release if we need to - it just hasn't come up yet (usually folks use the sync cluster and bucket to bootstrap since it's convenient and then drop into async() every time they need to do something with rx).
On another note, diagnostics right now never touches the network since it just aggregates local state - we could even wrap that into a Single and call it a day.
@wilkinsona which approach do you prefer?
Thank you, @daschl. It sounds like we can just use Cluster#diagnostics for now as it doesn't touch the network. That's a little bit of a dangerous assumption to make and it will make the code look a little bit odd. The addition of an async method on Cluster in a future bug fix release would be most welcome.
tracked via https://issues.couchbase.com/browse/JCBC-1259
Most helpful comment
tracked via https://issues.couchbase.com/browse/JCBC-1259