On 6/15/20 6:21 PM, Han Zhou wrote: > > > On Mon, Jun 15, 2020 at 8:18 AM Dumitru Ceara <[email protected] > <mailto:[email protected]>> 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 <[email protected] <mailto:[email protected]>> >> >>> Fixes: db2b5757328c ("lib: add monitor2 support in ovsdb-idl.") >> >>> Signed-off-by: Dumitru Ceara <[email protected] >> >>> <mailto:[email protected]>> >> >>> --- >> >>> 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. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
