| Message ID | 20260319142709.317535-1-panos.kostopouloskyrimis@canonical.com |
|---|---|
| State | Rejected |
| Delegated to: | Ilya Maximets |
| Headers | show |
| Series | [ovs-dev,v2] ovsdb-client: Allow multiple servers in ovsdb-client wait. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/cirrus-robot | success | cirrus build: passed |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On 3/19/26 3:27 PM, Panos Kostopoulos Kyrimis via dev wrote: > Unlike other ovsdb-client commands such as needs-conversion, > wait does not accept multiple servers, causing users to run > multiple ovsdb-client and wait for each server until one is connected. > > Reported-at: https://bugs.launchpad.net/bugs/2127931 > Signed-off-by: Panos Kostopoulos Kyrimis <panos.kostopouloskyrimis@canonical.com> > --- > Looking at the example of needs-conversion, since I was going to add > this feature, I thought maybe it would be beneficial to try use the open_rpc > function in do_wait as well. At start it seamed like the transition > would be easy as the code is similar. I tried some approaches with no result. An > issue was open_rpc in a single server calls open_jsonrpc which basically > is blocking request/receive, while in the case of multiple servers it > uses jsonrpc_session_open + jsonrpc_session_run in a loop. Another issue > is the retry flag which is false in open_rpc (but could be fixed with > the addition of a retry boolean in open_rpc). > > I wonder if my addition is sufficient, or removing code duplication from > do_wait is as important (if open_rpc can be used in this case that is). > In the latter case I would need some guidance as to how to approach this > as the number of available options for rpc connections is overwhelming. Hi, Panos. Thanks for the patch! Though specifying multiple destinations for the 'wait' command doesn't make a lot of sense. It is designed to check the status of a particular cluster member and that is the documented behavior. If we're connecting to multiple servers, then it would probably need to connect to all of them one by one and wait until all of them are 'added', 'connected' or 'removed', which is not that different from just calling it for each server one by one. Checking the 'connected' status on the leader doesn't make much sense, as the leader is always connected, added and not removed. Same partially applies to the needs-conversion command. It's not clear what it supposed to return if the servers are not fully caught up with the leader and some of them are on the older schema still. Does that make sense? Modifying the open_rpc() may also not be desired as tests and maybe some users rely on the failure to connect without delays, which will be problematic if the reconnection is turned on and we're running in the loop. Best regards, Ilya Maximets.
diff --git a/ovsdb/ovsdb-client.1.in b/ovsdb/ovsdb-client.1.in index c15804597..7ce650159 100644 --- a/ovsdb/ovsdb-client.1.in +++ b/ovsdb/ovsdb-client.1.in @@ -314,8 +314,8 @@ to check for databases that have not yet been added to the server, so that the \fBovsdb\-client\fR semantics of acting on a default database do not work. .IP -This command acts on a particular database server, not on a cluster, -so \fIserver\fR must name a single server, not a comma-delimited list +This command acts on a particular database server, as well as on a cluster, +so \fIserver\fR can name a single server, or a comma-delimited list of servers. .SS "Testing commands" These commands are mostly of interest for testing the correctness diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c index 37cfa8b56..a33ba6165 100644 --- a/ovsdb/ovsdb-client.c +++ b/ovsdb/ovsdb-client.c @@ -2453,9 +2453,17 @@ do_wait(struct jsonrpc *rpc_unused OVS_UNUSED, ovs_fatal(0, "%s: unknown state", state); } - char *remote = argc > 2 ? xstrdup(argv[0]) : default_remote(); - struct jsonrpc_session *js = jsonrpc_session_open(remote, true); - free(remote); + struct svec remotes = SVEC_EMPTY_INITIALIZER; + struct uuid cid = UUID_ZERO; + + if (argc > 2) { + ovsdb_session_parse_remote(argv[0], &remotes, &cid); + } else { + svec_add_nocopy(&remotes, default_remote()); + } + + struct jsonrpc_session *js = jsonrpc_session_open_multiple(&remotes, true); + svec_destroy(&remotes); unsigned int seqno = 0; struct json *sdca_id = NULL;
Unlike other ovsdb-client commands such as needs-conversion, wait does not accept multiple servers, causing users to run multiple ovsdb-client and wait for each server until one is connected. Reported-at: https://bugs.launchpad.net/bugs/2127931 Signed-off-by: Panos Kostopoulos Kyrimis <panos.kostopouloskyrimis@canonical.com> --- Looking at the example of needs-conversion, since I was going to add this feature, I thought maybe it would be beneficial to try use the open_rpc function in do_wait as well. At start it seamed like the transition would be easy as the code is similar. I tried some approaches with no result. An issue was open_rpc in a single server calls open_jsonrpc which basically is blocking request/receive, while in the case of multiple servers it uses jsonrpc_session_open + jsonrpc_session_run in a loop. Another issue is the retry flag which is false in open_rpc (but could be fixed with the addition of a retry boolean in open_rpc). I wonder if my addition is sufficient, or removing code duplication from do_wait is as important (if open_rpc can be used in this case that is). In the latter case I would need some guidance as to how to approach this as the number of available options for rpc connections is overwhelming. ovsdb/ovsdb-client.1.in | 4 ++-- ovsdb/ovsdb-client.c | 14 +++++++++++--- 2 files changed, 13 insertions(+), 5 deletions(-)