| Message ID | 20210902153414.304125-1-twilson@redhat.com |
|---|---|
| State | Accepted |
| Delegated to: | Ilya Maximets |
| Headers | show |
| Series | [ovs-dev,v3,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 |
On 9/2/21 5:34 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> > --- > python/ovs/db/idl.py | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py > index ecae5e143..87ee06cde 100644 > --- a/python/ovs/db/idl.py > +++ b/python/ovs/db/idl.py > @@ -1505,6 +1505,11 @@ class Transaction(object): > if self != self.idl.txn: > return self._status > Sorry, I should've probably mentioned this in the previous review, but I missed it. Nit: the comment from f50714bf9a72 ("ovsdb-idl: Avoid sending transactions when the DB is not synced up.") would be nice to have here too: # If we're still connecting or re-connecting, don't bother sending a # transaction. I guess this can be fixed up at apply time so: Acked-by: Dumitru Ceara <dceara@redhat.com> > + if self.idl.state != Idl.IDL_S_MONITORING: > + self._status = Transaction.TRY_AGAIN > + self.__disassemble() > + return self._status > + > # If we need a lock but don't have it, give up quickly. > if self.idl.lock_name and not self.idl.has_lock: > self._status = Transaction.NOT_LOCKED >
On Thu, Sep 2, 2021 at 10:53 AM Dumitru Ceara <dceara@redhat.com> wrote: > > On 9/2/21 5:34 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> > > --- > > python/ovs/db/idl.py | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py > > index ecae5e143..87ee06cde 100644 > > --- a/python/ovs/db/idl.py > > +++ b/python/ovs/db/idl.py > > @@ -1505,6 +1505,11 @@ class Transaction(object): > > if self != self.idl.txn: > > return self._status > > > > Sorry, I should've probably mentioned this in the previous review, but I > missed it. > > Nit: the comment from f50714bf9a72 ("ovsdb-idl: Avoid sending > transactions when the DB is not synced up.") would be nice to have here too: > > # If we're still connecting or re-connecting, don't bother sending a > # transaction. > > I guess this can be fixed up at apply time so: > > Acked-by: Dumitru Ceara <dceara@redhat.com> Just checking to see if we can get this merged? I'm happy to re-spin with the comment, but discussion here and IRC made it sound like it wasn't necessary. > > + if self.idl.state != Idl.IDL_S_MONITORING: > > + self._status = Transaction.TRY_AGAIN > > + self.__disassemble() > > + return self._status > > + > > # If we need a lock but don't have it, give up quickly. > > if self.idl.lock_name and not self.idl.has_lock: > > self._status = Transaction.NOT_LOCKED > > >
On 9/20/21 18:15, Terry Wilson wrote: > On Thu, Sep 2, 2021 at 10:53 AM Dumitru Ceara <dceara@redhat.com> wrote: >> >> On 9/2/21 5:34 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> >>> --- >>> python/ovs/db/idl.py | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py >>> index ecae5e143..87ee06cde 100644 >>> --- a/python/ovs/db/idl.py >>> +++ b/python/ovs/db/idl.py >>> @@ -1505,6 +1505,11 @@ class Transaction(object): >>> if self != self.idl.txn: >>> return self._status >>> >> >> Sorry, I should've probably mentioned this in the previous review, but I >> missed it. >> >> Nit: the comment from f50714bf9a72 ("ovsdb-idl: Avoid sending >> transactions when the DB is not synced up.") would be nice to have here too: >> >> # If we're still connecting or re-connecting, don't bother sending a >> # transaction. >> >> I guess this can be fixed up at apply time so: >> >> Acked-by: Dumitru Ceara <dceara@redhat.com> > > Just checking to see if we can get this merged? I'm happy to re-spin > with the comment, but discussion here and IRC made it sound like it > wasn't necessary. Hi, Terry. I wanted to apply this patch last week, but I wanted to ask if we need to treat that as a bug fix and backport to stable branches or is it just a thing that is nice to have? It's hard to tell what kind of issues this missing check is cousing. Best regards, Ilya Maximets. > >>> + if self.idl.state != Idl.IDL_S_MONITORING: >>> + self._status = Transaction.TRY_AGAIN >>> + self.__disassemble() >>> + return self._status >>> + >>> # If we need a lock but don't have it, give up quickly. >>> if self.idl.lock_name and not self.idl.has_lock: >>> self._status = Transaction.NOT_LOCKED >>> >> > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Mon, Sep 20, 2021 at 12:29 PM Ilya Maximets <i.maximets@ovn.org> wrote: > > On 9/20/21 18:15, Terry Wilson wrote: > > On Thu, Sep 2, 2021 at 10:53 AM Dumitru Ceara <dceara@redhat.com> wrote: > >> > >> On 9/2/21 5:34 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> > >>> --- > >>> python/ovs/db/idl.py | 5 +++++ > >>> 1 file changed, 5 insertions(+) > >>> > >>> diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py > >>> index ecae5e143..87ee06cde 100644 > >>> --- a/python/ovs/db/idl.py > >>> +++ b/python/ovs/db/idl.py > >>> @@ -1505,6 +1505,11 @@ class Transaction(object): > >>> if self != self.idl.txn: > >>> return self._status > >>> > >> > >> Sorry, I should've probably mentioned this in the previous review, but I > >> missed it. > >> > >> Nit: the comment from f50714bf9a72 ("ovsdb-idl: Avoid sending > >> transactions when the DB is not synced up.") would be nice to have here too: > >> > >> # If we're still connecting or re-connecting, don't bother sending a > >> # transaction. > >> > >> I guess this can be fixed up at apply time so: > >> > >> Acked-by: Dumitru Ceara <dceara@redhat.com> > > > > Just checking to see if we can get this merged? I'm happy to re-spin > > with the comment, but discussion here and IRC made it sound like it > > wasn't necessary. > > Hi, Terry. I wanted to apply this patch last week, but I wanted to ask > if we need to treat that as a bug fix and backport to stable branches > or is it just a thing that is nice to have? It's hard to tell what kind > of issues this missing check is cousing. > > Best regards, Ilya Maximets. It's a bugfix that needs backports. What can happen is that if neutron starts up and makes a transaction quickly after connecting, it can create objects before it has gotten the monitor requests set up. If you successfully insert an object, then call get_insert_uuid() on it, you get the UUID of the transaction but cannot actually access the row in Idl.tables because you aren't getting monitor updates yet. I'm a little worried about a more general case with get_insert_uuid() in that it returns the UUID from the insert txn response, which if it arrives before the monitor update notification, would still throw a KeyError if you tried to access idl.tables[...][uuid]. The way the IDL works (at least the python version), is that after the first call to commit() where we generate/send the transaction message, we clear all of the column data from the Row and don't get it back until we get the monitor update. So it puts us in the weird position of being able to call get_insert_uuid() immediately after a successful commit(), but not be sure that we can actually access the Row object by that UUID or have the column info that we wrote. That is, unless we are guaranteed to always get monitor updates before transaction insert responses--which it seems that we at least *mostly* do, but I'm just not sure if it is guaranteed or a happy accident. In any case, this was a case that was easy to solve and was already solved in the C IDL long ago and needs backports. Thanks! Terry > > > >>> + if self.idl.state != Idl.IDL_S_MONITORING: > >>> + self._status = Transaction.TRY_AGAIN > >>> + self.__disassemble() > >>> + return self._status > >>> + > >>> # If we need a lock but don't have it, give up quickly. > >>> if self.idl.lock_name and not self.idl.has_lock: > >>> self._status = Transaction.NOT_LOCKED > >>> > >> > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > >
On 9/20/21 19:58, Terry Wilson wrote: > On Mon, Sep 20, 2021 at 12:29 PM Ilya Maximets <i.maximets@ovn.org> wrote: >> >> On 9/20/21 18:15, Terry Wilson wrote: >>> On Thu, Sep 2, 2021 at 10:53 AM Dumitru Ceara <dceara@redhat.com> wrote: >>>> >>>> On 9/2/21 5:34 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> >>>>> --- >>>>> python/ovs/db/idl.py | 5 +++++ >>>>> 1 file changed, 5 insertions(+) >>>>> >>>>> diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py >>>>> index ecae5e143..87ee06cde 100644 >>>>> --- a/python/ovs/db/idl.py >>>>> +++ b/python/ovs/db/idl.py >>>>> @@ -1505,6 +1505,11 @@ class Transaction(object): >>>>> if self != self.idl.txn: >>>>> return self._status >>>>> >>>> >>>> Sorry, I should've probably mentioned this in the previous review, but I >>>> missed it. >>>> >>>> Nit: the comment from f50714bf9a72 ("ovsdb-idl: Avoid sending >>>> transactions when the DB is not synced up.") would be nice to have here too: >>>> >>>> # If we're still connecting or re-connecting, don't bother sending a >>>> # transaction. >>>> >>>> I guess this can be fixed up at apply time so: >>>> >>>> Acked-by: Dumitru Ceara <dceara@redhat.com> >>> >>> Just checking to see if we can get this merged? I'm happy to re-spin >>> with the comment, but discussion here and IRC made it sound like it >>> wasn't necessary. >> >> Hi, Terry. I wanted to apply this patch last week, but I wanted to ask >> if we need to treat that as a bug fix and backport to stable branches >> or is it just a thing that is nice to have? It's hard to tell what kind >> of issues this missing check is cousing. >> >> Best regards, Ilya Maximets. > > It's a bugfix that needs backports. What can happen is that if neutron > starts up and makes a transaction quickly after connecting, it can > create objects before it has gotten the monitor requests set up. Shouldn't it check for idl.has_ever_connected() before sending transactions? has_ever_connected() should be true only after successful monitor request. > If > you successfully insert an object, then call get_insert_uuid() on it, > you get the UUID of the transaction but cannot actually access the row > in Idl.tables because you aren't getting monitor updates yet. > > I'm a little worried about a more general case with get_insert_uuid() > in that it returns the UUID from the insert txn response, which if it > arrives before the monitor update notification, would still throw a > KeyError if you tried to access idl.tables[...][uuid]. The way the IDL > works (at least the python version), is that after the first call to > commit() where we generate/send the transaction message, we clear all > of the column data from the Row and don't get it back until we get the > monitor update. So it puts us in the weird position of being able to > call get_insert_uuid() immediately after a successful commit(), but > not be sure that we can actually access the Row object by that UUID or > have the column info that we wrote. That is, unless we are guaranteed > to always get monitor updates before transaction insert > responses--which it seems that we at least *mostly* do, but I'm just > not sure if it is guaranteed or a happy accident. > > In any case, this was a case that was easy to solve and was already > solved in the C IDL long ago and needs backports. Thanks! > > Terry > >>> >>>>> + if self.idl.state != Idl.IDL_S_MONITORING: >>>>> + self._status = Transaction.TRY_AGAIN >>>>> + self.__disassemble() >>>>> + return self._status >>>>> + >>>>> # If we need a lock but don't have it, give up quickly. >>>>> if self.idl.lock_name and not self.idl.has_lock: >>>>> self._status = Transaction.NOT_LOCKED >>>>> >>>> >>> >>> _______________________________________________ >>> dev mailing list >>> dev@openvswitch.org >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>> >> >
On 9/20/21 19:58, Terry Wilson wrote: > On Mon, Sep 20, 2021 at 12:29 PM Ilya Maximets <i.maximets@ovn.org> wrote: >> >> On 9/20/21 18:15, Terry Wilson wrote: >>> On Thu, Sep 2, 2021 at 10:53 AM Dumitru Ceara <dceara@redhat.com> wrote: >>>> >>>> On 9/2/21 5:34 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> >>>>> --- >>>>> python/ovs/db/idl.py | 5 +++++ >>>>> 1 file changed, 5 insertions(+) >>>>> >>>>> diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py >>>>> index ecae5e143..87ee06cde 100644 >>>>> --- a/python/ovs/db/idl.py >>>>> +++ b/python/ovs/db/idl.py >>>>> @@ -1505,6 +1505,11 @@ class Transaction(object): >>>>> if self != self.idl.txn: >>>>> return self._status >>>>> >>>> >>>> Sorry, I should've probably mentioned this in the previous review, but I >>>> missed it. >>>> >>>> Nit: the comment from f50714bf9a72 ("ovsdb-idl: Avoid sending >>>> transactions when the DB is not synced up.") would be nice to have here too: >>>> >>>> # If we're still connecting or re-connecting, don't bother sending a >>>> # transaction. >>>> >>>> I guess this can be fixed up at apply time so: >>>> >>>> Acked-by: Dumitru Ceara <dceara@redhat.com> >>> >>> Just checking to see if we can get this merged? I'm happy to re-spin >>> with the comment, but discussion here and IRC made it sound like it >>> wasn't necessary. >> >> Hi, Terry. I wanted to apply this patch last week, but I wanted to ask >> if we need to treat that as a bug fix and backport to stable branches >> or is it just a thing that is nice to have? It's hard to tell what kind >> of issues this missing check is cousing. >> >> Best regards, Ilya Maximets. > > It's a bugfix that needs backports. What can happen is that if neutron > starts up and makes a transaction quickly after connecting, it can > create objects before it has gotten the monitor requests set up. If > you successfully insert an object, then call get_insert_uuid() on it, > you get the UUID of the transaction but cannot actually access the row > in Idl.tables because you aren't getting monitor updates yet. > > I'm a little worried about a more general case with get_insert_uuid() > in that it returns the UUID from the insert txn response, which if it > arrives before the monitor update notification, would still throw a > KeyError if you tried to access idl.tables[...][uuid]. The way the IDL > works (at least the python version), is that after the first call to > commit() where we generate/send the transaction message, we clear all > of the column data from the Row and don't get it back until we get the > monitor update. So it puts us in the weird position of being able to > call get_insert_uuid() immediately after a successful commit(), but > not be sure that we can actually access the Row object by that UUID or > have the column info that we wrote. That is, unless we are guaranteed > to always get monitor updates before transaction insert > responses--which it seems that we at least *mostly* do, but I'm just > not sure if it is guaranteed or a happy accident. It's guaranteed that update always arrives before the transaction reply. If that is not happening, that should be a bug on the ovsdb-server side or monitor got cancelled or something like this. > > In any case, this was a case that was easy to solve and was already > solved in the C IDL long ago and needs backports. Thanks! > > Terry > >>> >>>>> + if self.idl.state != Idl.IDL_S_MONITORING: >>>>> + self._status = Transaction.TRY_AGAIN >>>>> + self.__disassemble() >>>>> + return self._status >>>>> + >>>>> # If we need a lock but don't have it, give up quickly. >>>>> if self.idl.lock_name and not self.idl.has_lock: >>>>> self._status = Transaction.NOT_LOCKED >>>>> >>>> >>> >>> _______________________________________________ >>> dev mailing list >>> dev@openvswitch.org >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>> >> >
The problem is that has_ever_connected() doesn't imply that we have downloaded the db into memory since the _Server stuff was added. On Mon, Sep 20, 2021 at 1:05 PM Ilya Maximets <i.maximets@ovn.org> wrote: > > On 9/20/21 19:58, Terry Wilson wrote: > > On Mon, Sep 20, 2021 at 12:29 PM Ilya Maximets <i.maximets@ovn.org> wrote: > >> > >> On 9/20/21 18:15, Terry Wilson wrote: > >>> On Thu, Sep 2, 2021 at 10:53 AM Dumitru Ceara <dceara@redhat.com> wrote: > >>>> > >>>> On 9/2/21 5:34 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> > >>>>> --- > >>>>> python/ovs/db/idl.py | 5 +++++ > >>>>> 1 file changed, 5 insertions(+) > >>>>> > >>>>> diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py > >>>>> index ecae5e143..87ee06cde 100644 > >>>>> --- a/python/ovs/db/idl.py > >>>>> +++ b/python/ovs/db/idl.py > >>>>> @@ -1505,6 +1505,11 @@ class Transaction(object): > >>>>> if self != self.idl.txn: > >>>>> return self._status > >>>>> > >>>> > >>>> Sorry, I should've probably mentioned this in the previous review, but I > >>>> missed it. > >>>> > >>>> Nit: the comment from f50714bf9a72 ("ovsdb-idl: Avoid sending > >>>> transactions when the DB is not synced up.") would be nice to have here too: > >>>> > >>>> # If we're still connecting or re-connecting, don't bother sending a > >>>> # transaction. > >>>> > >>>> I guess this can be fixed up at apply time so: > >>>> > >>>> Acked-by: Dumitru Ceara <dceara@redhat.com> > >>> > >>> Just checking to see if we can get this merged? I'm happy to re-spin > >>> with the comment, but discussion here and IRC made it sound like it > >>> wasn't necessary. > >> > >> Hi, Terry. I wanted to apply this patch last week, but I wanted to ask > >> if we need to treat that as a bug fix and backport to stable branches > >> or is it just a thing that is nice to have? It's hard to tell what kind > >> of issues this missing check is cousing. > >> > >> Best regards, Ilya Maximets. > > > > It's a bugfix that needs backports. What can happen is that if neutron > > starts up and makes a transaction quickly after connecting, it can > > create objects before it has gotten the monitor requests set up. > > Shouldn't it check for idl.has_ever_connected() before sending transactions? > has_ever_connected() should be true only after successful monitor request. > > > If > > you successfully insert an object, then call get_insert_uuid() on it, > > you get the UUID of the transaction but cannot actually access the row > > in Idl.tables because you aren't getting monitor updates yet. > > > > I'm a little worried about a more general case with get_insert_uuid() > > in that it returns the UUID from the insert txn response, which if it > > arrives before the monitor update notification, would still throw a > > KeyError if you tried to access idl.tables[...][uuid]. The way the IDL > > works (at least the python version), is that after the first call to > > commit() where we generate/send the transaction message, we clear all > > of the column data from the Row and don't get it back until we get the > > monitor update. So it puts us in the weird position of being able to > > call get_insert_uuid() immediately after a successful commit(), but > > not be sure that we can actually access the Row object by that UUID or > > have the column info that we wrote. That is, unless we are guaranteed > > to always get monitor updates before transaction insert > > responses--which it seems that we at least *mostly* do, but I'm just > > not sure if it is guaranteed or a happy accident. > > > > In any case, this was a case that was easy to solve and was already > > solved in the C IDL long ago and needs backports. Thanks! > > > > Terry > > > >>> > >>>>> + if self.idl.state != Idl.IDL_S_MONITORING: > >>>>> + self._status = Transaction.TRY_AGAIN > >>>>> + self.__disassemble() > >>>>> + return self._status > >>>>> + > >>>>> # If we need a lock but don't have it, give up quickly. > >>>>> if self.idl.lock_name and not self.idl.has_lock: > >>>>> self._status = Transaction.NOT_LOCKED > >>>>> > >>>> > >>> > >>> _______________________________________________ > >>> dev mailing list > >>> dev@openvswitch.org > >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >>> > >> > > >
On 9/20/21 20:10, Terry Wilson wrote: > The problem is that has_ever_connected() doesn't imply that we have > downloaded the db into memory since the _Server stuff was added. Hmm. That does sound like a bug to me. has_ever_connected() should reflect only changes in the actual database, not the '_Server' one. This patch itself seems OK. But the root cause of neutron issues seems to be that has_ever_connected() is broken. > > On Mon, Sep 20, 2021 at 1:05 PM Ilya Maximets <i.maximets@ovn.org> wrote: >> >> On 9/20/21 19:58, Terry Wilson wrote: >>> On Mon, Sep 20, 2021 at 12:29 PM Ilya Maximets <i.maximets@ovn.org> wrote: >>>> >>>> On 9/20/21 18:15, Terry Wilson wrote: >>>>> On Thu, Sep 2, 2021 at 10:53 AM Dumitru Ceara <dceara@redhat.com> wrote: >>>>>> >>>>>> On 9/2/21 5:34 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> >>>>>>> --- >>>>>>> python/ovs/db/idl.py | 5 +++++ >>>>>>> 1 file changed, 5 insertions(+) >>>>>>> >>>>>>> diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py >>>>>>> index ecae5e143..87ee06cde 100644 >>>>>>> --- a/python/ovs/db/idl.py >>>>>>> +++ b/python/ovs/db/idl.py >>>>>>> @@ -1505,6 +1505,11 @@ class Transaction(object): >>>>>>> if self != self.idl.txn: >>>>>>> return self._status >>>>>>> >>>>>> >>>>>> Sorry, I should've probably mentioned this in the previous review, but I >>>>>> missed it. >>>>>> >>>>>> Nit: the comment from f50714bf9a72 ("ovsdb-idl: Avoid sending >>>>>> transactions when the DB is not synced up.") would be nice to have here too: >>>>>> >>>>>> # If we're still connecting or re-connecting, don't bother sending a >>>>>> # transaction. >>>>>> >>>>>> I guess this can be fixed up at apply time so: >>>>>> >>>>>> Acked-by: Dumitru Ceara <dceara@redhat.com> >>>>> >>>>> Just checking to see if we can get this merged? I'm happy to re-spin >>>>> with the comment, but discussion here and IRC made it sound like it >>>>> wasn't necessary. >>>> >>>> Hi, Terry. I wanted to apply this patch last week, but I wanted to ask >>>> if we need to treat that as a bug fix and backport to stable branches >>>> or is it just a thing that is nice to have? It's hard to tell what kind >>>> of issues this missing check is cousing. >>>> >>>> Best regards, Ilya Maximets. >>> >>> It's a bugfix that needs backports. What can happen is that if neutron >>> starts up and makes a transaction quickly after connecting, it can >>> create objects before it has gotten the monitor requests set up. >> >> Shouldn't it check for idl.has_ever_connected() before sending transactions? >> has_ever_connected() should be true only after successful monitor request. >> >>> If >>> you successfully insert an object, then call get_insert_uuid() on it, >>> you get the UUID of the transaction but cannot actually access the row >>> in Idl.tables because you aren't getting monitor updates yet. >>> >>> I'm a little worried about a more general case with get_insert_uuid() >>> in that it returns the UUID from the insert txn response, which if it >>> arrives before the monitor update notification, would still throw a >>> KeyError if you tried to access idl.tables[...][uuid]. The way the IDL >>> works (at least the python version), is that after the first call to >>> commit() where we generate/send the transaction message, we clear all >>> of the column data from the Row and don't get it back until we get the >>> monitor update. So it puts us in the weird position of being able to >>> call get_insert_uuid() immediately after a successful commit(), but >>> not be sure that we can actually access the Row object by that UUID or >>> have the column info that we wrote. That is, unless we are guaranteed >>> to always get monitor updates before transaction insert >>> responses--which it seems that we at least *mostly* do, but I'm just >>> not sure if it is guaranteed or a happy accident. >>> >>> In any case, this was a case that was easy to solve and was already >>> solved in the C IDL long ago and needs backports. Thanks! >>> >>> Terry >>> >>>>> >>>>>>> + if self.idl.state != Idl.IDL_S_MONITORING: >>>>>>> + self._status = Transaction.TRY_AGAIN >>>>>>> + self.__disassemble() >>>>>>> + return self._status >>>>>>> + >>>>>>> # If we need a lock but don't have it, give up quickly. >>>>>>> if self.idl.lock_name and not self.idl.has_lock: >>>>>>> self._status = Transaction.NOT_LOCKED >>>>>>> >>>>>> >>>>> >>>>> _______________________________________________ >>>>> dev mailing list >>>>> dev@openvswitch.org >>>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>>>> >>>> >>> >> >
On Mon, Sep 20, 2021 at 1:18 PM Ilya Maximets <i.maximets@ovn.org> wrote: > > On 9/20/21 20:10, Terry Wilson wrote: > > The problem is that has_ever_connected() doesn't imply that we have > > downloaded the db into memory since the _Server stuff was added. > > Hmm. That does sound like a bug to me. has_ever_connected() should > reflect only changes in the actual database, not the '_Server' one. > > This patch itself seems OK. But the root cause of neutron issues > seems to be that has_ever_connected() is broken. I can investigate has_ever_connected() as a separate patch, but since this patch just mirrors the behavior already in the C IDL code, can we go ahead and merge it? > > > > On Mon, Sep 20, 2021 at 1:05 PM Ilya Maximets <i.maximets@ovn.org> wrote: > >> > >> On 9/20/21 19:58, Terry Wilson wrote: > >>> On Mon, Sep 20, 2021 at 12:29 PM Ilya Maximets <i.maximets@ovn.org> wrote: > >>>> > >>>> On 9/20/21 18:15, Terry Wilson wrote: > >>>>> On Thu, Sep 2, 2021 at 10:53 AM Dumitru Ceara <dceara@redhat.com> wrote: > >>>>>> > >>>>>> On 9/2/21 5:34 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> > >>>>>>> --- > >>>>>>> python/ovs/db/idl.py | 5 +++++ > >>>>>>> 1 file changed, 5 insertions(+) > >>>>>>> > >>>>>>> diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py > >>>>>>> index ecae5e143..87ee06cde 100644 > >>>>>>> --- a/python/ovs/db/idl.py > >>>>>>> +++ b/python/ovs/db/idl.py > >>>>>>> @@ -1505,6 +1505,11 @@ class Transaction(object): > >>>>>>> if self != self.idl.txn: > >>>>>>> return self._status > >>>>>>> > >>>>>> > >>>>>> Sorry, I should've probably mentioned this in the previous review, but I > >>>>>> missed it. > >>>>>> > >>>>>> Nit: the comment from f50714bf9a72 ("ovsdb-idl: Avoid sending > >>>>>> transactions when the DB is not synced up.") would be nice to have here too: > >>>>>> > >>>>>> # If we're still connecting or re-connecting, don't bother sending a > >>>>>> # transaction. > >>>>>> > >>>>>> I guess this can be fixed up at apply time so: > >>>>>> > >>>>>> Acked-by: Dumitru Ceara <dceara@redhat.com> > >>>>> > >>>>> Just checking to see if we can get this merged? I'm happy to re-spin > >>>>> with the comment, but discussion here and IRC made it sound like it > >>>>> wasn't necessary. > >>>> > >>>> Hi, Terry. I wanted to apply this patch last week, but I wanted to ask > >>>> if we need to treat that as a bug fix and backport to stable branches > >>>> or is it just a thing that is nice to have? It's hard to tell what kind > >>>> of issues this missing check is cousing. > >>>> > >>>> Best regards, Ilya Maximets. > >>> > >>> It's a bugfix that needs backports. What can happen is that if neutron > >>> starts up and makes a transaction quickly after connecting, it can > >>> create objects before it has gotten the monitor requests set up. > >> > >> Shouldn't it check for idl.has_ever_connected() before sending transactions? > >> has_ever_connected() should be true only after successful monitor request. > >> > >>> If > >>> you successfully insert an object, then call get_insert_uuid() on it, > >>> you get the UUID of the transaction but cannot actually access the row > >>> in Idl.tables because you aren't getting monitor updates yet. > >>> > >>> I'm a little worried about a more general case with get_insert_uuid() > >>> in that it returns the UUID from the insert txn response, which if it > >>> arrives before the monitor update notification, would still throw a > >>> KeyError if you tried to access idl.tables[...][uuid]. The way the IDL > >>> works (at least the python version), is that after the first call to > >>> commit() where we generate/send the transaction message, we clear all > >>> of the column data from the Row and don't get it back until we get the > >>> monitor update. So it puts us in the weird position of being able to > >>> call get_insert_uuid() immediately after a successful commit(), but > >>> not be sure that we can actually access the Row object by that UUID or > >>> have the column info that we wrote. That is, unless we are guaranteed > >>> to always get monitor updates before transaction insert > >>> responses--which it seems that we at least *mostly* do, but I'm just > >>> not sure if it is guaranteed or a happy accident. > >>> > >>> In any case, this was a case that was easy to solve and was already > >>> solved in the C IDL long ago and needs backports. Thanks! > >>> > >>> Terry > >>> > >>>>> > >>>>>>> + if self.idl.state != Idl.IDL_S_MONITORING: > >>>>>>> + self._status = Transaction.TRY_AGAIN > >>>>>>> + self.__disassemble() > >>>>>>> + return self._status > >>>>>>> + > >>>>>>> # If we need a lock but don't have it, give up quickly. > >>>>>>> if self.idl.lock_name and not self.idl.has_lock: > >>>>>>> self._status = Transaction.NOT_LOCKED > >>>>>>> > >>>>>> > >>>>> > >>>>> _______________________________________________ > >>>>> dev mailing list > >>>>> dev@openvswitch.org > >>>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >>>>> > >>>> > >>> > >> > > >
On 10/12/21 15:50, Terry Wilson wrote: > On Mon, Sep 20, 2021 at 1:18 PM Ilya Maximets <i.maximets@ovn.org> wrote: >> >> On 9/20/21 20:10, Terry Wilson wrote: >>> The problem is that has_ever_connected() doesn't imply that we have >>> downloaded the db into memory since the _Server stuff was added. >> >> Hmm. That does sound like a bug to me. has_ever_connected() should >> reflect only changes in the actual database, not the '_Server' one. >> >> This patch itself seems OK. But the root cause of neutron issues >> seems to be that has_ever_connected() is broken. > > I can investigate has_ever_connected() as a separate patch, but since > this patch just mirrors the behavior already in the C IDL code, can we > go ahead and merge it? Yes, sure. I'm looking at it right now. Will push as soon as CI will pass. Will also backport down to 2.13. Sorry for delay, I just returned from my PTO. > >>> >>> On Mon, Sep 20, 2021 at 1:05 PM Ilya Maximets <i.maximets@ovn.org> wrote: >>>> >>>> On 9/20/21 19:58, Terry Wilson wrote: >>>>> On Mon, Sep 20, 2021 at 12:29 PM Ilya Maximets <i.maximets@ovn.org> wrote: >>>>>> >>>>>> On 9/20/21 18:15, Terry Wilson wrote: >>>>>>> On Thu, Sep 2, 2021 at 10:53 AM Dumitru Ceara <dceara@redhat.com> wrote: >>>>>>>> >>>>>>>> On 9/2/21 5:34 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> >>>>>>>>> --- >>>>>>>>> python/ovs/db/idl.py | 5 +++++ >>>>>>>>> 1 file changed, 5 insertions(+) >>>>>>>>> >>>>>>>>> diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py >>>>>>>>> index ecae5e143..87ee06cde 100644 >>>>>>>>> --- a/python/ovs/db/idl.py >>>>>>>>> +++ b/python/ovs/db/idl.py >>>>>>>>> @@ -1505,6 +1505,11 @@ class Transaction(object): >>>>>>>>> if self != self.idl.txn: >>>>>>>>> return self._status >>>>>>>>> >>>>>>>> >>>>>>>> Sorry, I should've probably mentioned this in the previous review, but I >>>>>>>> missed it. >>>>>>>> >>>>>>>> Nit: the comment from f50714bf9a72 ("ovsdb-idl: Avoid sending >>>>>>>> transactions when the DB is not synced up.") would be nice to have here too: >>>>>>>> >>>>>>>> # If we're still connecting or re-connecting, don't bother sending a >>>>>>>> # transaction. >>>>>>>> >>>>>>>> I guess this can be fixed up at apply time so: >>>>>>>> >>>>>>>> Acked-by: Dumitru Ceara <dceara@redhat.com> >>>>>>> >>>>>>> Just checking to see if we can get this merged? I'm happy to re-spin >>>>>>> with the comment, but discussion here and IRC made it sound like it >>>>>>> wasn't necessary. >>>>>> >>>>>> Hi, Terry. I wanted to apply this patch last week, but I wanted to ask >>>>>> if we need to treat that as a bug fix and backport to stable branches >>>>>> or is it just a thing that is nice to have? It's hard to tell what kind >>>>>> of issues this missing check is cousing. >>>>>> >>>>>> Best regards, Ilya Maximets. >>>>> >>>>> It's a bugfix that needs backports. What can happen is that if neutron >>>>> starts up and makes a transaction quickly after connecting, it can >>>>> create objects before it has gotten the monitor requests set up. >>>> >>>> Shouldn't it check for idl.has_ever_connected() before sending transactions? >>>> has_ever_connected() should be true only after successful monitor request. >>>> >>>>> If >>>>> you successfully insert an object, then call get_insert_uuid() on it, >>>>> you get the UUID of the transaction but cannot actually access the row >>>>> in Idl.tables because you aren't getting monitor updates yet. >>>>> >>>>> I'm a little worried about a more general case with get_insert_uuid() >>>>> in that it returns the UUID from the insert txn response, which if it >>>>> arrives before the monitor update notification, would still throw a >>>>> KeyError if you tried to access idl.tables[...][uuid]. The way the IDL >>>>> works (at least the python version), is that after the first call to >>>>> commit() where we generate/send the transaction message, we clear all >>>>> of the column data from the Row and don't get it back until we get the >>>>> monitor update. So it puts us in the weird position of being able to >>>>> call get_insert_uuid() immediately after a successful commit(), but >>>>> not be sure that we can actually access the Row object by that UUID or >>>>> have the column info that we wrote. That is, unless we are guaranteed >>>>> to always get monitor updates before transaction insert >>>>> responses--which it seems that we at least *mostly* do, but I'm just >>>>> not sure if it is guaranteed or a happy accident. >>>>> >>>>> In any case, this was a case that was easy to solve and was already >>>>> solved in the C IDL long ago and needs backports. Thanks! >>>>> >>>>> Terry >>>>> >>>>>>> >>>>>>>>> + if self.idl.state != Idl.IDL_S_MONITORING: >>>>>>>>> + self._status = Transaction.TRY_AGAIN >>>>>>>>> + self.__disassemble() >>>>>>>>> + return self._status >>>>>>>>> + >>>>>>>>> # If we need a lock but don't have it, give up quickly. >>>>>>>>> if self.idl.lock_name and not self.idl.has_lock: >>>>>>>>> self._status = Transaction.NOT_LOCKED >>>>>>>>> >>>>>>>> >>>>>>> >>>>>>> _______________________________________________ >>>>>>> dev mailing list >>>>>>> dev@openvswitch.org >>>>>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>>>>>> >>>>>> >>>>> >>>> >>> >> >
On 10/12/21 15:56, Ilya Maximets wrote: > On 10/12/21 15:50, Terry Wilson wrote: >> On Mon, Sep 20, 2021 at 1:18 PM Ilya Maximets <i.maximets@ovn.org> wrote: >>> >>> On 9/20/21 20:10, Terry Wilson wrote: >>>> The problem is that has_ever_connected() doesn't imply that we have >>>> downloaded the db into memory since the _Server stuff was added. >>> >>> Hmm. That does sound like a bug to me. has_ever_connected() should >>> reflect only changes in the actual database, not the '_Server' one. >>> >>> This patch itself seems OK. But the root cause of neutron issues >>> seems to be that has_ever_connected() is broken. >> >> I can investigate has_ever_connected() as a separate patch, but since >> this patch just mirrors the behavior already in the C IDL code, can we >> go ahead and merge it? > > Yes, sure. I'm looking at it right now. Will push as soon as CI will pass. > Will also backport down to 2.13. Hmm. It doesn't work on 2.14 and 2.13 since there were some code changes. I'll backport down to 2.15. If you think this needs to be backported further, please, submit a new patch for older branches. Best regards, Ilya Maximets. > > Sorry for delay, I just returned from my PTO. > >> >>>> >>>> On Mon, Sep 20, 2021 at 1:05 PM Ilya Maximets <i.maximets@ovn.org> wrote: >>>>> >>>>> On 9/20/21 19:58, Terry Wilson wrote: >>>>>> On Mon, Sep 20, 2021 at 12:29 PM Ilya Maximets <i.maximets@ovn.org> wrote: >>>>>>> >>>>>>> On 9/20/21 18:15, Terry Wilson wrote: >>>>>>>> On Thu, Sep 2, 2021 at 10:53 AM Dumitru Ceara <dceara@redhat.com> wrote: >>>>>>>>> >>>>>>>>> On 9/2/21 5:34 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> >>>>>>>>>> --- >>>>>>>>>> python/ovs/db/idl.py | 5 +++++ >>>>>>>>>> 1 file changed, 5 insertions(+) >>>>>>>>>> >>>>>>>>>> diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py >>>>>>>>>> index ecae5e143..87ee06cde 100644 >>>>>>>>>> --- a/python/ovs/db/idl.py >>>>>>>>>> +++ b/python/ovs/db/idl.py >>>>>>>>>> @@ -1505,6 +1505,11 @@ class Transaction(object): >>>>>>>>>> if self != self.idl.txn: >>>>>>>>>> return self._status >>>>>>>>>> >>>>>>>>> >>>>>>>>> Sorry, I should've probably mentioned this in the previous review, but I >>>>>>>>> missed it. >>>>>>>>> >>>>>>>>> Nit: the comment from f50714bf9a72 ("ovsdb-idl: Avoid sending >>>>>>>>> transactions when the DB is not synced up.") would be nice to have here too: >>>>>>>>> >>>>>>>>> # If we're still connecting or re-connecting, don't bother sending a >>>>>>>>> # transaction. >>>>>>>>> >>>>>>>>> I guess this can be fixed up at apply time so: >>>>>>>>> >>>>>>>>> Acked-by: Dumitru Ceara <dceara@redhat.com> >>>>>>>> >>>>>>>> Just checking to see if we can get this merged? I'm happy to re-spin >>>>>>>> with the comment, but discussion here and IRC made it sound like it >>>>>>>> wasn't necessary. >>>>>>> >>>>>>> Hi, Terry. I wanted to apply this patch last week, but I wanted to ask >>>>>>> if we need to treat that as a bug fix and backport to stable branches >>>>>>> or is it just a thing that is nice to have? It's hard to tell what kind >>>>>>> of issues this missing check is cousing. >>>>>>> >>>>>>> Best regards, Ilya Maximets. >>>>>> >>>>>> It's a bugfix that needs backports. What can happen is that if neutron >>>>>> starts up and makes a transaction quickly after connecting, it can >>>>>> create objects before it has gotten the monitor requests set up. >>>>> >>>>> Shouldn't it check for idl.has_ever_connected() before sending transactions? >>>>> has_ever_connected() should be true only after successful monitor request. >>>>> >>>>>> If >>>>>> you successfully insert an object, then call get_insert_uuid() on it, >>>>>> you get the UUID of the transaction but cannot actually access the row >>>>>> in Idl.tables because you aren't getting monitor updates yet. >>>>>> >>>>>> I'm a little worried about a more general case with get_insert_uuid() >>>>>> in that it returns the UUID from the insert txn response, which if it >>>>>> arrives before the monitor update notification, would still throw a >>>>>> KeyError if you tried to access idl.tables[...][uuid]. The way the IDL >>>>>> works (at least the python version), is that after the first call to >>>>>> commit() where we generate/send the transaction message, we clear all >>>>>> of the column data from the Row and don't get it back until we get the >>>>>> monitor update. So it puts us in the weird position of being able to >>>>>> call get_insert_uuid() immediately after a successful commit(), but >>>>>> not be sure that we can actually access the Row object by that UUID or >>>>>> have the column info that we wrote. That is, unless we are guaranteed >>>>>> to always get monitor updates before transaction insert >>>>>> responses--which it seems that we at least *mostly* do, but I'm just >>>>>> not sure if it is guaranteed or a happy accident. >>>>>> >>>>>> In any case, this was a case that was easy to solve and was already >>>>>> solved in the C IDL long ago and needs backports. Thanks! >>>>>> >>>>>> Terry >>>>>> >>>>>>>> >>>>>>>>>> + if self.idl.state != Idl.IDL_S_MONITORING: >>>>>>>>>> + self._status = Transaction.TRY_AGAIN >>>>>>>>>> + self.__disassemble() >>>>>>>>>> + return self._status >>>>>>>>>> + >>>>>>>>>> # If we need a lock but don't have it, give up quickly. >>>>>>>>>> if self.idl.lock_name and not self.idl.has_lock: >>>>>>>>>> self._status = Transaction.NOT_LOCKED >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>>> _______________________________________________ >>>>>>>> dev mailing list >>>>>>>> dev@openvswitch.org >>>>>>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >
On 9/2/21 17:53, Dumitru Ceara wrote: > On 9/2/21 5:34 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> >> --- >> python/ovs/db/idl.py | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py >> index ecae5e143..87ee06cde 100644 >> --- a/python/ovs/db/idl.py >> +++ b/python/ovs/db/idl.py >> @@ -1505,6 +1505,11 @@ class Transaction(object): >> if self != self.idl.txn: >> return self._status >> > > Sorry, I should've probably mentioned this in the previous review, but I > missed it. > > Nit: the comment from f50714bf9a72 ("ovsdb-idl: Avoid sending > transactions when the DB is not synced up.") would be nice to have here too: > > # If we're still connecting or re-connecting, don't bother sending a > # transaction. > > I guess this can be fixed up at apply time so: > > Acked-by: Dumitru Ceara <dceara@redhat.com> Thanks! Applied and backported down to 2.15. Further backports will require code changes, so, please, send the backported patches if they are needed. Dumitru, sorry, I messed up and didn't include the suggested comment. Feel free to submit a separate patch for that if you think it's needed. Best regards, Ilya Maximets.
On 10/12/21 9:24 PM, Ilya Maximets wrote: > Dumitru, sorry, I messed up and didn't include the suggested comment. > Feel free to submit a separate patch for that if you think it's needed. It was a nit, I'm not sure it's worth a separate patch. It's OK, thanks!
diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py index ecae5e143..87ee06cde 100644 --- a/python/ovs/db/idl.py +++ b/python/ovs/db/idl.py @@ -1505,6 +1505,11 @@ class Transaction(object): 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 + # If we need a lock but don't have it, give up quickly. if self.idl.lock_name and not self.idl.has_lock: self._status = Transaction.NOT_LOCKED
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 | 5 +++++ 1 file changed, 5 insertions(+)