Retry serialization failures in test request creation

Miroslav Vadkertirequested to merge
mvadkert/nucleus:fix-create-request-serialization-retry into main

Summary

Fixes a regression where POST /v0.1/requests returned 500 Internal Server Error on CockroachDB serialization failures.

1145e34 (Handle CockroachDB serialization failures) wrapped update_test_request() and delete_test_request() in retry_on_serialization_failure() but left create_test_request() unprotected, so the create path regressed to a 500 on conflicts.

Reported in Sentry: TESTING-FARM-API-3S. The aborted transaction is the request INSERT itself — the conflict key in the event decodes to exactly the request_id from the same event.

Root cause

Under CockroachDB SERIALIZABLE, inserting a Request performs a foreign-key existence read of the parent tokens row. Shared worker tokens submit many requests concurrently, and once the 60s throttle window expires _touch_last_used_at() issues UPDATE tokens SET last_used_at=... on that same row. The insert's FK read races the token UPDATE, and CockroachDB aborts the insert at commit — the same class of conflict the original fix targeted, just reaching a path it did not cover.

Changes

  • crud/test_request.py: wrap the create in retry_on_serialization_failure(). Because the helper rolls back the session between attempts (which expunges the pending ORM instance), the Request is now built and added inside the retried closure. The request_id is generated once outside so it stays stable across retries — an aborted attempt commits nothing.
  • tests/test_crud_test_request.py: add create-path retry coverage and consolidate the three transient-failure retry tests (create, update, delete) into a single pytest.mark.parametrized test over scenario functions. They shared an identical skeleton and differed only in arrange and assertions. Each scenario arranges its target before the flaky commit is installed, so the arrange itself is not affected by the injected failure.

Assisted-by: Claude Code

Merge request reports