dstandish commented on PR #27344: URL: https://github.com/apache/airflow/pull/27344#issuecomment-1309860668
> > @NickYadance did you try this fix? did it resolve the issue? do you think it's possible to write a test for it? i imagine you could might simulate the scenario by managing two sessions. if so, you could write a test that reliably fails without the fix, then add the fix and leave the test. > > the reason I ask is because i see that that the retry is within the scope of the session, and i'm not sure whether, if we get a deadlock error, the session will need to be rolled back. > > and, another option would be to put `@tenacity.retry(stop=tenacity.stop_after_attempt(MAX_DB_TRIES))` as the outermost decorator, and then each failed attempt would get rolled back and a new session created for the next attempt > > No actually. The solution is from here as these are similiar cases. > > https://github.com/apache/airflow/blob/b29ca4e77d4d80fb1f4d6d4b497a3a14979dd244/airflow/models/trigger.py#L100-L104 > > > Working on to reproduce this... Here's an example that illustrates what I'm saying: ``` from airflow.models import Log from airflow.settings import Session session = Session() for i in range(3): val = 'hi' if i == 2 else {} print(f"{val=}") try: session.add(Log(event=val)) session.commit() break except Exception as e: print(f'failure: {e}') ``` This line `session.add(Log(event=val))` fails when val is dict but succeeds when string. The first two times it runs, it runs with dict. The third time it runs with string. But you will see that it doesn't matter that the third try uses a good value; it will still fail because the transaction has not been rolled back. The only difference is the error is `(sqlite3.InterfaceError) Error binding parameter 4 - probably unsupported type` instead of deadlock error. Now... is that a difference that makes a difference? I am not sure. But, I think it's likely. If that's true, this won't work and you'll have to do the retry around the whole transaction instead of within it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
