| Message ID | 20210908121859.3922083-1-i.maximets@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] ovn-northd: Avoid verification of NB_Global.options if nothing changed. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On 08/09/2021 13:18, Ilya Maximets wrote: > northd constructs transaction for the Northbound database options even > if they don't need to be changed. This also includes verification of > a current state. IDL will eventually drop the transaction if it will > not contain any other operations, but if there are some other > operations, this useless update to the same value will be included > along with the verification. This causes transaction failures because > NB_Global.options can be updated by CMS at the same time. > > For example, ovn-kubernetes sets 'options:e2e_timestamp' for it's own > purposes, and if this value will be updated while there is an in-flight > transaction from the northd, transaction will fail the verification and > northd will have to re-try it. > > To avoid these issues, updating and verifying options only if needed. > > Fixes: b07f1bc3d068 ("Add VXLAN support for non-VTEP datapath bindings") > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > --- Acked-by: Mark D. Gray <mark.d.gray@redhat.com>
I merged this to master, branch-21.06 and branch-21.03. On 9/9/21 6:41 AM, Mark Gray wrote: > On 08/09/2021 13:18, Ilya Maximets wrote: >> northd constructs transaction for the Northbound database options even >> if they don't need to be changed. This also includes verification of >> a current state. IDL will eventually drop the transaction if it will >> not contain any other operations, but if there are some other >> operations, this useless update to the same value will be included >> along with the verification. This causes transaction failures because >> NB_Global.options can be updated by CMS at the same time. >> >> For example, ovn-kubernetes sets 'options:e2e_timestamp' for it's own >> purposes, and if this value will be updated while there is an in-flight >> transaction from the northd, transaction will fail the verification and >> northd will have to re-try it. >> >> To avoid these issues, updating and verifying options only if needed. >> >> Fixes: b07f1bc3d068 ("Add VXLAN support for non-VTEP datapath bindings") >> Signed-off-by: Ilya Maximets <i.maximets@ovn.org> >> --- > > Acked-by: Mark D. Gray <mark.d.gray@redhat.com> > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index eef752542..baaddb73e 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -14233,8 +14233,10 @@ ovnnb_db_run(struct northd_context *ctx, ovn_internal_version); } - nbrec_nb_global_verify_options(nb); - nbrec_nb_global_set_options(nb, &options); + if (!smap_equal(&nb->options, &options)) { + nbrec_nb_global_verify_options(nb); + nbrec_nb_global_set_options(nb, &options); + } smap_destroy(&options);
northd constructs transaction for the Northbound database options even if they don't need to be changed. This also includes verification of a current state. IDL will eventually drop the transaction if it will not contain any other operations, but if there are some other operations, this useless update to the same value will be included along with the verification. This causes transaction failures because NB_Global.options can be updated by CMS at the same time. For example, ovn-kubernetes sets 'options:e2e_timestamp' for it's own purposes, and if this value will be updated while there is an in-flight transaction from the northd, transaction will fail the verification and northd will have to re-try it. To avoid these issues, updating and verifying options only if needed. Fixes: b07f1bc3d068 ("Add VXLAN support for non-VTEP datapath bindings") Signed-off-by: Ilya Maximets <i.maximets@ovn.org> --- northd/ovn-northd.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)