On Fri, Mar 5, 2021 at 5:24 PM Ben Pfaff <[email protected]> wrote: > > The DDlog code used by ovn-northd-ddlog relies on always having a value > (exactly one of them) in the NbCfgTimestamp relation. The C code > inserts and updates this value. It's supposed to insert the initial > value the first time that it processes any update from the northbound > database. > > However, it actually only did this if the first update it processed was > from the northbound database--if it was from the southbound database, > it forgot to do so even when the first northbound update showed up. > The DDlog code would then go kind of berzerk creating and deleting the > NB_Global record until something actually happened to increment nb_cfg > (such as "ovn-nbctl --wait=sb sync"). > > This fixes the problem by only recording that a value was inserted to > NbCfgTimestamp when the first update from the northbound database is > received, same as originally intended. > > Signed-off-by: Ben Pfaff <[email protected]> > --- > northd/ovn-northd-ddlog.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/northd/ovn-northd-ddlog.c b/northd/ovn-northd-ddlog.c > index aa0ea73e401d..f373bb129a89 100644 > --- a/northd/ovn-northd-ddlog.c > +++ b/northd/ovn-northd-ddlog.c > @@ -445,8 +445,10 @@ northd_parse_updates(struct northd_ctx *ctx, struct ovs_list *updates) > if (ddlog_commit(ctx->ddlog)) { > goto error; > } > - old_nb_cfg = new_nb_cfg; > - old_nb_cfg_timestamp = new_nb_cfg_timestamp; > + if (ctx->has_timestamp_columns) { > + old_nb_cfg = new_nb_cfg; > + old_nb_cfg_timestamp = new_nb_cfg_timestamp; > + }
Hi Ben, sorry that I didn't understand this patch. It seems this change wouldn't change the behavior. It only adds the if condition before assigning old_nb_cfg and old_nb_cfg_timestamp. If ctx->has_timestamp_columns is true, it is the same behavior as before; if ctx->has_timestamp_columns is false, new_nb_cfg and new_nb_cfg_timestamp wouldn't change (in the previous if-block not shown in this diff), then before this change the old_nb_cfg and old_nb_cfg_timestamp would just be re-assigned to their original values (a no-op) - same effect as this change. Did I misunderstand anything? I also couldn't see any link between this change and the commit message. Thanks, Han > > /* This update may have implications for the other side, so > * immediately wake to check for more changes to be applied. */ > -- > 2.29.2 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
