From be863f865c0c511899d75b3554a248313dad56e0 Mon Sep 17 00:00:00 2001 From: Michael Pawliszyn Date: Thu, 21 Dec 2023 10:35:55 -0500 Subject: [PATCH 1/3] Adding connection information when a prepare fails. This is usually the first point when Backfila is trying to communicate with your service so this should help immensely with debugging. --- .../selfbackfill/LocalClientServiceClient.kt | 2 ++ .../BackfilaSelfBackfillDevelopmentService.kt | 2 ++ .../app/cash/backfila/BackfillCreator.kt | 24 +++++++++++++++---- .../client/BackfilaClientServiceClient.kt | 5 ++++ .../client/EnvoyClientServiceClient.kt | 3 +++ .../EnvoyClientServiceClientProvider.kt | 5 +++- .../client/HttpClientServiceClient.kt | 3 +++ .../client/HttpClientServiceClientProvider.kt | 4 +++- .../client/FakeBackfilaClientServiceClient.kt | 2 ++ .../development/BackfilaDevelopmentService.kt | 2 ++ 10 files changed, 45 insertions(+), 7 deletions(-) diff --git a/service-self-backfill/src/main/kotlin/app/cash/backfila/service/selfbackfill/LocalClientServiceClient.kt b/service-self-backfill/src/main/kotlin/app/cash/backfila/service/selfbackfill/LocalClientServiceClient.kt index e83873f38..7956a5998 100644 --- a/service-self-backfill/src/main/kotlin/app/cash/backfila/service/selfbackfill/LocalClientServiceClient.kt +++ b/service-self-backfill/src/main/kotlin/app/cash/backfila/service/selfbackfill/LocalClientServiceClient.kt @@ -24,4 +24,6 @@ internal class LocalClientServiceClient @Inject internal constructor( override suspend fun runBatch(request: RunBatchRequest): RunBatchResponse { return backfilaClientServiceHandler.runBatch(request) } + + override fun connectionLogData() = "LocalClientServiceClient so no connection" } diff --git a/service-self-backfill/src/test/kotlin/app/cash/backfila/service/selfbackfill/BackfilaSelfBackfillDevelopmentService.kt b/service-self-backfill/src/test/kotlin/app/cash/backfila/service/selfbackfill/BackfilaSelfBackfillDevelopmentService.kt index abeddb720..bf87b7bf4 100644 --- a/service-self-backfill/src/test/kotlin/app/cash/backfila/service/selfbackfill/BackfilaSelfBackfillDevelopmentService.kt +++ b/service-self-backfill/src/test/kotlin/app/cash/backfila/service/selfbackfill/BackfilaSelfBackfillDevelopmentService.kt @@ -85,6 +85,8 @@ internal fun main(args: Array) { override suspend fun runBatch(request: RunBatchRequest): RunBatchResponse { TODO("Not yet implemented") } + + override fun connectionLogData() = "Fake development service client" } } }, diff --git a/service/src/main/kotlin/app/cash/backfila/BackfillCreator.kt b/service/src/main/kotlin/app/cash/backfila/BackfillCreator.kt index 8b29ae1b1..8718dd877 100644 --- a/service/src/main/kotlin/app/cash/backfila/BackfillCreator.kt +++ b/service/src/main/kotlin/app/cash/backfila/BackfillCreator.kt @@ -126,24 +126,38 @@ class BackfillCreator @Inject constructor( ) } catch (e: Exception) { logger.info(e) { "PrepareBackfill on `$service` failed" } - throw BadRequestException("PrepareBackfill on `$service` failed: ${e.message}", e) + throw BadRequestException( + "PrepareBackfill on `$service` failed: ${e.message}. " + + "connectionData: ${client.connectionLogData()}", + e, + ) } prepareBackfillResponse.error_message?.let { - throw BadRequestException("PrepareBackfill on `$service` failed: $it") + throw BadRequestException( + "PrepareBackfill on `$service` failed: $it. " + + "connectionData: ${client.connectionLogData()}", + ) } val partitions = prepareBackfillResponse.partitions if (partitions.isEmpty()) { - throw BadRequestException("PrepareBackfill returned no partitions") + throw BadRequestException( + "PrepareBackfill returned no partitions. " + + "connectionData: ${client.connectionLogData()}", + ) } if (partitions.any { it.partition_name == null }) { - throw BadRequestException("PrepareBackfill returned unnamed partitions") + throw BadRequestException( + "PrepareBackfill returned unnamed partitions. " + + "connectionData: ${client.connectionLogData()}", + ) } if (partitions.distinctBy { it.partition_name }.size != partitions.size) { throw BadRequestException( "PrepareBackfill did not return distinct partition names:" + - " ${partitions.map { it.partition_name }}", + " ${partitions.map { it.partition_name }}. " + + "connectionData: ${client.connectionLogData()}", ) } diff --git a/service/src/main/kotlin/app/cash/backfila/client/BackfilaClientServiceClient.kt b/service/src/main/kotlin/app/cash/backfila/client/BackfilaClientServiceClient.kt index d8b79fed9..863e6fc47 100644 --- a/service/src/main/kotlin/app/cash/backfila/client/BackfilaClientServiceClient.kt +++ b/service/src/main/kotlin/app/cash/backfila/client/BackfilaClientServiceClient.kt @@ -13,4 +13,9 @@ interface BackfilaClientServiceClient { suspend fun getNextBatchRange(request: GetNextBatchRangeRequest): GetNextBatchRangeResponse suspend fun runBatch(request: RunBatchRequest): RunBatchResponse + + /** + * Gives us a way to probe for connection information when something fails to help with debugging. + */ + fun connectionLogData(): String } diff --git a/service/src/main/kotlin/app/cash/backfila/client/EnvoyClientServiceClient.kt b/service/src/main/kotlin/app/cash/backfila/client/EnvoyClientServiceClient.kt index c508904c2..6322bf8d3 100644 --- a/service/src/main/kotlin/app/cash/backfila/client/EnvoyClientServiceClient.kt +++ b/service/src/main/kotlin/app/cash/backfila/client/EnvoyClientServiceClient.kt @@ -11,6 +11,7 @@ import retrofit2.Response internal class EnvoyClientServiceClient internal constructor( private val api: EnvoyClientServiceApi, + private val connectionLogData: String, ) : BackfilaClientServiceClient { override fun prepareBackfill(request: PrepareBackfillRequest): PrepareBackfillResponse { @@ -26,6 +27,8 @@ internal class EnvoyClientServiceClient internal constructor( return api.runBatch(request) } + override fun connectionLogData() = connectionLogData + private fun Response.getOrThrow(): T { if (!this.isSuccessful) { throw HttpException(this) diff --git a/service/src/main/kotlin/app/cash/backfila/client/EnvoyClientServiceClientProvider.kt b/service/src/main/kotlin/app/cash/backfila/client/EnvoyClientServiceClientProvider.kt index 2b1d8cbb9..ef765913e 100644 --- a/service/src/main/kotlin/app/cash/backfila/client/EnvoyClientServiceClientProvider.kt +++ b/service/src/main/kotlin/app/cash/backfila/client/EnvoyClientServiceClientProvider.kt @@ -71,7 +71,10 @@ class EnvoyClientServiceClientProvider @Inject constructor( .addCallAdapterFactory(GuavaCallAdapterFactory.create()) .build() val api = retrofit.create(EnvoyClientServiceApi::class.java) - return EnvoyClientServiceClient(api) + val logData = "envoyConfig: ${httpClientEndpointConfig.envoy}, " + + "url: ${httpClientEndpointConfig.url}, " + + "headers: $headers" + return EnvoyClientServiceClient(api, logData) } private fun adapter() = moshi.adapter() diff --git a/service/src/main/kotlin/app/cash/backfila/client/HttpClientServiceClient.kt b/service/src/main/kotlin/app/cash/backfila/client/HttpClientServiceClient.kt index a2b765eb7..2a9e64eaf 100644 --- a/service/src/main/kotlin/app/cash/backfila/client/HttpClientServiceClient.kt +++ b/service/src/main/kotlin/app/cash/backfila/client/HttpClientServiceClient.kt @@ -11,6 +11,7 @@ import retrofit2.Response internal class HttpClientServiceClient internal constructor( private val api: HttpClientServiceApi, + private val connectionLogData: String, ) : BackfilaClientServiceClient { override fun prepareBackfill(request: PrepareBackfillRequest): PrepareBackfillResponse { @@ -26,6 +27,8 @@ internal class HttpClientServiceClient internal constructor( return api.runBatch(request) } + override fun connectionLogData() = connectionLogData + private fun Response.getOrThrow(): T { if (!this.isSuccessful) { throw HttpException(this) diff --git a/service/src/main/kotlin/app/cash/backfila/client/HttpClientServiceClientProvider.kt b/service/src/main/kotlin/app/cash/backfila/client/HttpClientServiceClientProvider.kt index ce5a31fb4..2a13219e0 100644 --- a/service/src/main/kotlin/app/cash/backfila/client/HttpClientServiceClientProvider.kt +++ b/service/src/main/kotlin/app/cash/backfila/client/HttpClientServiceClientProvider.kt @@ -67,7 +67,9 @@ class HttpClientServiceClientProvider @Inject constructor( .addCallAdapterFactory(GuavaCallAdapterFactory.create()) .build() val api = retrofit.create(HttpClientServiceApi::class.java) - return HttpClientServiceClient(api) + val logData = "url: ${httpClientEndpointConfig.url}, " + + "headers: $headers" + return HttpClientServiceClient(api, logData) } private fun adapter() = moshi.adapter() diff --git a/service/src/test/kotlin/app/cash/backfila/client/FakeBackfilaClientServiceClient.kt b/service/src/test/kotlin/app/cash/backfila/client/FakeBackfilaClientServiceClient.kt index 3b92d2ddb..a5e33a88d 100644 --- a/service/src/test/kotlin/app/cash/backfila/client/FakeBackfilaClientServiceClient.kt +++ b/service/src/test/kotlin/app/cash/backfila/client/FakeBackfilaClientServiceClient.kt @@ -93,4 +93,6 @@ class FakeBackfilaClientServiceClient @Inject constructor() : BackfilaClientServ runBatchRequests.send(request) return runBatchResponses.receive().getOrThrow() } + + override fun connectionLogData() = "FakeBackfilaClientServiceClient so no connection" } diff --git a/service/src/test/kotlin/app/cash/backfila/development/BackfilaDevelopmentService.kt b/service/src/test/kotlin/app/cash/backfila/development/BackfilaDevelopmentService.kt index 88ccfa7f1..348229584 100644 --- a/service/src/test/kotlin/app/cash/backfila/development/BackfilaDevelopmentService.kt +++ b/service/src/test/kotlin/app/cash/backfila/development/BackfilaDevelopmentService.kt @@ -90,6 +90,8 @@ fun main(args: Array) { override suspend fun runBatch(request: RunBatchRequest): RunBatchResponse { TODO("Not yet implemented") } + + override fun connectionLogData() = "Fake development service client" } } }, From 4409dea987c4dcfaddc00daf374ca6b655d64ab3 Mon Sep 17 00:00:00 2001 From: Michael Pawliszyn Date: Thu, 21 Dec 2023 11:28:28 -0500 Subject: [PATCH 2/3] Adding a Clumsy meal backfill to help with error testing. --- .../finedining/ClumsyMealsBackfill.kt | 46 +++++++++++++++++++ .../finedining/FineDiningServiceModule.kt | 1 + .../finedining/SlowMealsBackfill.kt | 4 +- 3 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 service/src/test/kotlin/app/cash/backfila/development/finedining/ClumsyMealsBackfill.kt diff --git a/service/src/test/kotlin/app/cash/backfila/development/finedining/ClumsyMealsBackfill.kt b/service/src/test/kotlin/app/cash/backfila/development/finedining/ClumsyMealsBackfill.kt new file mode 100644 index 000000000..9e307e547 --- /dev/null +++ b/service/src/test/kotlin/app/cash/backfila/development/finedining/ClumsyMealsBackfill.kt @@ -0,0 +1,46 @@ +package app.cash.backfila.development.finedining + +import app.cash.backfila.client.BackfillConfig +import app.cash.backfila.client.Description +import app.cash.backfila.client.PrepareBackfillConfig +import app.cash.backfila.client.stat.StaticDatasourceBackfill +import javax.inject.Inject +import wisp.logging.getLogger + +class ClumsyMealsBackfill @Inject constructor() : StaticDatasourceBackfill() { + var servedPlates = 0 // Keeps track of the number of successfully served plates. + var brokenPlatesSoFar = 0 // When plates break it keeps track of how many have broken so far. + override fun validate(config: PrepareBackfillConfig) { + if (config.parameters.blockedEntrance) { + throw RuntimeException("Entrance is BLOCKED. No customers can be served!") + } + } + + override fun runOne(item: String, config: BackfillConfig) { + Thread.sleep(100L) // Sleep for half a second + // We potentially break on every 25th plate. + if (servedPlates % 25 == 0 && brokenPlatesSoFar < config.parameters.brokenPlates) { + brokenPlatesSoFar++ + logger.info { "Broke a plate!" } + throw RuntimeException("Failed to serve: Broke a plate!!!") + } + logger.info { "Poorly served $item" } + servedPlates++ + brokenPlatesSoFar = 0 + } + + @Description("Adaptable clumsiness.") + data class ClumsyMealsAttributes( + @Description("Whether the entrance is blocked or not. Can be used to test Prepare failures.") + val blockedEntrance: Boolean = true, + @Description("How many plates break before a successful plate is served. Can be used to test retry logic.") + val brokenPlates: Int = 1, + ) + + // Generate the meal place settings for the backfill. + override val staticDatasource: List = (1..5000).map { i -> "place setting $i" } + + companion object { + private val logger = getLogger() + } +} diff --git a/service/src/test/kotlin/app/cash/backfila/development/finedining/FineDiningServiceModule.kt b/service/src/test/kotlin/app/cash/backfila/development/finedining/FineDiningServiceModule.kt index 034ac41b9..f8341c310 100644 --- a/service/src/test/kotlin/app/cash/backfila/development/finedining/FineDiningServiceModule.kt +++ b/service/src/test/kotlin/app/cash/backfila/development/finedining/FineDiningServiceModule.kt @@ -19,5 +19,6 @@ internal class FineDiningServiceModule : KAbstractModule() { ), ) install(StaticDatasourceBackfillModule.create()) + install(StaticDatasourceBackfillModule.create()) } } diff --git a/service/src/test/kotlin/app/cash/backfila/development/finedining/SlowMealsBackfill.kt b/service/src/test/kotlin/app/cash/backfila/development/finedining/SlowMealsBackfill.kt index d77ab0d07..8782b69e0 100644 --- a/service/src/test/kotlin/app/cash/backfila/development/finedining/SlowMealsBackfill.kt +++ b/service/src/test/kotlin/app/cash/backfila/development/finedining/SlowMealsBackfill.kt @@ -12,11 +12,11 @@ class SlowMealsBackfill @Inject constructor() : StaticDatasourceBackfill = (1..1000).map { i -> "place setting $i" } + override val staticDatasource: List = (1..5000).map { i -> "place setting $i" } companion object { private val logger = getLogger() From e7dab4f4e1f2f9904add1b5dc99268c7debfa339 Mon Sep 17 00:00:00 2001 From: Michael Pawliszyn Date: Thu, 21 Dec 2023 11:45:31 -0500 Subject: [PATCH 3/3] Adding test update to show connection data. --- .../app/cash/backfila/actions/CreateBackfillActionTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/src/test/kotlin/app/cash/backfila/actions/CreateBackfillActionTest.kt b/service/src/test/kotlin/app/cash/backfila/actions/CreateBackfillActionTest.kt index 63a8c6eb9..c438036e3 100644 --- a/service/src/test/kotlin/app/cash/backfila/actions/CreateBackfillActionTest.kt +++ b/service/src/test/kotlin/app/cash/backfila/actions/CreateBackfillActionTest.kt @@ -406,7 +406,7 @@ class CreateBackfillActionTest { .build(), ) }.isInstanceOf(BadRequestException::class.java) - .hasMessage("PrepareBackfill on `deep-fryer` failed: We're out of chicken") + .hasMessage("PrepareBackfill on `deep-fryer` failed: We're out of chicken. connectionData: FakeBackfilaClientServiceClient so no connection") transacter.transaction { session -> val runs = queryFactory.newQuery().list(session)