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

Legg til backoff på distribusjons-retry #1843

Merged
merged 1 commit into from
Jun 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package no.nav.su.se.bakover.common.domain.backoff

import arrow.core.Either
import arrow.core.left
import arrow.core.right
import no.nav.su.se.bakover.common.tid.Tidspunkt
import java.time.Clock
import kotlin.time.Duration
import kotlin.time.Duration.Companion.hours
import kotlin.time.Duration.Companion.minutes

fun Failures.shouldRetry(
clock: Clock,
delayTable: Map<Long, Duration> = mapOf(
1L to 1.minutes,
2L to 5.minutes,
3L to 15.minutes,
4L to 30.minutes,
5L to 1.hours,
6L to 2.hours,
7L to 4.hours,
8L to 8.hours,
9L to 12.hours,
10L to 24.hours,
),
maxDelay: Duration = 24.hours,
): Either<Unit, Failures> {
if (last == null) return this.inc(clock).right()
val delayDuration = delayTable[count]?.coerceAtMost(maxDelay) ?: maxDelay
val nextRetryTime = last.plus(delayDuration)
return if (Tidspunkt.now(clock) >= nextRetryTime) {
this.inc(clock).right()
} else {
Unit.left()
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package no.nav.su.se.bakover.common.domain.backoff

import no.nav.su.se.bakover.common.tid.Tidspunkt
import java.time.Clock

/**
* Oversikt over antall feil og tidspunkt for siste feil.
* Typisk brukt for å vurdere om en operasjon skal prøves på nytt.
*/
data class Failures(
val count: Long,
val last: Tidspunkt?,
) {
fun inc(clock: Clock): Failures {
return Failures(
count = count + 1,
last = Tidspunkt.now(clock),
)
}

companion object {
val EMPTY = Failures(0, null)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import java.time.temporal.TemporalAdjuster
import java.time.temporal.TemporalAmount
import java.time.temporal.TemporalUnit
import java.util.Date
import kotlin.time.toJavaDuration

private val tidspunktPresisjon: ChronoUnit = ChronoUnit.MICROS

Expand Down Expand Up @@ -77,6 +78,7 @@ private constructor(
override fun hashCode() = instant.hashCode()
override fun plus(amount: Long, unit: TemporalUnit): Tidspunkt = instant.plus(amount, unit).toTidspunkt()
override fun plus(amount: TemporalAmount): Tidspunkt = instant.plus(amount).toTidspunkt()
fun plus(duration: kotlin.time.Duration): Tidspunkt = instant.plus(duration.toJavaDuration()).toTidspunkt()
override fun minus(amount: Long, unit: TemporalUnit): Tidspunkt = instant.minus(amount, unit).toTidspunkt()
fun toLocalDate(zoneId: ZoneId): LocalDate = LocalDate.ofInstant(instant, zoneId)
fun plusUnits(units: Int): Tidspunkt = this.plus(units.toLong(), tidspunktPresisjon)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
package no.nav.su.se.bakover.common.domain.backoff

import arrow.core.left
import arrow.core.right
import io.kotest.matchers.shouldBe
import no.nav.su.se.bakover.common.tid.Tidspunkt
import no.nav.su.se.bakover.test.fixedClock
import no.nav.su.se.bakover.test.plus
import org.junit.jupiter.api.Test
import java.time.temporal.ChronoUnit

internal class ExponentialBackoffTest {
@Test
fun `Should retry when empty`() {
Failures.EMPTY.shouldRetry(fixedClock) shouldBe Failures(
count = 1,
last = Tidspunkt.now(fixedClock),
).right()
}

@Test
fun `Should not retry when 1 failure and less than a minute has passed`() {
val clockBefore = fixedClock
val clockAfter = clockBefore.plus(59, ChronoUnit.SECONDS)
Failures(
count = 1,
last = Tidspunkt.now(fixedClock),
).shouldRetry(clockAfter) shouldBe Unit.left()
}

@Test
fun `Should retry when 1 failure and 1 minute has passed`() {
val clockBefore = fixedClock
val clockAfter = clockBefore.plus(1, ChronoUnit.MINUTES)
Failures(
count = 1,
last = Tidspunkt.now(fixedClock),
).shouldRetry(clockAfter) shouldBe Failures(
count = 2,
last = Tidspunkt.now(clockAfter),
).right()
}

@Test
fun `Should retry when 1 failure and more than 1 minute has passed`() {
val clockBefore = fixedClock
val clockAfter = clockBefore.plus(1, ChronoUnit.DAYS)
Failures(
count = 1,
last = Tidspunkt.now(fixedClock),
).shouldRetry(clockAfter) shouldBe Failures(
count = 2,
last = Tidspunkt.now(clockAfter),
).right()
}

@Test
fun `Should not retry when 2 failure and less than 5 minutes has passed`() {
val clockBefore = fixedClock
val clockAfter = clockBefore.plus(299, ChronoUnit.SECONDS)
Failures(
count = 2,
last = Tidspunkt.now(fixedClock),
).shouldRetry(clockAfter) shouldBe Unit.left()
}

@Test
fun `Should retry when 2 failure and 5 minute has passed`() {
val clockBefore = fixedClock
val clockAfter = clockBefore.plus(5, ChronoUnit.MINUTES)
Failures(
count = 2,
last = Tidspunkt.now(fixedClock),
).shouldRetry(clockAfter) shouldBe Failures(
count = 3,
last = Tidspunkt.now(clockAfter),
).right()
}

@Test
fun `should not retry if less than max value`() {
val clockBefore = fixedClock
val clockAfter = clockBefore.plus(1439, ChronoUnit.MINUTES)
Failures(
count = 11,
last = Tidspunkt.now(fixedClock),
).shouldRetry(clockAfter) shouldBe Unit.left()
}

@Test
fun `Should support retries outside Map values`() {
val clockBefore = fixedClock
val clockAfter = clockBefore.plus(24, ChronoUnit.HOURS)
Failures(
count = 11,
last = Tidspunkt.now(fixedClock),
).shouldRetry(clockAfter) shouldBe Failures(
count = 12,
last = Tidspunkt.now(clockAfter),
).right()
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
ALTER TABLE dokument_distribusjon
ADD COLUMN IF NOT EXISTS
distribusjon_failure_count bigint not null default 0;

ALTER TABLE dokument_distribusjon
ADD COLUMN IF NOT EXISTS
distribusjon_last_failure_timestamp timestamptz default null;
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package no.nav.su.se.bakover.database
import dokument.domain.JournalføringOgBrevdistribusjon
import dokument.domain.brev.BrevbestillingId
import io.kotest.matchers.shouldBe
import no.nav.su.se.bakover.common.domain.backoff.Failures
import no.nav.su.se.bakover.common.journal.JournalpostId
import org.junit.jupiter.api.Nested
import org.junit.jupiter.api.Test
Expand All @@ -16,6 +17,7 @@ internal class JournalføringOgBrevdistribusjonTest {
JournalføringOgBrevdistribusjon.fromId(
iverksattJournalpostId = null,
iverksattBrevbestillingId = null,
distribusjonFailures = Failures.EMPTY,
) shouldBe JournalføringOgBrevdistribusjon.IkkeJournalførtEllerDistribuert.also {
JournalføringOgBrevdistribusjon.iverksattJournalpostId(it) shouldBe null
JournalføringOgBrevdistribusjon.iverksattBrevbestillingId(it) shouldBe null
Expand All @@ -27,7 +29,11 @@ internal class JournalføringOgBrevdistribusjonTest {
JournalføringOgBrevdistribusjon.fromId(
iverksattJournalpostId = JournalpostId("13"),
iverksattBrevbestillingId = null,
) shouldBe JournalføringOgBrevdistribusjon.Journalført(JournalpostId("13")).also {
distribusjonFailures = Failures.EMPTY,
) shouldBe JournalføringOgBrevdistribusjon.Journalført(
JournalpostId("13"),
Failures.EMPTY,
).also {
JournalføringOgBrevdistribusjon.iverksattJournalpostId(it) shouldBe JournalpostId("13")
JournalføringOgBrevdistribusjon.iverksattBrevbestillingId(it) shouldBe null
}
Expand All @@ -38,9 +44,11 @@ internal class JournalføringOgBrevdistribusjonTest {
JournalføringOgBrevdistribusjon.fromId(
iverksattJournalpostId = JournalpostId("13"),
iverksattBrevbestillingId = BrevbestillingId("45"),
distribusjonFailures = Failures.EMPTY,
) shouldBe JournalføringOgBrevdistribusjon.JournalførtOgDistribuertBrev(
JournalpostId("13"),
BrevbestillingId("45"),
Failures.EMPTY,
).also {
JournalføringOgBrevdistribusjon.iverksattJournalpostId(it) shouldBe JournalpostId("13")
JournalføringOgBrevdistribusjon.iverksattBrevbestillingId(it) shouldBe BrevbestillingId("45")
Expand All @@ -53,6 +61,7 @@ internal class JournalføringOgBrevdistribusjonTest {
JournalføringOgBrevdistribusjon.fromId(
iverksattJournalpostId = null,
iverksattBrevbestillingId = BrevbestillingId("45"),
distribusjonFailures = Failures.EMPTY,
)
}
}
Expand All @@ -65,7 +74,8 @@ internal class JournalføringOgBrevdistribusjonTest {
JournalføringOgBrevdistribusjon.fromId(
iverksattJournalpostId = JournalpostId("13"),
iverksattBrevbestillingId = null,
) shouldBe JournalføringOgBrevdistribusjon.Journalført(JournalpostId("13")).also {
distribusjonFailures = Failures.EMPTY,
) shouldBe JournalføringOgBrevdistribusjon.Journalført(JournalpostId("13"), Failures.EMPTY).also {
JournalføringOgBrevdistribusjon.iverksattJournalpostId(it) shouldBe JournalpostId("13")
JournalføringOgBrevdistribusjon.iverksattBrevbestillingId(it) shouldBe null
}
Expand All @@ -76,9 +86,11 @@ internal class JournalføringOgBrevdistribusjonTest {
JournalføringOgBrevdistribusjon.fromId(
iverksattJournalpostId = JournalpostId("13"),
iverksattBrevbestillingId = BrevbestillingId("45"),
distribusjonFailures = Failures.EMPTY,
) shouldBe JournalføringOgBrevdistribusjon.JournalførtOgDistribuertBrev(
JournalpostId("13"),
BrevbestillingId("45"),
Failures.EMPTY,
).also {
JournalføringOgBrevdistribusjon.iverksattJournalpostId(it) shouldBe JournalpostId("13")
JournalføringOgBrevdistribusjon.iverksattBrevbestillingId(it) shouldBe BrevbestillingId("45")
Expand All @@ -91,6 +103,7 @@ internal class JournalføringOgBrevdistribusjonTest {
JournalføringOgBrevdistribusjon.fromId(
iverksattJournalpostId = null,
iverksattBrevbestillingId = BrevbestillingId("45"),
distribusjonFailures = Failures.EMPTY,
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import dokument.domain.distribuering.Distribueringsadresse
import io.kotest.matchers.collections.shouldHaveSize
import io.kotest.matchers.equality.shouldBeEqualToIgnoringFields
import io.kotest.matchers.shouldBe
import no.nav.su.se.bakover.common.domain.backoff.Failures
import no.nav.su.se.bakover.common.journal.JournalpostId
import no.nav.su.se.bakover.test.fixedClock
import no.nav.su.se.bakover.test.fixedTidspunkt
import no.nav.su.se.bakover.test.nyDistribueringsAdresse
import no.nav.su.se.bakover.test.pdfATom
Expand Down Expand Up @@ -113,10 +115,11 @@ internal class DokumentPostgresRepoTest {

journalført.journalføringOgBrevdistribusjon shouldBe JournalføringOgBrevdistribusjon.Journalført(
JournalpostId("jp"),
Failures.EMPTY,
)

dokumentRepo.oppdaterDokumentdistribusjon(
journalført.distribuerBrev { BrevbestillingId("brev").right() }.getOrElse {
journalført.distribuerBrev(fixedClock) { BrevbestillingId("brev").right() }.getOrElse {
fail { "Skulle fått bestilt brev" }
},
)
Expand All @@ -128,6 +131,7 @@ internal class DokumentPostgresRepoTest {
journalførtOgBestiltBrev.journalføringOgBrevdistribusjon shouldBe JournalføringOgBrevdistribusjon.JournalførtOgDistribuertBrev(
JournalpostId("jp"),
BrevbestillingId("brev"),
Failures.EMPTY,
)
}
}
Expand Down Expand Up @@ -159,6 +163,7 @@ internal class DokumentPostgresRepoTest {
journalførtDistribusjon.let {
it.journalføringOgBrevdistribusjon shouldBe JournalføringOgBrevdistribusjon.Journalført(
JournalpostId("jp"),
Failures.EMPTY,
)
it.dokument.distribueringsadresse shouldBe nyDistribueringsAdresse()
}
Expand Down Expand Up @@ -193,7 +198,7 @@ internal class DokumentPostgresRepoTest {
dokumentdistribusjonUtenJournalpostIdOgBrevbestillingsId.journalfør { JournalpostId("jp").right() }
.getOrElse {
fail { "Skulle fått journalført" }
}.distribuerBrev { BrevbestillingId("brev").right() }.getOrElse {
}.distribuerBrev(fixedClock) { BrevbestillingId("brev").right() }.getOrElse {
fail { "Skulle fått bestilt brev" }
},
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -763,7 +763,7 @@ internal class VedtakPostgresRepoTest {
dokumentRepo.oppdaterDokumentdistribusjon(
journalført.journalfør { JournalpostId("jp").right() }.getOrElse {
fail { "Skulle fått journalført" }
}.distribuerBrev { BrevbestillingId("brev").right() }.getOrElse {
}.distribuerBrev(testDataHelper.clock) { BrevbestillingId("brev").right() }.getOrElse {
fail { "Skulle fått bestilt brev" }
},
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,10 @@ class DistribuerDokumentHendelserKonsument(
journalpostId = journalpostId,
).left()
}
// TODO jah: Her hopper vi bukk over [dokument.domain.Dokumentdistribusjon] og vil skippe retry-backoff logikk.
// Vi måtte uansett ha persistert en (failure)-hendelse for å kunne benytte oss av retry-backoff logikken.
return dokDistFordeling.bestillDistribusjon(
journalPostId = journalpostId,
journalpostId = journalpostId,
distribusjonstype = generertDokumentHendelse.dokumentUtenFil.distribusjonstype,
distribusjonstidspunkt = generertDokumentHendelse.dokumentUtenFil.distribusjonstidspunkt,
distribueringsadresse = distribueringsadresse,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@ package dokument.domain

import arrow.core.Either
import dokument.domain.brev.BrevbestillingId
import dokument.domain.brev.KunneIkkeDistribuereBrev
import dokument.domain.distribuering.KunneIkkeBestilleDistribusjon
import no.nav.su.se.bakover.common.domain.backoff.Failures
import no.nav.su.se.bakover.common.journal.JournalpostId
import no.nav.su.se.bakover.common.tid.Tidspunkt
import java.time.Clock
import java.util.UUID

/**
Expand All @@ -16,13 +20,41 @@ data class Dokumentdistribusjon(
val dokument: Dokument.MedMetadata,
val journalføringOgBrevdistribusjon: JournalføringOgBrevdistribusjon,
) {

val distribusjonFailures: Failures
get() = journalføringOgBrevdistribusjon.distribusjonFailures

fun journalfør(journalfør: () -> Either<KunneIkkeJournalføreOgDistribuereBrev.KunneIkkeJournalføre.FeilVedJournalføring, JournalpostId>): Either<KunneIkkeJournalføreOgDistribuereBrev.KunneIkkeJournalføre, Dokumentdistribusjon> {
return journalføringOgBrevdistribusjon.journalfør(journalfør)
.map { copy(journalføringOgBrevdistribusjon = it) }
}

fun distribuerBrev(distribuerBrev: (journalpostId: JournalpostId) -> Either<KunneIkkeJournalføreOgDistribuereBrev.KunneIkkeDistribuereBrev.FeilVedDistribueringAvBrev, BrevbestillingId>): Either<KunneIkkeJournalføreOgDistribuereBrev.KunneIkkeDistribuereBrev, Dokumentdistribusjon> {
return journalføringOgBrevdistribusjon.distribuerBrev(distribuerBrev)
/**
* @param ignoreBackoff I noen tilfeller ønsker vi ikke å ta hensyn til backoff-strategien. F.eks. dersom saksbehandler ønsker overstyre distribusjonsadresse.
*/
fun distribuerBrev(
clock: Clock,
ignoreBackoff: Boolean = false,
distribuerBrev: (journalpostId: JournalpostId) -> Either<KunneIkkeBestilleDistribusjon, BrevbestillingId>,
): Either<KunneIkkeJournalføreOgDistribuereBrev.KunneIkkeDistribuereBrev, Dokumentdistribusjon> {
return journalføringOgBrevdistribusjon.distribuerBrev(clock, ignoreBackoff, distribuerBrev)
.mapLeft {
when (it) {
is JournalføringOgBrevdistribusjon.KunneIkkeDistribuereBrev.ForTidligÅPrøvePåNytt -> KunneIkkeJournalføreOgDistribuereBrev.KunneIkkeDistribuereBrev.ForTidligÅPrøvePåNytt
is JournalføringOgBrevdistribusjon.KunneIkkeDistribuereBrev.OppdatertFailures -> KunneIkkeJournalføreOgDistribuereBrev.KunneIkkeDistribuereBrev.OppdatertFailures(
journalpostId = it.journalføringOgBrevdistribusjon.journalpostId,
dokumentdistribusjon = this.copy(
journalføringOgBrevdistribusjon = it.journalføringOgBrevdistribusjon,
),
)

is JournalføringOgBrevdistribusjon.KunneIkkeDistribuereBrev.MåJournalføresFørst -> KunneIkkeJournalføreOgDistribuereBrev.KunneIkkeDistribuereBrev.MåJournalføresFørst
is JournalføringOgBrevdistribusjon.KunneIkkeDistribuereBrev.AlleredeDistribuertBrev -> KunneIkkeJournalføreOgDistribuereBrev.KunneIkkeDistribuereBrev.AlleredeDistribuertBrev(
it.journalpostId,
it.brevbestillingId,
)
}
}
.map { copy(journalføringOgBrevdistribusjon = it) }
}
}
Loading