| Message ID | 20240415162049.1073623-1-twilson@redhat.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Ilya Maximets |
| Headers | show |
| Series | [ovs-dev,v2] python: ovsdb-idl: Add custom transaction operations. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
On 4/15/24 18:20, Terry Wilson wrote: > It can be useful to be able to send raw transaction operations > through the Idl's connection. For example, to clean up MAC_Binding > entries for floating IPs without having to monitor the MAC_Binding > table which can be quite large. > > Signed-off-by: Terry Wilson <twilson@redhat.com> Thanks, Terry. I wish there were a better way to deal with this issue, but this solution seems fine as well. See some comments below. > --- > NEWS | 2 ++ > python/ovs/db/idl.py | 18 ++++++++++++++- > tests/ovsdb-idl.at | 27 ++++++++++++++++++++++ > tests/test-ovsdb.py | 55 ++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 101 insertions(+), 1 deletion(-) > > diff --git a/NEWS b/NEWS > index b92cec532..f30b859fd 100644 > --- a/NEWS > +++ b/NEWS > @@ -7,6 +7,8 @@ Post-v3.3.0 > - The primary development branch has been renamed from 'master' to 'main'. > The OVS tree remains hosted on GitHub. > https://github.com/openvswitch/ovs.git > + - Python: > + * Add custom transaction support to the Idl via add_op(). > > > v3.3.0 - 16 Feb 2024 > diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py > index a80da84e7..1220e4cc6 100644 > --- a/python/ovs/db/idl.py > +++ b/python/ovs/db/idl.py > @@ -1703,6 +1703,8 @@ class Transaction(object): > > self._inserted_rows = {} # Map from UUID to _InsertedRow > > + self._operations = [] > + > def add_comment(self, comment): > """Appends 'comment' to the comments that will be passed to the OVSDB > server when this transaction is committed. (The comment will be > @@ -1838,7 +1840,7 @@ class Transaction(object): > "rows": [rows]}) > > # Add updates. > - any_updates = False > + any_updates = bool(self._operations) > for row in self._txn_rows.values(): > if row._changes is None: > if row._table.is_root: > @@ -1977,6 +1979,7 @@ class Transaction(object): > if self.dry_run: > operations.append({"op": "abort"}) > > + operations += self._operations I think, we should add them before the abort, so the execution gets to these even with dry run. > if not any_updates: > self._status = Transaction.UNCHANGED > else: > @@ -1991,6 +1994,19 @@ class Transaction(object): > self.__disassemble() > return self._status > > + def add_op(self, op): > + """Add a raw OVSDB operation to the transaction > + > + This can be useful for re-using the existing Idl connection to take > + actions that are difficult or expensive to do with the Idl itself. e.g. > + deleting a bunch of rows on the server that you don't want to store > + in memory. The docs generally shouldn't talk to the reader, but describe the behavior instead. How about 'E.g. bulk deleting rows from the server without downloading them into a local cache.' ? Also, we need to document that these operations are always added to the end of transaction, i.e. after all other operations. > + > + :param op: An "op" for an OVSDB "transact" request (rfc 7047 Sec 5.2) > + :type op: dict > + """ > + self._operations.append(op) > + > def commit_block(self): > """Attempts to commit this transaction, blocking until the commit > either succeeds or fails. Returns the final commit status, which may > diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at > index fb568dd82..487545a13 100644 > --- a/tests/ovsdb-idl.at > +++ b/tests/ovsdb-idl.at > @@ -2758,6 +2758,33 @@ OVSDB_CHECK_IDL_PERS_UUID_INSERT([simple idl, persistent uuid insert], > [['This UUID would duplicate a UUID already present within the table or deleted within the same transaction']]) > > > +OVSDB_CHECK_IDL_PY([simple idl, python, add_op], > + [], > + [['insert 1, insert 2, insert 3, insert 1' \ > + 'add_op {"op": "delete", "table": "simple", "where": [["i", "==", 1]]}' \ > + 'add_op {"op": "insert", "table": "simple", "row": {"i": 2}}, delete 3' \ > + 'insert 2, add_op {"op": "update", "table": "simple", "row": {"i": 1}, "where": [["i", "==", 2]]}' > + ]], > + [[000: empty > +001: commit, status=success > +002: table simple: i=1 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> > +002: table simple: i=1 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<2> > +002: table simple: i=2 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<3> > +002: table simple: i=3 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<4> > +003: commit, status=success > +004: table simple: i=2 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<3> > +004: table simple: i=3 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<4> > +005: commit, status=success > +006: table simple: i=2 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<3> > +006: table simple: i=2 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<5> > +007: commit, status=success > +008: table simple: i=1 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<3> > +008: table simple: i=1 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<5> > +008: table simple: i=1 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<6> > +009: done > +]],[],sort) > + > + > m4_define([OVSDB_CHECK_IDL_CHANGE_AWARE], > [AT_SETUP([simple idl, database change aware, online conversion - $1]) > AT_KEYWORDS([ovsdb server idl db_change_aware conversion $1]) > diff --git a/tests/test-ovsdb.py b/tests/test-ovsdb.py > index 48f8ee2d7..58829f520 100644 > --- a/tests/test-ovsdb.py > +++ b/tests/test-ovsdb.py > @@ -36,6 +36,53 @@ vlog.set_levels_from_string("console:dbg") > vlog.init(None) > > > +OBJ_STR = "_OBJECT_{}_" > + > + > +def substitute_object_text(data, quotechar='"', obj_chars=("{}", "[]")): > + obj_chars = dict(obj_chars) > + in_quote = False > + in_object = [] > + removed_text = [] > + output = "" > + start = end = 0 > + for i, s in enumerate(data): > + if not in_object: > + if not in_quote and s in obj_chars: > + in_object.append(s) > + start = i > + else: > + output += s > + if s == quotechar: > + in_quote = not in_quote > + elif not in_quote: # unquoted object > + if s == in_object[0]: > + in_object.append(s) > + elif s == obj_chars[in_object[0]]: > + in_object.pop() > + if not in_object: > + end = i + 1 > + removed_text.append(data[start:end]) > + output += OBJ_STR.format(len(removed_text) - 1) > + return output, removed_text > + > + > +def recover_object_text_from_list(words, json): > + if not json: > + return words > + results = [] > + i = 0 > + for word in words: > + while json: > + if OBJ_STR.format(i) not in word: > + results.append(word) > + break > + replacement = json.pop(0) > + results.append(word.replace(OBJ_STR.format(i), replacement)) > + i += 1 > + return results > + > + AFAIU, we need to escape the commas, so the commands.split(',') doesn't split the JSON in parts, right? In that case, maybe we can do something simpler, like replacing all the commas with '|' and recovering them later? It's a test-only application so we don't have to handle every single case, we may just assume that tests do not have '|' character. Something like: >>> inside = 0 >>> result = '' >>> for i, s in enumerate(commands): ... if s == '{': ... inside = inside + 1 ... if s == '}': ... inside = inside - 1 ... if s == ',' and inside > 0: ... result = result + '|' ... else: ... result = result + s And then just replace back once we get the words[1:]. What do you think? For me it's much easier to understand what's going on this way. (I just hacked together the code above, it's probably can be written better, e.g. it doesn't need an enumeration, I guess.) Best regards, Ilya Maximets.
diff --git a/NEWS b/NEWS index b92cec532..f30b859fd 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,8 @@ Post-v3.3.0 - The primary development branch has been renamed from 'master' to 'main'. The OVS tree remains hosted on GitHub. https://github.com/openvswitch/ovs.git + - Python: + * Add custom transaction support to the Idl via add_op(). v3.3.0 - 16 Feb 2024 diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py index a80da84e7..1220e4cc6 100644 --- a/python/ovs/db/idl.py +++ b/python/ovs/db/idl.py @@ -1703,6 +1703,8 @@ class Transaction(object): self._inserted_rows = {} # Map from UUID to _InsertedRow + self._operations = [] + def add_comment(self, comment): """Appends 'comment' to the comments that will be passed to the OVSDB server when this transaction is committed. (The comment will be @@ -1838,7 +1840,7 @@ class Transaction(object): "rows": [rows]}) # Add updates. - any_updates = False + any_updates = bool(self._operations) for row in self._txn_rows.values(): if row._changes is None: if row._table.is_root: @@ -1977,6 +1979,7 @@ class Transaction(object): if self.dry_run: operations.append({"op": "abort"}) + operations += self._operations if not any_updates: self._status = Transaction.UNCHANGED else: @@ -1991,6 +1994,19 @@ class Transaction(object): self.__disassemble() return self._status + def add_op(self, op): + """Add a raw OVSDB operation to the transaction + + This can be useful for re-using the existing Idl connection to take + actions that are difficult or expensive to do with the Idl itself. e.g. + deleting a bunch of rows on the server that you don't want to store + in memory. + + :param op: An "op" for an OVSDB "transact" request (rfc 7047 Sec 5.2) + :type op: dict + """ + self._operations.append(op) + def commit_block(self): """Attempts to commit this transaction, blocking until the commit either succeeds or fails. Returns the final commit status, which may diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at index fb568dd82..487545a13 100644 --- a/tests/ovsdb-idl.at +++ b/tests/ovsdb-idl.at @@ -2758,6 +2758,33 @@ OVSDB_CHECK_IDL_PERS_UUID_INSERT([simple idl, persistent uuid insert], [['This UUID would duplicate a UUID already present within the table or deleted within the same transaction']]) +OVSDB_CHECK_IDL_PY([simple idl, python, add_op], + [], + [['insert 1, insert 2, insert 3, insert 1' \ + 'add_op {"op": "delete", "table": "simple", "where": [["i", "==", 1]]}' \ + 'add_op {"op": "insert", "table": "simple", "row": {"i": 2}}, delete 3' \ + 'insert 2, add_op {"op": "update", "table": "simple", "row": {"i": 1}, "where": [["i", "==", 2]]}' + ]], + [[000: empty +001: commit, status=success +002: table simple: i=1 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> +002: table simple: i=1 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<2> +002: table simple: i=2 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<3> +002: table simple: i=3 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<4> +003: commit, status=success +004: table simple: i=2 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<3> +004: table simple: i=3 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<4> +005: commit, status=success +006: table simple: i=2 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<3> +006: table simple: i=2 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<5> +007: commit, status=success +008: table simple: i=1 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<3> +008: table simple: i=1 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<5> +008: table simple: i=1 r=0 b=false s= u=<0> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<6> +009: done +]],[],sort) + + m4_define([OVSDB_CHECK_IDL_CHANGE_AWARE], [AT_SETUP([simple idl, database change aware, online conversion - $1]) AT_KEYWORDS([ovsdb server idl db_change_aware conversion $1]) diff --git a/tests/test-ovsdb.py b/tests/test-ovsdb.py index 48f8ee2d7..58829f520 100644 --- a/tests/test-ovsdb.py +++ b/tests/test-ovsdb.py @@ -36,6 +36,53 @@ vlog.set_levels_from_string("console:dbg") vlog.init(None) +OBJ_STR = "_OBJECT_{}_" + + +def substitute_object_text(data, quotechar='"', obj_chars=("{}", "[]")): + obj_chars = dict(obj_chars) + in_quote = False + in_object = [] + removed_text = [] + output = "" + start = end = 0 + for i, s in enumerate(data): + if not in_object: + if not in_quote and s in obj_chars: + in_object.append(s) + start = i + else: + output += s + if s == quotechar: + in_quote = not in_quote + elif not in_quote: # unquoted object + if s == in_object[0]: + in_object.append(s) + elif s == obj_chars[in_object[0]]: + in_object.pop() + if not in_object: + end = i + 1 + removed_text.append(data[start:end]) + output += OBJ_STR.format(len(removed_text) - 1) + return output, removed_text + + +def recover_object_text_from_list(words, json): + if not json: + return words + results = [] + i = 0 + for word in words: + while json: + if OBJ_STR.format(i) not in word: + results.append(word) + break + replacement = json.pop(0) + results.append(word.replace(OBJ_STR.format(i), replacement)) + i += 1 + return results + + def unbox_json(json): if type(json) is list and len(json) == 1: return json[0] @@ -377,8 +424,10 @@ def idl_set(idl, commands, step): increment = False fetch_cmds = [] events = [] + commands, data = substitute_object_text(commands) for command in commands.split(','): words = command.split() + words = recover_object_text_from_list(words, data) name = words[0] args = words[1:] @@ -437,6 +486,12 @@ def idl_set(idl, commands, step): s = txn.insert(idl.tables["simple"], new_uuid=args[0], persist_uuid=True) s.i = int(args[1]) + elif name == "add_op": + if len(args) != 1: + sys.stderr.write('"add_op" command requires 1 argument\n') + sys.stderr.write(f"args={args}\n") + sys.exit(1) + txn.add_op(ovs.json.from_string(args[0])) elif name == "delete": if len(args) != 1: sys.stderr.write('"delete" command requires 1 argument\n')
It can be useful to be able to send raw transaction operations through the Idl's connection. For example, to clean up MAC_Binding entries for floating IPs without having to monitor the MAC_Binding table which can be quite large. Signed-off-by: Terry Wilson <twilson@redhat.com> --- NEWS | 2 ++ python/ovs/db/idl.py | 18 ++++++++++++++- tests/ovsdb-idl.at | 27 ++++++++++++++++++++++ tests/test-ovsdb.py | 55 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 101 insertions(+), 1 deletion(-)