[ovs-dev] [PATCH v6 3/3] ovsdb-idl: Force IDL retry when missing updates encountered.

Dumitru Ceara dceara at redhat.com
Mon Jun 15 15:17:57 UTC 2020


On 6/15/20 2:41 PM, Dumitru Ceara wrote:
> On 6/15/20 2:35 PM, Ilya Maximets wrote:
>> On 5/28/20 2:32 PM, Dumitru Ceara wrote:
>>> Adds a generic recovery mechanism which triggers an IDL retry with fast
>>> resync disabled in case the IDL has detected that it ended up in an
>>> inconsistent state due to other bugs in the ovsdb-server/ovsdb-idl
>>> implementation.
>>>
>>> CC: Andy Zhou <azhou at ovn.org>
>>> Fixes: db2b5757328c ("lib: add monitor2 support in ovsdb-idl.")
>>> Signed-off-by: Dumitru Ceara <dceara at redhat.com>
>>> ---
>>>  lib/ovsdb-idl.c |   37 +++++++++++++++++++++++++++++++++----
>>>  1 file changed, 33 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
>>> index 5abe40f..676ea2c 100644
>>> --- a/lib/ovsdb-idl.c
>>> +++ b/lib/ovsdb-idl.c
>>> @@ -328,7 +328,8 @@ static bool ovsdb_idl_process_update(struct ovsdb_idl_table *,
>>>  static bool ovsdb_idl_process_update2(struct ovsdb_idl_table *,
>>>                                        const struct uuid *,
>>>                                        const char *operation,
>>> -                                      const struct json *row);
>>> +                                      const struct json *row,
>>> +                                      bool *inconsistent);
>>
>> Hi Dumitru,
>> The change looks good to me logically, but I don't really like that pointer-like
>> result returning.  I think it might be better to change result type from boolean
>> to enum like this:
>>
>> enum update2_result {
>>     OVSDB_IDL_UPDATE2_DB_CHANGED,
>>     OVSDB_IDL_UPDATE2_NO_CHANGES,
>>     OVSDB_IDL_UPDATE2_INCONSISTENT,
>> };
>>
>> This might make code more readable.
>>
>> Dumitru, Han, what do you think?
>>
> 
> Hi Ilya,
> 
> Sounds good to me, should be more readable indeed. I'll change
> ovsdb_idl_process_update/ovsdb_idl_process_update2 to return the enum
> value as you suggested.
> 

Hi Han, others,

Ilya pointed out that we don't really need this last patch as we fixed
the original issue and if we hit other bugs we will just hide them.

I started having second thoughts too.

Do you agree with abandoning this last patch of the series?

Thanks,
Dumitru



More information about the dev mailing list