Skip to content

Commit

Permalink
Merge pull request #1843 from navikt/distribusjon-retries
Browse files Browse the repository at this point in the history
Legg til backoff på distribusjons-retry
  • Loading branch information
RamziAbuQassim authored Jun 18, 2024
2 parents 93e586f + 8ab5496 commit fc286a1
Show file tree
Hide file tree
Showing 21 changed files with 465 additions and 97 deletions.
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

0 comments on commit fc286a1

Please sign in to comment.