As we discussed offline, it would be good to see the refactoring changes
separated from the functional changes. That makes it easier to see what's
changed and refer back in future.
On 12 April 2014 06:20, Alex Wang <al...@nicira.com> wrote:
>
> @@ -2450,8 +2373,40 @@ bridge_run(void)
> iface_stats_timer = time_msec() + IFACE_STATS_INTERVAL;
> }
>
> + /* Check the need to update status. */
> + seq = seq_read(connectivity_seq_get());
>
> + if (seq != connectivity_seqno) {
>
I think that in the previous version, this if statement...
> + enum ovsdb_idl_txn_status status;
> +
> + if (!status_txn) {
>
And this one are in the opposite order. More to follow...
> ...
> + status = ovsdb_idl_txn_commit(status_txn);
> + /* Do not destroy "status_txn" if the transaction is
> + * "TXN_INCOMPLETE". */
> + if (status != TXN_INCOMPLETE) {
> + ovsdb_idl_txn_destroy(status_txn);
> + status_txn = NULL;
> + }
The if statement order above makes this code not get run unless there was a
connectivity change. Is this an optimisation?
> @@ -2481,8 +2436,17 @@ bridge_wait(void)
> poll_timer_wait_until(iface_stats_timer);
> }
>
> + /* If the status database transaction is "TXN_INCOMPLETE" in this run,
> + * register a timeout in "STATUS_CHECK_AGAIN_MSEC". Else, wait on the
> + * global connectivity sequence number. Note, this also helps batch
> + * multiple status changes into one transaction. */
> + if (status_txn) {
> + poll_timer_wait_until(time_msec() + STATUS_CHECK_AGAIN_MSEC);
> + } else {
> + seq_wait(connectivity_seq_get(), connectivity_seqno);
> + }
>
So, this is like a backoff? If there's so much database update happening
that we can't immediately transact (I equate this with TXN_INCOMPLETE),
then don't try again for another 500ms?
Otherwise, respond immediately to any changes to connectivity?
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
