Adds a generic recovery mechanism which triggers an IDL retry with fast
resync disabled in case the IDL has detected that it ended up in an
inconsistent state due to other bugs in the ovsdb-server/ovsdb-idl
implementation.
Additionally, this commit also:
- bumps IDL semantic error logs to level ERR to make them more visible.
- triggers an IDL retry in cases when the IDL client used to try to
recover (i.e., trying to add an existing row, trying to remove a non
existent row, trying to modify a missing row).
_process_update()/_process_update2() now return an update result
(OVSDB_IDL_UPDATE_DB_CHANGED / _NO_CHANGES / _INCONSISTENT) alongside the
Notice, and _do_parse_update() flags an inconsistency and reconnects when
one is reported. flag_inconsistency() resets 'last_id' and forces a
reconnect, mirroring the C ovsdb_idl_retry() behavior.
De-mangle __process_update/__process_update2/__do_parse_update to
single-underscore names as they are rewritten here.
This is a port of OVS commit 9ed69557e5fa ("ovsdb-idl: Force IDL retry
when missing updates encountered.").
Assisted-by: Claude Opus 4.8 <[email protected]>
Signed-off-by: Terry Wilson <[email protected]>
---
python/ovs/db/idl.py | 174 +++++++++++++++++++++++++++++--------------
1 file changed, 118 insertions(+), 56 deletions(-)
diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
index 61107c189..417e8c020 100644
--- a/python/ovs/db/idl.py
+++ b/python/ovs/db/idl.py
@@ -39,6 +39,11 @@ OVSDB_UPDATE = "update"
OVSDB_UPDATE2 = "update2"
OVSDB_UPDATE3 = "update3"
+# Result of processing a single <row-update>.
+OVSDB_IDL_UPDATE_DB_CHANGED = 0
+OVSDB_IDL_UPDATE_NO_CHANGES = 1
+OVSDB_IDL_UPDATE_INCONSISTENT = 2
+
CLUSTERED = "clustered"
RELAY = "relay"
@@ -745,6 +750,16 @@ class Idl(object):
self._session.reset_backoff()
self._session.force_reconnect()
+ def flag_inconsistency(self):
+ """Tells the IDL that the client detected an inconsistency in the
+ database, so it must reconnect and re-download the whole database.
+
+ The 'last_id' is reset so that the IDL cannot request a fast resync
+ (monitor_cond_since) against the same server; instead the whole
+ database is re-downloaded, potentially from a different server."""
+ self.last_id = str(uuid.UUID(int=0))
+ self.force_reconnect()
+
def session_name(self):
return self._session.get_name()
@@ -922,14 +937,14 @@ class Idl(object):
def __parse_update(self, update, version, tables=None):
try:
if not tables:
- self.__do_parse_update(update, version, self.tables)
+ self._do_parse_update(update, version, self.tables)
else:
- self.__do_parse_update(update, version, tables)
+ self._do_parse_update(update, version, tables)
except error.Error as e:
vlog.err("%s: error parsing update: %s"
% (self._session.get_name(), e))
- def __do_parse_update(self, table_updates, version, tables):
+ def _do_parse_update(self, table_updates, version, tables):
if not isinstance(table_updates, dict):
raise error.Error("<table-updates> is not an object",
table_updates)
@@ -962,44 +977,70 @@ class Idl(object):
self.cooperative_yield()
if version in (OVSDB_UPDATE2, OVSDB_UPDATE3):
- changes = self._process_update2(table, uuid, row_update)
- if changes and tables is not self.server_tables:
- notices.append(changes)
- self.change_seqno += 1
- continue
-
- parser = ovs.db.parser.Parser(row_update, "row-update")
- old = parser.get_optional("old", [dict])
- new = parser.get_optional("new", [dict])
- parser.finish()
-
- if not old and not new:
- raise error.Error('<row-update> missing "old" and '
- '"new" members', row_update)
-
- changes = self.__process_update(table, uuid, old, new)
- if changes and tables is not self.server_tables:
- notices.append(changes)
+ result, notice = self._process_update2(table, uuid,
+ row_update)
+ else:
+ parser = ovs.db.parser.Parser(row_update, "row-update")
+ old = parser.get_optional("old", [dict])
+ new = parser.get_optional("new", [dict])
+ parser.finish()
+
+ if not old and not new:
+ raise error.Error('<row-update> missing "old" and '
+ '"new" members', row_update)
+
+ result, notice = self._process_update(table, uuid,
+ old, new)
+
+ if result == OVSDB_IDL_UPDATE_INCONSISTENT:
+ # The IDL ended up in an inconsistent state, e.g. because
+ # of a bug in the ovsdb-server/ovsdb-idl. Even though the
+ # client could recover, it's best to reconnect and resync
+ # the whole database (potentially from a different server).
+ self.flag_inconsistency()
+ raise error.Error("<row-update> received for inconsistent "
+ "IDL: reconnecting IDL and resync all "
+ "data")
+
+ if (result == OVSDB_IDL_UPDATE_DB_CHANGED
+ and tables is not self.server_tables):
+ notices.append(notice)
self.change_seqno += 1
for notice in notices:
self.notify(*notice)
def _process_update2(self, table, uuid, row_update):
- """Returns Notice if a column changed, False otherwise."""
+ """Returns a tuple (result, notice).
+
+ 'result' is one of OVSDB_IDL_UPDATE_DB_CHANGED,
+ OVSDB_IDL_UPDATE_NO_CHANGES or OVSDB_IDL_UPDATE_INCONSISTENT, and
+ 'notice' is a Notice describing the change or None.
+
+ Some IDL inconsistencies can be detected when processing updates:
+ - trying to insert an already existing row
+ - trying to update a missing row
+ - trying to delete a non existent row
+
+ In such cases OVSDB_IDL_UPDATE_INCONSISTENT is returned. Even though
+ the client could recover, it's best to report the inconsistent state
+ because the state the server is in is unknown, so the safest thing to
+ do is to retry (potentially connecting to a new server)."""
row = table.rows.get(uuid)
if "delete" in row_update:
if row:
del table.rows[uuid]
- return Notice(ROW_DELETE, row)
+ return OVSDB_IDL_UPDATE_DB_CHANGED, Notice(ROW_DELETE, row)
else:
# XXX rate-limit
- vlog.warn("cannot delete missing row %s from table"
- "%s" % (uuid, table.name))
+ vlog.err("cannot delete missing row %s from table %s"
+ % (uuid, table.name))
+ return OVSDB_IDL_UPDATE_INCONSISTENT, None
elif "insert" in row_update or "initial" in row_update:
if row:
- vlog.warn("cannot add existing row %s from table"
- " %s" % (uuid, table.name))
- del table.rows[uuid]
+ # XXX rate-limit
+ vlog.err("cannot add existing row %s to table %s"
+ % (uuid, table.name))
+ return OVSDB_IDL_UPDATE_INCONSISTENT, None
row = self.__create_row(table, uuid)
if "insert" in row_update:
row_update = row_update['insert']
@@ -1009,69 +1050,90 @@ class Idl(object):
changed = self.__row_update(table, row, row_update)
table.rows[uuid] = row
if changed:
- return Notice(ROW_CREATE, row)
+ return OVSDB_IDL_UPDATE_DB_CHANGED, Notice(ROW_CREATE, row)
+ return OVSDB_IDL_UPDATE_NO_CHANGES, None
elif "modify" in row_update:
if not row:
- raise error.Error('Modify non-existing row')
+ # XXX rate-limit
+ vlog.err("cannot modify missing row %s in table %s"
+ % (uuid, table.name))
+ return OVSDB_IDL_UPDATE_INCONSISTENT, None
del table.rows[uuid]
old_row, changed = self._apply_diff(table, row,
row_update['modify'])
table.rows[uuid] = row
if changed:
- return Notice(ROW_UPDATE, row, Row(self, table, uuid, old_row))
+ return (OVSDB_IDL_UPDATE_DB_CHANGED,
+ Notice(ROW_UPDATE, row, Row(self, table, uuid,
+ old_row)))
+ return OVSDB_IDL_UPDATE_NO_CHANGES, None
else:
- raise error.Error('<row-update> unknown operation',
- row_update)
- return False
-
- def __process_update(self, table, uuid, old, new):
- """Returns Notice if a column changed, False otherwise."""
+ # XXX rate-limit
+ vlog.err("unknown operation in <row-update> for table %s"
+ % table.name)
+ return OVSDB_IDL_UPDATE_NO_CHANGES, None
+
+ def _process_update(self, table, uuid, old, new):
+ """Returns a tuple (result, notice).
+
+ 'result' is one of OVSDB_IDL_UPDATE_DB_CHANGED,
+ OVSDB_IDL_UPDATE_NO_CHANGES or OVSDB_IDL_UPDATE_INCONSISTENT, and
+ 'notice' is a Notice describing the change or None.
+
+ Some IDL inconsistencies can be detected when processing updates:
+ - trying to insert an already existing row
+ - trying to update a missing row
+ - trying to delete a non existent row
+
+ In such cases OVSDB_IDL_UPDATE_INCONSISTENT is returned. Even though
+ the client could recover, it's best to report the inconsistent state
+ because the state the server is in is unknown, so the safest thing to
+ do is to retry (potentially connecting to a new server)."""
row = table.rows.get(uuid)
changed = False
if not new:
# Delete row.
if row:
del table.rows[uuid]
- return Notice(ROW_DELETE, row)
+ return OVSDB_IDL_UPDATE_DB_CHANGED, Notice(ROW_DELETE, row)
else:
# XXX rate-limit
- vlog.warn("cannot delete missing row %s from table %s"
- % (uuid, table.name))
+ vlog.err("cannot delete missing row %s from table %s"
+ % (uuid, table.name))
+ return OVSDB_IDL_UPDATE_INCONSISTENT, None
elif not old:
# Insert row.
- op = ROW_CREATE
if not row:
row = self.__create_row(table, uuid)
changed = True
else:
# XXX rate-limit
- op = ROW_UPDATE
- vlog.warn("cannot add existing row %s to table %s"
- % (uuid, table.name))
- del table.rows[uuid]
+ vlog.err("cannot add existing row %s to table %s"
+ % (uuid, table.name))
+ return OVSDB_IDL_UPDATE_INCONSISTENT, None
changed |= self.__row_update(table, row, new)
table.rows[uuid] = row
if changed:
- return Notice(ROW_CREATE, row)
+ return OVSDB_IDL_UPDATE_DB_CHANGED, Notice(ROW_CREATE, row)
+ return OVSDB_IDL_UPDATE_NO_CHANGES, None
else:
- op = ROW_UPDATE
+ # Modify row.
if not row:
- row = self.__create_row(table, uuid)
- changed = True
- op = ROW_CREATE
# XXX rate-limit
- vlog.warn("cannot modify missing row %s in table %s"
- % (uuid, table.name))
- else:
- del table.rows[uuid]
+ vlog.err("cannot modify missing row %s in table %s"
+ % (uuid, table.name))
+ return OVSDB_IDL_UPDATE_INCONSISTENT, None
+ del table.rows[uuid]
changed |= self.__row_update(table, row, new)
table.rows[uuid] = row
if changed:
- return Notice(op, row, Row.from_json(self, table, uuid, old))
- return False
+ return (OVSDB_IDL_UPDATE_DB_CHANGED,
+ Notice(ROW_UPDATE, row,
+ Row.from_json(self, table, uuid, old)))
+ return OVSDB_IDL_UPDATE_NO_CHANGES, None
def __check_server_db(self):
"""Returns True if this is a valid server database, False otherwise."""
--
2.49.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
