From 30506b0cc4f7c52ddbcd47a8960c720117115098 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D0=BB=D0=B5=D0=BA=D1=81=D0=B0=D0=BD=D0=B4=D1=80=20?= =?UTF-8?q?=D0=97=D0=B8=D0=BC=D0=B8=D0=BD?= Date: Tue, 2 Jun 2026 11:56:03 +0000 Subject: [PATCH] =?UTF-8?q?fix(approve):=20retry=20=D0=BD=D0=B0=20Postgres?= =?UTF-8?q?=20SERIALIZABLE=2040001=20(=D0=BC=D0=B0=D1=81=D1=81=D0=BE=D0=B2?= =?UTF-8?q?=D1=8B=D0=B9=20approve=20drafts)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../restapi/service/SerializableRetry.java | 83 +++++++++++++++++++ .../ordinis/restapi/web/DraftController.java | 11 ++- .../restapi/web/SchemaDraftController.java | 7 +- .../service/SerializableRetryTest.java | 57 +++++++++++++ 4 files changed, 154 insertions(+), 4 deletions(-) create mode 100644 ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/service/SerializableRetry.java create mode 100644 ordinis-rest-api/src/test/java/cloud/nstart/terravault/ordinis/restapi/service/SerializableRetryTest.java diff --git a/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/service/SerializableRetry.java b/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/service/SerializableRetry.java new file mode 100644 index 0000000..fac1c71 --- /dev/null +++ b/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/service/SerializableRetry.java @@ -0,0 +1,83 @@ +package cloud.nstart.terravault.ordinis.restapi.service; + +import java.util.concurrent.ThreadLocalRandom; +import java.util.function.Supplier; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.dao.CannotAcquireLockException; + +/** + * Retry helper для SERIALIZABLE-isolation транзакций (DraftService.approve / + * reject и SchemaDraftService.publish). + * + *

Postgres при isolation=SERIALIZABLE использует SSI (Serializable + * Snapshot Isolation). Когда две транзакции читают/пишут пересекающиеся + * read-write зависимости, одна из них кэнселится с {@code SQLSTATE 40001}: + * + * 

{@code
+ * ERROR: could not serialize access due to read/write dependencies among transactions
+ *   Hint: The transaction might succeed if retried.
+ * }
+ * + *

Это transient ошибка — Postgres сам в Hint говорит retry. Без + * retry даже корректный код упадёт 500 при concurrent approve drafts (юзер + * выбрал 10 drafts → bulk approve fan-out → 1-2 транзакции aborted). + * + *

ВАЖНО: этот helper должен вызываться ИЗ ВНЕ @Transactional + * boundary (например, из Controller'а). Если retry внутри @Transactional — + * каждый attempt будет запускаться в той же aborted транзакции и снова + * упадёт. Service'ы остаются @Transactional как раньше — controller + * заворачивает их вызов. + * + *

Backoff: exponential (50ms / 100ms / 200ms) + jitter [0..50ms). После + * 3 неуспешных попыток — пробрасываем последнюю CannotAcquireLockException + * → GlobalExceptionHandler → 500. Это очень редкий case (3 retries при + * high contention) — лучше fail чем infinite loop. + */ +public final class SerializableRetry { + private static final Logger log = LoggerFactory.getLogger(SerializableRetry.class); + private static final int MAX_ATTEMPTS = 3; + private static final long BASE_DELAY_MS = 50; + + private SerializableRetry() {} + + /** + * Запустить операцию с retry на {@link CannotAcquireLockException} (Spring + * translation для Postgres 40001 serialization_failure). + * + * @param op операция — обычно лямбда вызывающая service.approve(...) + * @return результат успешной попытки + * @throws CannotAcquireLockException если все {@value #MAX_ATTEMPTS} + * попыток упали (бросаем последнюю — GlobalExceptionHandler + * маппит в 500) + */ + public static T execute(Supplier op) { + CannotAcquireLockException last = null; + for (int attempt = 0; attempt < MAX_ATTEMPTS; attempt++) { + try { + return op.get(); + } catch (CannotAcquireLockException e) { + last = e; + if (attempt < MAX_ATTEMPTS - 1) { + long backoff = (1L << attempt) * BASE_DELAY_MS; + long jitter = ThreadLocalRandom.current().nextLong(0, BASE_DELAY_MS); + long sleepMs = backoff + jitter; + log.warn( + "Postgres SERIALIZABLE conflict; retry {}/{} after {}ms", + attempt + 1, MAX_ATTEMPTS, sleepMs); + try { + Thread.sleep(sleepMs); + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + throw e; + } + } + } + } + log.error( + "Postgres SERIALIZABLE conflict: exhausted {} attempts; surfacing 500", + MAX_ATTEMPTS, last); + throw last; + } +} diff --git a/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/web/DraftController.java b/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/web/DraftController.java index f3092e6..6be5039 100644 --- a/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/web/DraftController.java +++ b/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/web/DraftController.java @@ -2,6 +2,7 @@ package cloud.nstart.terravault.ordinis.restapi.web; import cloud.nstart.terravault.ordinis.restapi.dto.draft.DraftResponse; import cloud.nstart.terravault.ordinis.restapi.dto.draft.SubmitDraftRequest; +import cloud.nstart.terravault.ordinis.restapi.service.SerializableRetry; import cloud.nstart.terravault.ordinis.restapi.service.draft.DraftService; import jakarta.validation.Valid; import org.springframework.data.domain.PageRequest; @@ -89,7 +90,12 @@ public class DraftController { public DraftResponse approve( @PathVariable UUID id, @RequestParam(required = false) String comment) { - return DraftResponse.from(draftService.approve(id, comment)); + // SERIALIZABLE retry — при bulk approve N drafts (UI fan-out) + // Postgres SSI кэнселит concurrent transactions с 40001. Без retry + // юзер получит 500 на random subset. Retry — outside @Transactional + // (каждый attempt — fresh tx). + return DraftResponse.from( + SerializableRetry.execute(() -> draftService.approve(id, comment))); } /** @@ -100,7 +106,8 @@ public class DraftController { public DraftResponse reject( @PathVariable UUID id, @RequestParam String comment) { - return DraftResponse.from(draftService.reject(id, comment)); + return DraftResponse.from( + SerializableRetry.execute(() -> draftService.reject(id, comment))); } /** diff --git a/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/web/SchemaDraftController.java b/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/web/SchemaDraftController.java index 4b6793b..cbf7358 100644 --- a/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/web/SchemaDraftController.java +++ b/ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/web/SchemaDraftController.java @@ -9,6 +9,7 @@ import cloud.nstart.terravault.ordinis.restapi.dto.schemaworkflow.SchemaDraftRes import cloud.nstart.terravault.ordinis.restapi.dto.schemaworkflow.SubmitForReviewRequest; import cloud.nstart.terravault.ordinis.restapi.dto.schemaworkflow.UpdateSchemaDraftRequest; import cloud.nstart.terravault.ordinis.restapi.service.DictionaryDefinitionService; +import cloud.nstart.terravault.ordinis.restapi.service.SerializableRetry; import cloud.nstart.terravault.ordinis.restapi.service.schemaworkflow.SchemaDraftService; import jakarta.validation.Valid; import org.springframework.data.domain.PageRequest; @@ -107,7 +108,8 @@ public class SchemaDraftController { @PathVariable String name, @PathVariable UUID draftId, @Valid @RequestBody DecisionRequest req) { - DictionarySchemaDraft draft = draftService.decide(draftId, req); + DictionarySchemaDraft draft = SerializableRetry.execute( + () -> draftService.decide(draftId, req)); return SchemaDraftResponse.from(draft, name); } @@ -125,7 +127,8 @@ public class SchemaDraftController { @PathVariable String name, @PathVariable UUID draftId, @Valid @RequestBody PublishDraftRequest req) { - DictionarySchemaDraft draft = draftService.publish(draftId, req); + DictionarySchemaDraft draft = SerializableRetry.execute( + () -> draftService.publish(draftId, req)); return Map.of( "draft", SchemaDraftResponse.from(draft, name), "newVersion", draft.getPublishedVersion(), diff --git a/ordinis-rest-api/src/test/java/cloud/nstart/terravault/ordinis/restapi/service/SerializableRetryTest.java b/ordinis-rest-api/src/test/java/cloud/nstart/terravault/ordinis/restapi/service/SerializableRetryTest.java new file mode 100644 index 0000000..5539f24 --- /dev/null +++ b/ordinis-rest-api/src/test/java/cloud/nstart/terravault/ordinis/restapi/service/SerializableRetryTest.java @@ -0,0 +1,57 @@ +package cloud.nstart.terravault.ordinis.restapi.service; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.util.concurrent.atomic.AtomicInteger; + +import org.junit.jupiter.api.Test; +import org.springframework.dao.CannotAcquireLockException; + +class SerializableRetryTest { + + @Test + void returnsResultOnFirstAttemptWhenNoConflict() { + AtomicInteger calls = new AtomicInteger(); + String result = SerializableRetry.execute(() -> { + calls.incrementAndGet(); + return "ok"; + }); + assertThat(result).isEqualTo("ok"); + assertThat(calls.get()).isEqualTo(1); + } + + @Test + void retriesOnSerializableConflictAndSucceeds() { + AtomicInteger calls = new AtomicInteger(); + String result = SerializableRetry.execute(() -> { + int attempt = calls.incrementAndGet(); + if (attempt < 3) { + throw new CannotAcquireLockException("40001 serialization_failure"); + } + return "ok"; + }); + assertThat(result).isEqualTo("ok"); + assertThat(calls.get()).isEqualTo(3); + } + + @Test + void surfacesExceptionAfterMaxAttempts() { + AtomicInteger calls = new AtomicInteger(); + assertThatThrownBy(() -> SerializableRetry.execute(() -> { + calls.incrementAndGet(); + throw new CannotAcquireLockException("40001 still"); + })).isInstanceOf(CannotAcquireLockException.class); + assertThat(calls.get()).isEqualTo(3); + } + + @Test + void doesNotRetryOnOtherExceptions() { + AtomicInteger calls = new AtomicInteger(); + assertThatThrownBy(() -> SerializableRetry.execute(() -> { + calls.incrementAndGet(); + throw new IllegalStateException("nope"); + })).isInstanceOf(IllegalStateException.class); + assertThat(calls.get()).isEqualTo(1); + } +}