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

Dumitru Ceara dceara at redhat.com
Mon Jun 15 18:38:07 UTC 2020


On 6/15/20 7:52 PM, Ilya Maximets wrote:
> On 6/15/20 6:21 PM, Han Zhou wrote:
>>
>>
>> On Mon, Jun 15, 2020 at 8:18 AM Dumitru Ceara <dceara at redhat.com <mailto:dceara at redhat.com>> wrote:
>>>
>>> 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 <mailto:azhou at ovn.org>>
>>>>>> Fixes: db2b5757328c ("lib: add monitor2 support in ovsdb-idl.")
>>>>>> Signed-off-by: Dumitru Ceara <dceara at redhat.com <mailto: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?
>>
>> Agree.
>>
> 
> OK.  I marked this patch as "Deferred" in patchwork.
> We could revisit this approach sometime later if needed.
> 
> Best regards, Ilya Maximets.
> 

Sounds good to me.

Thanks!



More information about the dev mailing list