On Mon, Mar 08, 2021 at 10:47:40AM -0800, Han Zhou wrote: > 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.
This changes the scenario where ctx->has_timestamp_columns is false and old_nb_cfg is INT64_MIN. In the previous version of the code would change old_nb_cfg to 0; in the new version of the code, it keeps its value of INT64_MIN. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
