"wrap_db_retry" with autocommit=False

[Rodolfo Alonso]

This is more a question than a bug.

We have updated Neutron/neutron-lib code to be compliant with SQLAlchemy 2.0. We have set "autocommit=False" [1] in the session creation. However, we are seeing multiple errors in our tempest tests. E.g.: [2] (but any other CI execution of [1] have similar errors in tempest).

Those errors have always the same pattern: a register "standardattributes" is not updated [3] and the transaction is rolled back. If we use "oslo_db.api.wrap_db_retry", the active transaction is "reused", triggering the error in [2].

Should "oslo_db.api.wrap_db_retry" abort the current failed transaction and create a new one? Or should we do this in our code?

Thank you in advance for your time.

[1]https://review.opendev.org/c/openstack/neutron-lib/+/828738/6/neutron_lib/db/api.py#54
[2]https://paste.opendev.org/show/bMCMScXFLXXg98yfCKh3/
[3]"UPDATE statement on table 'standardattributes' expected to update 1 row(s); 0 were matched"

[Mike Bayer (zzzeek)]

the oslo_db retry wrapper does not have any knowledge of the current transaction in progress or by what mechanism the current transaction is being managed. My understanding is that Neutron has its own retry wrappers that are used internally that are extending the oslo.db one to include database aware behaviors, that logic is all here: https://github.com/openstack/neutron-lib/blob/master/neutron_lib/db/api.py . This is the part where you'd want to make sure that when a function is retried, the new run of the function can access the session/connection in a "ready" state.

If the session was previously in a plain transaction, this means rolling it back when an error occurs. If it was inside of a savepoint, you might want to be just reverting that savepoint only; in https://github.com/openstack/neutron-lib/blob/master/neutron_lib/db/api.py there's references to "nested" which is what a SAVEPOINT is referred towards in SQLAlchemy Session.

For oslo.db, the original idea was that everyone would be using the "new" enginefacade at https://github.com/openstack/oslo.db/blob/master/oslo_db/sqlalchemy/enginefacade.py ; when using this library, either the decorator or the context manager will always call rollback() when an exception is encountered, before re-propagating the exception outward, see https://github.com/openstack/oslo.db/blob/22c602f075795b6d5ecbbc2e229817f759613ea2/oslo_db/sqlalchemy/enginefacade.py#L662 . My understanding is that neutron didn't fully migrate to enginefacade when this API was introduced.

[Rodolfo Alonso (rodolfo-alonso-hernandez)]

"My understanding is that neutron didn't fully migrate to enginefacade when this API was introduced." ufff this is more serious than I expected... OK, I need to check the oslo.db enginefacade and what bits are missing in Neutron.

Thanks a lot Mike!