| Message ID | 20210901161526.237479-1-twilson@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,python] Avoid sending transactions when the DB is not synced up | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
This should backport cleanly down to 2.15. Before that, nothing had added the IDL_S_MONITORING state to python-ovs ( added by https://github.com/openvswitch/ovs/commit/daf627f459ffbc7171d42a2c01f80754bfd54edc ). It would be nice to take this back to 2.13, but if we need to maintain the fix downstream-only, we can do that. Adding just the state is a pretty straightforward couple of lines. For OpenStack/Neutron, ovsdbapp had worked around this problem by waiting for the seqno to change with the first reply, which until https://github.com/openvswitch/ovs/commit/c39751e44539a014e642bcd930cb9e3a33af1805 was the update notification from the monitor request. Now that there are requests for _Server, etc., that no longer works. On Wed, Sep 1, 2021 at 11:15 AM Terry Wilson <twilson@redhat.com> wrote: > > This ports the C IDL change f50714b to the Python IDL: > > Until now the code here would happily try to send transactions to the > database server even if the database connection was not in the correct > state. In some cases this could lead to strange behavior, such as sending > a database transaction for a database that the IDL had just learned did not > exist on the server. > > Signed-off-by: Terry Wilson <twilson@redhat.com> > --- > python/ovs/db/idl.py | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py > index ecae5e143..0be0064de 100644 > --- a/python/ovs/db/idl.py > +++ b/python/ovs/db/idl.py > @@ -1500,6 +1500,12 @@ class Transaction(object): > the IDL's copy of the database. If the transaction commits > successfully, then the database server will send an update and, thus, > the IDL will be updated with the committed changes.""" > + > + if self.idl.state != Idl.IDL_S_MONITORING: > + self._status = Transaction.TRY_AGAIN > + self.__disassemble() > + return self._status > + > # The status can only change if we're the active transaction. > # (Otherwise, our status will change only in Idl.run().) > if self != self.idl.txn: > -- > 2.27.0 >
On 9/1/21 6:15 PM, Terry Wilson wrote: > This ports the C IDL change f50714b to the Python IDL: > > Until now the code here would happily try to send transactions to the > database server even if the database connection was not in the correct > state. In some cases this could lead to strange behavior, such as sending > a database transaction for a database that the IDL had just learned did not > exist on the server. > > Signed-off-by: Terry Wilson <twilson@redhat.com> > --- Hi Terry, > python/ovs/db/idl.py | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py > index ecae5e143..0be0064de 100644 > --- a/python/ovs/db/idl.py > +++ b/python/ovs/db/idl.py > @@ -1500,6 +1500,12 @@ class Transaction(object): > the IDL's copy of the database. If the transaction commits > successfully, then the database server will send an update and, thus, > the IDL will be updated with the committed changes.""" > + > + if self.idl.state != Idl.IDL_S_MONITORING: > + self._status = Transaction.TRY_AGAIN > + self.__disassemble() > + return self._status > + > # The status can only change if we're the active transaction. > # (Otherwise, our status will change only in Idl.run().) > if self != self.idl.txn: > The C IDL actually checks the IDL state here, after it verified that this is the active transaction. I think it makes sense to do the same thing for the Python IDL, that is change the order to: # The status can only change if we're the active transaction. # (Otherwise, our status will change only in Idl.run().) if self != self.idl.txn: return self._status if self.idl.state != Idl.IDL_S_MONITORING: self._status = Transaction.TRY_AGAIN self.__disassemble() return self._status Regards, Dumitru
diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py index ecae5e143..0be0064de 100644 --- a/python/ovs/db/idl.py +++ b/python/ovs/db/idl.py @@ -1500,6 +1500,12 @@ class Transaction(object): the IDL's copy of the database. If the transaction commits successfully, then the database server will send an update and, thus, the IDL will be updated with the committed changes.""" + + if self.idl.state != Idl.IDL_S_MONITORING: + self._status = Transaction.TRY_AGAIN + self.__disassemble() + return self._status + # The status can only change if we're the active transaction. # (Otherwise, our status will change only in Idl.run().) if self != self.idl.txn:
This ports the C IDL change f50714b to the Python IDL: Until now the code here would happily try to send transactions to the database server even if the database connection was not in the correct state. In some cases this could lead to strange behavior, such as sending a database transaction for a database that the IDL had just learned did not exist on the server. Signed-off-by: Terry Wilson <twilson@redhat.com> --- python/ovs/db/idl.py | 6 ++++++ 1 file changed, 6 insertions(+)