2
0
mirror of https://github.com/openvswitch/ovs synced 2025-08-22 01:51:26 +00:00

python: idl: Fix last-id update from a monitor reply.

While sending a reply to the monitor_cond_since request, server
includes the last transaction ID.  And it sends new IDs with each
subsequent update.  Current implementation doesn't use the one
supplied with a monitor reply, and only takes into account IDs
provided with monitor updates.  That may cause various issues:

1. Performance: During initialization, the last-id is set to zero.
   If re-connection will happen after receiving a monitor reply,
   but before any monitor update, the client will send a new
   monitor request with an all-zero last-id and will re-download
   the whole database again.

2. Data inconsistency: Assuming one of the clients sends a
   transaction, but our python client disconnects before receiving
   a monitor update for this transaction.  The last-id will point
   to a database state before this transaction.  On re-connection,
   this last-id will be sent and the monitor reply will contain
   a diff with a new data from that transaction.  But if another
   disconnection happens right after that, on second re-connection
   our python client will send another monitor_cond_since with
   exactly the same last-id.  That will cause receiving the same
   set of updates again.  And since it's an update2 message with
   a diff of the data, the client will remove previously applied
   result of the transaction.  At this point it will have a
   different database view with the server potentially leading
   to all sorts of data inconsistency problems.

Fix that by always updating the last-id from the latest monitor
reply.

Fixes: 46d44cf3be0d ("python: idl: Add monitor_cond_since support.")
Acked-by: Simon Horman <horms@ovn.org>
Acked-by: Han Zhou <hzhou@ovn.org>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
This commit is contained in:
Ilya Maximets 2023-09-09 04:18:36 +02:00
parent bac34b26a7
commit 0896dc19ef
2 changed files with 22 additions and 1 deletions

View File

@ -494,6 +494,7 @@ class Idl(object):
if not msg.result[0]: if not msg.result[0]:
self.__clear() self.__clear()
self.__parse_update(msg.result[2], OVSDB_UPDATE3) self.__parse_update(msg.result[2], OVSDB_UPDATE3)
self.last_id = msg.result[1]
elif self.state == self.IDL_S_DATA_MONITOR_COND_REQUESTED: elif self.state == self.IDL_S_DATA_MONITOR_COND_REQUESTED:
self.__clear() self.__clear()
self.__parse_update(msg.result, OVSDB_UPDATE2) self.__parse_update(msg.result, OVSDB_UPDATE2)

View File

@ -2332,6 +2332,23 @@ CHECK_STREAM_OPEN_BLOCK([Python3], [$PYTHON3 $srcdir/test-stream.py],
CHECK_STREAM_OPEN_BLOCK([Python3], [$PYTHON3 $srcdir/test-stream.py], CHECK_STREAM_OPEN_BLOCK([Python3], [$PYTHON3 $srcdir/test-stream.py],
[ssl6], [[[::1]]]) [ssl6], [[[::1]]])
dnl OVSDB_CLUSTER_CHECK_MONITOR_COND_SINCE_TXN_IDS(LOG)
dnl
dnl Looks up transaction IDs in the log of OVSDB client application.
dnl All-zero UUID should not be sent within a monitor request more than once,
dnl unless some database requests were lost (not replied).
m4_define([OVSDB_CLUSTER_CHECK_MONITOR_COND_SINCE_TXN_IDS],
[
requests=$(grep -c 'send request' $1)
replies=$(grep -c 'received reply' $1)
if test "$requests" -eq "$replies"; then
AT_CHECK([grep 'monitor_cond_since' $1 \
| grep -c "00000000-0000-0000-0000-000000000000" | tr -d '\n'],
[0], [1])
fi
])
# same as OVSDB_CHECK_IDL but uses Python IDL implementation with tcp # same as OVSDB_CHECK_IDL but uses Python IDL implementation with tcp
# with multiple remotes to assert the idl connects to the leader of the Raft cluster # with multiple remotes to assert the idl connects to the leader of the Raft cluster
m4_define([OVSDB_CHECK_IDL_LEADER_ONLY_PY], m4_define([OVSDB_CHECK_IDL_LEADER_ONLY_PY],
@ -2347,10 +2364,11 @@ m4_define([OVSDB_CHECK_IDL_LEADER_ONLY_PY],
pids=$(cat s2.pid s3.pid s1.pid | tr '\n' ',') pids=$(cat s2.pid s3.pid s1.pid | tr '\n' ',')
echo $pids echo $pids
AT_CHECK([$PYTHON3 $srcdir/test-ovsdb.py -t30 idl-cluster $srcdir/idltest.ovsschema $remotes $pids $3], AT_CHECK([$PYTHON3 $srcdir/test-ovsdb.py -t30 idl-cluster $srcdir/idltest.ovsschema $remotes $pids $3],
[0], [stdout], [ignore]) [0], [stdout], [stderr])
remote=$(ovsdb_cluster_leader $remotes "idltest") remote=$(ovsdb_cluster_leader $remotes "idltest")
leader=$(echo $remote | cut -d'|' -f 1) leader=$(echo $remote | cut -d'|' -f 1)
AT_CHECK([grep -F -- "${leader}" stdout], [0], [ignore]) AT_CHECK([grep -F -- "${leader}" stdout], [0], [ignore])
OVSDB_CLUSTER_CHECK_MONITOR_COND_SINCE_TXN_IDS([stderr])
AT_CLEANUP]) AT_CLEANUP])
OVSDB_CHECK_IDL_LEADER_ONLY_PY([Check Python IDL connects to leader], 3, ['remote']) OVSDB_CHECK_IDL_LEADER_ONLY_PY([Check Python IDL connects to leader], 3, ['remote'])
@ -2393,6 +2411,7 @@ m4_define([OVSDB_CHECK_CLUSTER_IDL_C],
AT_CHECK([sort stdout | uuidfilt]m4_if([$7],,, [[| $7]]), AT_CHECK([sort stdout | uuidfilt]m4_if([$7],,, [[| $7]]),
[0], [$5]) [0], [$5])
m4_ifval([$8], [AT_CHECK([grep '$8' stderr], [1])], [], []) m4_ifval([$8], [AT_CHECK([grep '$8' stderr], [1])], [], [])
OVSDB_CLUSTER_CHECK_MONITOR_COND_SINCE_TXN_IDS([stderr])
AT_CLEANUP]) AT_CLEANUP])
# Same as OVSDB_CHECK_CLUSTER_IDL_C but uses the Python IDL implementation. # Same as OVSDB_CHECK_CLUSTER_IDL_C but uses the Python IDL implementation.
@ -2413,6 +2432,7 @@ m4_define([OVSDB_CHECK_CLUSTER_IDL_PY],
AT_CHECK([sort stdout | uuidfilt]m4_if([$7],,, [[| $7]]), AT_CHECK([sort stdout | uuidfilt]m4_if([$7],,, [[| $7]]),
[0], [$5]) [0], [$5])
m4_if([$8], [AT_CHECK([grep '$8' stderr], [1])], [], []) m4_if([$8], [AT_CHECK([grep '$8' stderr], [1])], [], [])
OVSDB_CLUSTER_CHECK_MONITOR_COND_SINCE_TXN_IDS([stderr])
AT_CLEANUP]) AT_CLEANUP])
m4_define([OVSDB_CHECK_CLUSTER_IDL], m4_define([OVSDB_CHECK_CLUSTER_IDL],