Hi Xavier, Rosemarie, Thanks for the fix and review!
On 10/20/26 2:46 PM, Xavier Simonart via dev wrote: > CHECK_UNEXPECTED_DB_CHANGES only samples a short live window, so it > only catches db instability that is still ongoing right when the > macro is called. > It is usually called at the end of each test and hence detects > instability when the test completes. > However, a temporary fight between two processes can run for a while, > before the condition which caused the fight is deleted, leaving no > trace for that macro to see at the end of the test. > > Add a new CHECK_NO_REPEATED_DB_ENTRIES() macro which checks whether > entries were added and removed repeatedly (i.e. more than 500 times). > CHECK_NO_REPEATED_DB_ENTRIES_THRESHOLD can be used to higher this Nit: "to increase this threshold" reads better to me. > threshold or skip this test. > > Assisted-by: Claude Sonnet 5, Claude Code > Signed-off-by: Xavier Simonart <[email protected]> > --- > tests/ovn-controller-vtep.at | 1 + > tests/ovn-macros.at | 31 +++++++++++++++++++++++++++++++ > 2 files changed, 32 insertions(+) > > diff --git a/tests/ovn-controller-vtep.at b/tests/ovn-controller-vtep.at > index 5b2895676..f391ee177 100644 > --- a/tests/ovn-controller-vtep.at > +++ b/tests/ovn-controller-vtep.at > @@ -72,6 +72,7 @@ m4_define([OVN_CONTROLLER_VTEP_STOP],[ > test -n "$2" && as "$2" > #AT_CHECK([check_logs "$1"]) > OVS_APP_EXIT_AND_WAIT([ovs-vtep]) > + CHECK_NO_REPEATED_DB_ENTRIES > CHECK_UNEXPECTED_DB_CHANGES > OVS_APP_EXIT_AND_WAIT([ovn-controller-vtep]) > OVS_APP_EXIT_AND_WAIT([ovsdb-server]) > diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at > index aa2e4453b..e842a3746 100644 > --- a/tests/ovn-macros.at > +++ b/tests/ovn-macros.at > @@ -204,6 +204,34 @@ m4_define([CHECK_UNEXPECTED_DB_CHANGES],[ > fi > ]) > > +# CHECK_NO_REPEATED_DB_ENTRIES() > +# Checks every db used by the test for a transaction that recurs more then Typo: more than. > +# 500 times, once # uuids and "_date" are normalized away over the db's Stray #. > +# whole recorded history. > +# > +# A test that purposely adds/removes the same entry many times can raise the > +# threshold, or skip ths check entirely, by setting Typo: ths. > +# CHECK_NO_REPEATED_DB_ENTRIES_THRESHOLD. > +m4_define([CHECK_NO_REPEATED_DB_ENTRIES], [ > + dup_threshold=${CHECK_NO_REPEATED_DB_ENTRIES_THRESHOLD-500} > + if test -n "$dup_threshold"; then > + db_list=$(find "$ovs_base" -name "*.db" | sort) > + for db in $db_list; do > + # normalize away uuids and "_date" so that transactions with the > + # same logical content compare equal, count how many times each > one > + # occurs, and keep only if occuring > $dup_threshold". Typo: occuring. And stray '"'. > + dup=$(sed > 's/[[0-9a-f]]\{8\}-[[0-9a-f]]\{4\}-[[0-9a-f]]\{4\}-[[0-9a-f]]\{4\}-[[0-9a-f]]\{12\}/UUID/g > + s/"_date":[[0-9]]*/"_date":0/' "$db" | \ > + sort | uniq -c | sort -rn | awk -v thresh="$dup_threshold" > '$'1' > thresh') The '$'1' > thresh' really made me read this a bunch of times. I understand now it's because autoconf would expand it to the "first macro argument" if we wrote it as '$1 > thresh'. But it turns out we can use autoconf quadigraphs and write this as '@S|@1 > thresh'. Where "@S|@" is the quadigraph for "$". https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.73/autoconf.html#Quadrigraphs > + dup_max=$(printf '%s\n' "$dup" | head -n 1 | tr -s ' ' | cut -d' > ' -f2) > + if test -n "$dup_max" && test "$dup_max" -gt "$dup_threshold"; > then > + printf 'Repeated db transactions in %s:\n%s\n' "$db" "$dup" > + AT_CHECK([test "$dup_max" -le "$dup_threshold"]) > + fi > + done > + fi > +]) > + > # OVN_CLEANUP_DBS > # > # Gracefully terminate NB, SB OVN daemons. > @@ -219,6 +247,7 @@ m4_define([OVN_CLEANUP_DBS],[ > # > # Gracefully terminate all central OVN daemons: NB, SB and ovn-northd. > m4_define([OVN_CLEANUP_NORTHD],[ > + CHECK_NO_REPEATED_DB_ENTRIES > CHECK_UNEXPECTED_DB_CHANGES > OVN_CLEANUP_DBS > > @@ -479,6 +508,7 @@ m4_define([OVN_CLEANUP_SBOX],[ > # Gracefully terminate all OVN daemons, including those in the > # specified sandbox instances. > m4_define([OVN_CLEANUP],[ > + CHECK_NO_REPEATED_DB_ENTRIES > CHECK_UNEXPECTED_DB_CHANGES > m4_foreach([sbox_and_error], [$@], [ > sbox=$(echo "sbox_and_error" |sed -n '1p') > @@ -543,6 +573,7 @@ m4_define([OVN_CLEANUP_AZ],[ > # Gracefully terminate all interconnection DBs, and daemons in the > # specified AZs, if any. > m4_define([OVN_CLEANUP_IC],[ > + CHECK_NO_REPEATED_DB_ENTRIES > CHECK_UNEXPECTED_DB_CHANGES > m4_foreach([az], [$@], [ > OVN_CLEANUP_AZ([az]) With the minor changes above addressed, I pushed this patch to main. I initially also wanted to backport it but realized we miss the CHECK_UNEXPECTED_DB_CHANGES() macro on older branches (<=26.09). Do you think we should backport that one too first? Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
