-
Notifications
You must be signed in to change notification settings - Fork 138
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
Handle changes to Couchbase Server cluster topology at runtime #942
Comments
go-couchbase doesn't actually support natively - we're using ConnectWithAuth to establish the standard bucket connection, which takes a single URL. https://github.com/couchbase/go-couchbase/blob/master/pools.go#L579 It's cb-datasource (the DCP feed runner that was moved to the go-couchbase repo) that's added the additional layer to support trying multiple server URLs (https://github.com/couchbase/go-couchbase/blob/master/cbdatasource/cbdatasource.go#L481). I don't think it makes sense for us to reimplement the server URL iteration in SG - it would make more sense as a go-couchbase enhancement to move the handling down from cb-datasource into go-couchbase. It looks like the functionality is already in place in gocb - another option would be to wait until we move to that library. https://github.com/couchbaselabs/gocb/blob/fab5b631290a984e214702c7190d5833849e8b6e/connspec.go#L25 |
Just a general note that this enhancement is intended to make it easier to upgrade CBS. |
Followup based on the discussion of the upgrade scenario: go-couchbase will try to refresh it's bucket definition when it gets an error making a memcached request. (see https://github.com/couchbase/go-couchbase/blob/master/client.go#L264 for one example). It's going to use the original server URL to do that refresh. If that's the node that's unavailable (because it's getting upgraded), the refresh fails. Ideally Sync Gateway would provide a list of server URLs to go-couchbase, and go-couchbase would use all of these when attempting to refresh a bucket. |
This issue is causing the upgrade process grow more complex, and hence more error prone. @ashvindersingh is going to open up a ticket for go-couchbase, and link back to here, so that we can get Manik's help. |
Created ticket: https://issues.couchbase.com/browse/MB-15525 |
So you guys need a ConnectWithAuth() type API that accepts a list of Server URLs and automatically reconnects to different url if one that it was previously connected to fails ? If that is the case then it sounds like a wrapper that can be easily implemented in the application. |
@maniktaneja The main issue is that we need the list of server URLs to be used when go-couchbase does a bucket.refresh(), like mentioned above, which doesn't work with the wrapper approach. |
@adamcfraser Can you define the exact API spec that you need ? |
Re-assigning to myself due to this being a pre-requisite to #969 which is assigned to me -- see #969 (comment) |
@maniktaneja The key idea is for clients to provide a list of URLs to go-couchbase when initializing a client, and then have the client have the ability to retry over all entries in that list until it finds one it's able to connect to successfully. As mentioned above, the other key point is that the list should get used internally when doing things like a bucket refresh(). The particular use case QE is focusing on at the moment is during upgrade, when the cluster node corresponding to the current single URL is brought down. We need to be able to handle this scenario smoothly and transition to using one of the other server nodes if we need to refresh the bucket (e.g. rebalance, etc). |
Retry would be needed here:
in pools.go |
Update:
|
Note: we should log a warning on cluster configuration change, tracked on issue #1011. |
Currently, the "server" key of the database config JSON only supports a single string value,
despite the fact that the bucket source's constructor in go-couchbase actually takes a vector value of URLs in the client config.
The proposal is to permit the user to directly provide multiple cluster manager endpoints to use during cluster map download, for failover purposes:
The text was updated successfully, but these errors were encountered: