Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix flaky sqlite tests with test_xcom_map_nest hopefully #33145

Merged
merged 1 commit into from
Aug 5, 2023

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Aug 5, 2023

Recently sqlite started to fail randomly during teardown of test_xcom_map_nest or test_xcom_map_zip_nest.
It looks very strange:

sqlite3.ProgrammingError: SQLite objects created in a thread can only be
used in that same thread

By analysing possible reasons it seems that it was a side effect of the existing test_xcom_map_nest that allocated a new session in the run method of task instance rather than pass the sesion that is created and torrn down in the pytest fixture.

The hypothesis is that the session created in the test_xcom_map_nest were being reclaimed and closed while the test_xcom_map_zip_nest test was already starting in a different thread started by Pytest.

The fix is to pass the session object to run method of the taskinstance in the test_xcom_map_nest test.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

Recently sqlite started to fail randomly during teardown of
`test_xcom_map_nest` or `test_xcom_map_zip_nest`. This happpened
after adding `test_xcom_map_zip_nest` . It looked very strange:

```
sqlite3.ProgrammingError: SQLite objects created in a thread can only be
used in that same thread
```

By analysing possible reasons it seems that it was a side effect
of the existing `test_xcom_map_nest` that allocated a new
session in the run method of task instance rather than pass
the sesion that is created and torrn down in the pytest fixture.

The hypothesis is that the session created in the ``test_xcom_map_nest``
were being reclaimed and closed while the `test_xcom_map_zip_nest` test
was already starting in a different thread started by Pytest.

The fix is to pass the session object to run method of the taskinstance
in the ``test_xcom_map_nest`` test.
@potiuk potiuk requested a review from uranusjr August 5, 2023 20:51
@potiuk
Copy link
Member Author

potiuk commented Aug 5, 2023

Hey @uranusjr -> I think I found the reason of the test_xcom_map_nest behaving flaky. Not sure why it started to happen now - but I have a hypothesis explaining the side-effect of one "map_nest" on the "zip" one.

@potiuk
Copy link
Member Author

potiuk commented Aug 5, 2023

Example flaky tests:

---------------------------- Captured log teardown -----------------------------
ERROR    sqlalchemy.pool.impl.NullPool:base.py:791 Exception during reset or similar
Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/pool/base.py", line 763, in _finalize_fairy
    fairy._reset(pool, transaction_was_reset)
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/pool/base.py", line 1038, in _reset
    pool._dialect.do_rollback(self)
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/engine/default.py", line 683, in do_rollback
    dbapi_connection.rollback()
sqlite3.ProgrammingError: SQLite objects created in a thread can only be used in that same thread. The object was created in thread id 140220884317952 and this is thread id 140222344125312.
ERROR    sqlalchemy.pool.impl.NullPool:base.py:264 Exception closing connection <sqlite3.Connection object at 0x7f879f914c70>
Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/pool/base.py", line 763, in _finalize_fairy
    fairy._reset(pool, transaction_was_reset)
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/pool/base.py", line 1038, in _reset
    pool._dialect.do_rollback(self)
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/engine/default.py", line 683, in do_rollback
    dbapi_connection.rollback()
sqlite3.ProgrammingError: SQLite objects created in a thread can only be used in that same thread. The object was created in thread id 140220884317952 and this is thread id 140222344125312.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/pool/base.py", line 260, in _close_connection
    self._dialect.do_terminate(connection)
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/engine/default.py", line 689, in do_terminate
    self.do_close(dbapi_connection)
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/engine/default.py", line 692, in do_close
    dbapi_connection.close()
sqlite3.ProgrammingError: SQLite objects created in a thread can only be used in that same thread. The object was created in thread id 140220884317952 and this is thread id 140222344125312.
=================================== FAILURES ===================================

Copy link
Member

@hussein-awala hussein-awala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

@potiuk potiuk merged commit d1d6fc9 into apache:main Aug 5, 2023
@potiuk potiuk deleted the fix-sqlite-flaky-tests branch August 5, 2023 21:15
@potiuk
Copy link
Member Author

potiuk commented Aug 5, 2023

Nice catch!

Yeah.... Let's see if it helps, but looks plausible.

@potiuk potiuk added this to the Airflow 2.7.0 milestone Aug 5, 2023
@potiuk
Copy link
Member Author

potiuk commented Aug 5, 2023

Also add it to 2.7.0 in the hopes it will stabilize the tests there.

potiuk added a commit to potiuk/airflow that referenced this pull request Aug 6, 2023
Similarly to apache#33145 - this is an attempt to stabilise flaky tests
for the test_xcom_arg_map.

Even if the mechanism is not entirely clear (provide_session should
also close the connection) seems like using pytest-fixture provided
session works better than relying on a new session created in run()
methods.
potiuk added a commit that referenced this pull request Aug 6, 2023
Similarly to #33145 - this is an attempt to stabilise flaky tests
for the test_xcom_arg_map.

Even if the mechanism is not entirely clear (provide_session should
also close the connection) seems like using pytest-fixture provided
session works better than relying on a new session created in run()
methods.
ephraimbuddy pushed a commit that referenced this pull request Aug 8, 2023
Similarly to #33145 - this is an attempt to stabilise flaky tests
for the test_xcom_arg_map.

Even if the mechanism is not entirely clear (provide_session should
also close the connection) seems like using pytest-fixture provided
session works better than relying on a new session created in run()
methods.

(cherry picked from commit 3dd0c99)
ephraimbuddy pushed a commit that referenced this pull request Aug 9, 2023
Recently sqlite started to fail randomly during teardown of
`test_xcom_map_nest` or `test_xcom_map_zip_nest`. This happpened
after adding `test_xcom_map_zip_nest` . It looked very strange:

```
sqlite3.ProgrammingError: SQLite objects created in a thread can only be
used in that same thread
```

By analysing possible reasons it seems that it was a side effect
of the existing `test_xcom_map_nest` that allocated a new
session in the run method of task instance rather than pass
the sesion that is created and torrn down in the pytest fixture.

The hypothesis is that the session created in the ``test_xcom_map_nest``
were being reclaimed and closed while the `test_xcom_map_zip_nest` test
was already starting in a different thread started by Pytest.

The fix is to pass the session object to run method of the taskinstance
in the ``test_xcom_map_nest`` test.

(cherry picked from commit d1d6fc9)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants