On Mon, Dec 9, 2019 at 12:12 AM Dumitru Ceara <dce...@redhat.com> wrote: > > If the last ovn-controller main loop run started a transaction to the > Southbound DB and the transaction is still in progress, ovn-controller > will not call engine_run(). In case there were changes to the DB, > engine_need_run() would return true which would trigger an immediate > forced recompute in the next processing loop iteration. > > However, there are scenarios when updates can be processed incrementally > even if no Southbound DB transaction is available. One example, often > seen in the field, is when the MAC_Binding table is updated. Currently > such an update received while a transaction is still committing would > trigger a forced recompute. > > This patch performs only incremental processing when the SB DB txn > is NULL, which means executing change_handler() methods > only, without calling the run() methods of the I-P engine nodes. > Assuming that some change handlers need to update the DB and that > some don't, if the SB DB txn is NULL: > - if the incoming change doesn't trigger a SB DB update (currently true > for all existing change handlers) then the change handler might > complete without aborting if it doesn't trigger a run() of the node. > - if the incoming change tries to perform a SB DB update, the change > handler should return false (SB DB txn is NULL) triggering the engine > to call run() and abort computation. > > CC: Han Zhou <hz...@ovn.org> > CC: Numan Siddique <num...@ovn.org> > Signed-off-by: Dumitru Ceara <dce...@redhat.com> > > --- > v2: > - Address Han's comments: > - Remove unnecessary poll_immediate_wake() that could trigger a busy > loop until the transaction is committed. > - Document the change_handler requirements regarding using the txn > pointers returned by engine_get_context(). > - Rephrase commit message. > - Rebase. > --- > controller/ovn-controller.c | 8 ++++++++ > lib/inc-proc-eng.h | 15 ++++++++++++++- > 2 files changed, 22 insertions(+), 1 deletion(-) > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index 5874776..e47687b 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -2100,6 +2100,14 @@ main(int argc, char *argv[]) > } else { > engine_run(true); > } > + } else { > + /* Even if there's no SB DB transaction available, > + * try to run the engine so that we can handle any > + * incremental changes that don't require a recompute. > + * If a recompute is required, the engine will abort, > + * triggerring a full run in the next iteration. > + */ > + engine_run(false); > } > stopwatch_stop(CONTROLLER_LOOP_STOPWATCH_NAME, > time_msec()); > diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h > index 5b92971..780c3cd 100644 > --- a/lib/inc-proc-eng.h > +++ b/lib/inc-proc-eng.h > @@ -84,6 +84,10 @@ struct engine_node_input { > * evaluated against all the other inputs. Returns: > * - true: if change can be handled > * - false: if change cannot be handled (indicating full recompute needed) > + * A change handler can also call engine_get_context() but it must make > + * sure the txn pointers returned by it are non-NULL. In case the change > + * handler needs to use the txn pointers returned by engine_get_context(), > + * and the pointers are NULL, the change handler MUST return false. > */ > bool (*change_handler)(struct engine_node *node, void *data); > }; > @@ -133,6 +137,9 @@ struct engine_node { > > /* Fully processes all inputs of this node and regenerates the data > * of this node. The pointer to the node's data is passed as argument. > + * 'run' handlers can also call engine_get_context() and the > + * implementation guarantees that the txn pointers returned > + * engine_get_context() are not NULL and valid. > */ > void (*run)(struct engine_node *node, void *data); > > @@ -189,7 +196,13 @@ void engine_add_input(struct engine_node *node, struct engine_node *input, > * iteration, and the change can't be tracked across iterations */ > void engine_set_force_recompute(bool val); > > -const struct engine_context * engine_get_context(void); > +/* Return the current engine_context. The values in the context can be NULL > + * if the engine is run with allow_recompute == false in the current > + * iteration. > + * Therefore, it is the responsibility of the caller to check the context > + * values when called from change handlers. > + */ > +const struct engine_context *engine_get_context(void); > > void engine_set_context(const struct engine_context *); > > -- > 1.8.3.1 >
Thanks! I applied to master. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev