[ovs-dev] [PATCH] ovsdb-cs: Avoid unnecessary re-connections when updating remotes.

[Dumitru Ceara]

On 7/15/21 7:37 PM, Han Zhou wrote:
> On Thu, Jul 15, 2021 at 9:17 AM Ilya Maximets <i.maximets at ovn.org> wrote:
>>
>> On 6/29/21 9:57 PM, Ilya Maximets wrote:
>>>>> Regarding the current patch, I think it's better to add a test case to
>>>>> cover the scenario and confirm that existing connections didn't
> reset. With
>>>>> that:
>>>>> Acked-by: Han Zhou <hzhou at ovn.org>
>>>
>>> I'll work on a unit test for this.
>>
>> Hi.  Here is a unit test that I came up with:
>>
>> diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
>> index 62181dd4d..e32f9ec89 100644
>> --- a/tests/ovsdb-idl.at
>> +++ b/tests/ovsdb-idl.at
>> @@ -2282,3 +2282,27 @@ OVSDB_CHECK_CLUSTER_IDL_C([simple idl,
> monitor_cond_since, cluster disconnect],
>>  008: table simple: i=1 r=2 b=true s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[]
> uuid=<2>
>>  009: done
>>  ]])
>> +
>> +dnl This test checks that IDL keeps the existing connection to the
> server if
>> +dnl it's still on a list of remotes after update.
>> +OVSDB_CHECK_IDL_C([simple idl, initially empty, set remotes],
>> +  [],
>> +  [['set-remote unix:socket' \
>> +    '+set-remote unix:bad_socket,unix:socket' \
>> +    '+set-remote unix:bad_socket' \
>> +    '+set-remote unix:socket' \
>> +    'set-remote unix:bad_socket,unix:socket' \
>> +    '+set-remote unix:socket' \
>> +    '+reconnect']],
>> +  [[000: empty
>> +001: new remotes: unix:socket, is connected: true
>> +002: new remotes: unix:bad_socket,unix:socket, is connected: true
>> +003: new remotes: unix:bad_socket, is connected: false
>> +004: new remotes: unix:socket, is connected: false
>> +005: empty
>> +006: new remotes: unix:bad_socket,unix:socket, is connected: true
>> +007: new remotes: unix:socket, is connected: true
>> +008: reconnect
>> +009: empty
>> +010: done
>> +]])
>> diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
>> index a886f971e..93329cd4c 100644
>> --- a/tests/test-ovsdb.c
>> +++ b/tests/test-ovsdb.c
>> @@ -2621,6 +2621,7 @@ do_idl(struct ovs_cmdl_context *ctx)
>>      setvbuf(stdout, NULL, _IONBF, 0);
>>
>>      symtab = ovsdb_symbol_table_create();
>> +    const char remote_s[] = "set-remote ";
>>      const char cond_s[] = "condition ";
>>      if (ctx->argc > 2 && strstr(ctx->argv[2], cond_s)) {
>>          update_conditions(idl, ctx->argv[2] + strlen(cond_s));
>> @@ -2664,6 +2665,11 @@ do_idl(struct ovs_cmdl_context *ctx)
>>          if (!strcmp(arg, "reconnect")) {
>>              print_and_log("%03d: reconnect", step++);
>>              ovsdb_idl_force_reconnect(idl);
>> +        }  else if (!strncmp(arg, remote_s, strlen(remote_s))) {
>> +            ovsdb_idl_set_remote(idl, arg + strlen(remote_s), true);
>> +            print_and_log("%03d: new remotes: %s, is connected: %s",
> step++,
>> +                          arg + strlen(remote_s),
>> +                          ovsdb_idl_is_connected(idl) ? "true" :
> "false");
>>          }  else if (!strncmp(arg, cond_s, strlen(cond_s))) {
>>              update_conditions(idl, arg + strlen(cond_s));
>>              print_and_log("%03d: change conditions", step++);
>> ---
>>
>> Dumitru, Han, if it looks good to you, I can squash it in before
>> applying the patch.   What do you think?
> 
> Thanks Ilya. LGTM.

Looks good to me too, thanks!

> 
>>
>> Best regards, Ilya Maximets.
>