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 <dce...@redhat.com >> <mailto:dce...@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 <az...@ovn.org <mailto:az...@ovn.org>> >>>>>> Fixes: db2b5757328c ("lib: add monitor2 support in ovsdb-idl.") >>>>>> Signed-off-by: Dumitru Ceara <dce...@redhat.com >>>>>> <mailto:dce...@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! _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
