[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