manipatnam commented on code in PR #63874:
URL: https://github.com/apache/airflow/pull/63874#discussion_r3331407507
##########
airflow-core/tests/unit/models/test_renderedtifields.py:
##########
@@ -372,6 +372,37 @@ def test_write(self, dag_maker):
{"bash_command": "echo test_val_updated", "env": None, "cwd":
None},
)
+ def test_write_upsert_existing_record(self, dag_maker, session):
+ """
+ Test that writing RTIF for the same task instance twice updates the
existing
+ record rather than raising an IntegrityError. This is the scenario
that occurs
+ when the SDK retries an RTIF PUT after a client-side timeout.
+ """
Review Comment:
Fair point. Reproducing the actual race needs two truly concurrent
transactions hitting the same PK, which I couldn't get to reproduce reliably in
a unit test. So I changed it to seed the existing row via a direct INSERT
(bypassing write()) and assert write() updates it without an IntegrityError.
You're right, it'd also pass with the old merge() — the real win is in
production: merge() does SELECT-then-INSERT/UPDATE separately, which is what
opens the race; the ON CONFLICT DO UPDATE upsert makes it a single atomic
statement. Happy to add a proper concurrency test if anyone has a good
suggestion for doing it reliably.
--
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]
