On 9/20/21 18:15, Terry Wilson wrote:
> On Thu, Sep 2, 2021 at 10:53 AM Dumitru Ceara <dce...@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 <twil...@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 <dce...@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
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
