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

Ilya Maximets i.maximets at ovn.org
Mon Jun 15 17:52:19 UTC 2020


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.


More information about the dev mailing list