From 7a39f92927ff3ffbe75b57fa13607221bc8d157c Mon Sep 17 00:00:00 2001 From: John Oliver <1615532+johnoliver@users.noreply.github.com> Date: Mon, 17 Feb 2020 23:20:50 +0000 Subject: [PATCH 1/3] Add printing debug stats to narrow down missing stats issues (#111) --- .../api/v3/DownloadStatsInterface.kt | 10 ++++-- .../api/v3/stats/DockerStatsInterface.kt | 2 +- .../api/v3/stats/StatsInterface.kt | 35 ++++++++++++++----- .../src/main/resources/logback.xml | 4 +-- 4 files changed, 37 insertions(+), 14 deletions(-) diff --git a/adoptopenjdk-api-v3-persistance/src/main/kotlin/net/adoptopenjdk/api/v3/DownloadStatsInterface.kt b/adoptopenjdk-api-v3-persistance/src/main/kotlin/net/adoptopenjdk/api/v3/DownloadStatsInterface.kt index 30d244a7..e2427093 100644 --- a/adoptopenjdk-api-v3-persistance/src/main/kotlin/net/adoptopenjdk/api/v3/DownloadStatsInterface.kt +++ b/adoptopenjdk-api-v3-persistance/src/main/kotlin/net/adoptopenjdk/api/v3/DownloadStatsInterface.kt @@ -1,4 +1,4 @@ -package net.adoptopenjdk.api.v3; +package net.adoptopenjdk.api.v3 import net.adoptopenjdk.api.v3.dataSources.APIDataStore import net.adoptopenjdk.api.v3.dataSources.ApiPersistenceFactory @@ -21,7 +21,13 @@ class StatEntry( class DownloadStatsInterface { private val dataStore = ApiPersistenceFactory.get() - suspend fun getTrackingStats(days: Int?, from: ZonedDateTime?, to: ZonedDateTime?, source: StatsSource?, featureVersion: Int?, dockerRepo: String?): List { + suspend fun getTrackingStats( + days: Int? = null, + from: ZonedDateTime? = null, + to: ZonedDateTime? = null, + source: StatsSource? = null, + featureVersion: Int? = null, + dockerRepo: String? = null): List { //need +1 as for a diff you need num days +1 from db val daysSince = (days ?: 30) + 1 diff --git a/adoptopenjdk-api-v3-updater/src/main/kotlin/net/adoptopenjdk/api/v3/stats/DockerStatsInterface.kt b/adoptopenjdk-api-v3-updater/src/main/kotlin/net/adoptopenjdk/api/v3/stats/DockerStatsInterface.kt index f1aeb413..dfc165d5 100644 --- a/adoptopenjdk-api-v3-updater/src/main/kotlin/net/adoptopenjdk/api/v3/stats/DockerStatsInterface.kt +++ b/adoptopenjdk-api-v3-updater/src/main/kotlin/net/adoptopenjdk/api/v3/stats/DockerStatsInterface.kt @@ -67,7 +67,7 @@ class DockerStatsInterface { private fun getStatsForUrl(url: String): JsonObject { return runBlocking { - val stats = UpdaterHtmlClientFactory.client.get(url); + val stats = UpdaterHtmlClientFactory.client.get(url) if (stats == null) { throw Exception("Stats not returned") } diff --git a/adoptopenjdk-api-v3-updater/src/main/kotlin/net/adoptopenjdk/api/v3/stats/StatsInterface.kt b/adoptopenjdk-api-v3-updater/src/main/kotlin/net/adoptopenjdk/api/v3/stats/StatsInterface.kt index 4b2a550c..e5b14861 100644 --- a/adoptopenjdk-api-v3-updater/src/main/kotlin/net/adoptopenjdk/api/v3/stats/StatsInterface.kt +++ b/adoptopenjdk-api-v3-updater/src/main/kotlin/net/adoptopenjdk/api/v3/stats/StatsInterface.kt @@ -1,6 +1,5 @@ package net.adoptopenjdk.api.v3.stats -import kotlinx.coroutines.runBlocking import net.adoptopenjdk.api.v3.DownloadStatsInterface import net.adoptopenjdk.api.v3.dataSources.ApiPersistenceFactory import net.adoptopenjdk.api.v3.dataSources.models.AdoptRepos @@ -8,6 +7,7 @@ import net.adoptopenjdk.api.v3.dataSources.persitence.ApiPersistence import net.adoptopenjdk.api.v3.models.StatsSource import org.slf4j.LoggerFactory import java.time.ZoneOffset +import java.time.ZonedDateTime class StatsInterface { @@ -30,17 +30,36 @@ class StatsInterface { } private suspend fun removeBadDownloadStats() { - val tracking = downloadStatsInterface.getTrackingStats(10, null, null, StatsSource.all, null, null) + val tracking = downloadStatsInterface.getTrackingStats(days = 10, source = StatsSource.all) tracking .filter { it.daily <= 0 } .forEach { entry -> - val start = entry.date.toLocalDate().atStartOfDay() - val end = entry.date.toLocalDate().plusDays(1).atStartOfDay() - LOGGER.info("Removing bad stats between $start $end") - runBlocking { - database.removeStatsBetween(start.atZone(ZoneOffset.UTC), end.atZone(ZoneOffset.UTC)) - } + val start = entry.date.toLocalDate().atStartOfDay().atZone(ZoneOffset.UTC) + val end = entry.date.toLocalDate().plusDays(1).atStartOfDay().atZone(ZoneOffset.UTC) + + printStatDebugInfo(start, end) + database.removeStatsBetween(start, end) } } + private suspend fun printStatDebugInfo(start: ZonedDateTime, end: ZonedDateTime) { + LOGGER.info("Removing bad stats between $start $end") + printStats(start, end) + LOGGER.info("Day before: $start $end") + printStats(start.minusDays(1), end.minusDays(1)) + } + + private suspend fun printStats(start: ZonedDateTime, end: ZonedDateTime) { + database + .getGithubStats(start, end) + .forEach { stat -> + LOGGER.info("github stat: ${stat.feature_version} ${stat.downloads}") + } + + database + .getDockerStats(start, end) + .forEach { stat -> + LOGGER.info("docker stat: ${stat.repo} ${stat.pulls}") + } + } } \ No newline at end of file diff --git a/adoptopenjdk-api-v3-updater/src/main/resources/logback.xml b/adoptopenjdk-api-v3-updater/src/main/resources/logback.xml index fbd87183..e946735a 100644 --- a/adoptopenjdk-api-v3-updater/src/main/resources/logback.xml +++ b/adoptopenjdk-api-v3-updater/src/main/resources/logback.xml @@ -1,4 +1,5 @@ + @@ -6,16 +7,13 @@ - /tmp/updater.log - %date %level [%thread] %logger{10} [%file:%line]%n%msg%n - From dff4852fd410b7b95bb6cc0125107f009c728459 Mon Sep 17 00:00:00 2001 From: John Oliver <1615532+johnoliver@users.noreply.github.com> Date: Fri, 21 Feb 2020 06:27:32 +0000 Subject: [PATCH 2/3] If there are multiple stat entries for a single day, take the last (#114) --- .../api/v3/DownloadStatsInterface.kt | 24 +++++++++- .../api/DockerStatsInterfaceTest.kt | 46 +++++++++++++++++++ 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/adoptopenjdk-api-v3-persistance/src/main/kotlin/net/adoptopenjdk/api/v3/DownloadStatsInterface.kt b/adoptopenjdk-api-v3-persistance/src/main/kotlin/net/adoptopenjdk/api/v3/DownloadStatsInterface.kt index e2427093..eb2c5cc6 100644 --- a/adoptopenjdk-api-v3-persistance/src/main/kotlin/net/adoptopenjdk/api/v3/DownloadStatsInterface.kt +++ b/adoptopenjdk-api-v3-persistance/src/main/kotlin/net/adoptopenjdk/api/v3/DownloadStatsInterface.kt @@ -2,6 +2,7 @@ package net.adoptopenjdk.api.v3 import net.adoptopenjdk.api.v3.dataSources.APIDataStore import net.adoptopenjdk.api.v3.dataSources.ApiPersistenceFactory +import net.adoptopenjdk.api.v3.dataSources.persitence.ApiPersistence import net.adoptopenjdk.api.v3.models.DbStatsEntry import net.adoptopenjdk.api.v3.models.DownloadDiff import net.adoptopenjdk.api.v3.models.DownloadStats @@ -18,8 +19,9 @@ class StatEntry( val count: Long ) -class DownloadStatsInterface { - private val dataStore = ApiPersistenceFactory.get() +class DownloadStatsInterface( + private val dataStore: ApiPersistence = ApiPersistenceFactory.get() +) { suspend fun getTrackingStats( days: Int? = null, @@ -90,7 +92,16 @@ class DownloadStatsInterface { return sumDailyStats( dataStore .getGithubStats(start, end) + .groupBy { it.date.toLocalDate() } + .flatMap { grouped -> + grouped.value + .groupBy { it.feature_version } + .map { featureVersionsForDay -> + featureVersionsForDay.value.maxBy { it.date }!! + } + } .filter { featureVersion == null || it.feature_version == featureVersion } + .sortedBy { it.date } ) } @@ -98,7 +109,16 @@ class DownloadStatsInterface { return sumDailyStats( dataStore .getDockerStats(start, end) + .groupBy { it.date.toLocalDate() } + .flatMap { grouped -> + grouped.value + .groupBy { it.repo } + .map { repoForDay -> + repoForDay.value.maxBy { it.date }!! + } + } .filter { dockerRepo == null || it.repo == dockerRepo } + .sortedBy { it.date } ) } diff --git a/adoptopenjdk-api-v3-updater/src/test/kotlin/net/adoptopenjdk/api/DockerStatsInterfaceTest.kt b/adoptopenjdk-api-v3-updater/src/test/kotlin/net/adoptopenjdk/api/DockerStatsInterfaceTest.kt index fe23880e..2d9cd3ce 100644 --- a/adoptopenjdk-api-v3-updater/src/test/kotlin/net/adoptopenjdk/api/DockerStatsInterfaceTest.kt +++ b/adoptopenjdk-api-v3-updater/src/test/kotlin/net/adoptopenjdk/api/DockerStatsInterfaceTest.kt @@ -1,13 +1,22 @@ package net.adoptopenjdk.api +import io.mockk.coEvery +import io.mockk.mockk import kotlinx.coroutines.runBlocking +import net.adoptopenjdk.api.v3.DownloadStatsInterface import net.adoptopenjdk.api.v3.dataSources.ApiPersistenceFactory import net.adoptopenjdk.api.v3.dataSources.DefaultUpdaterHtmlClient import net.adoptopenjdk.api.v3.dataSources.UpdaterHtmlClientFactory +import net.adoptopenjdk.api.v3.dataSources.persitence.ApiPersistence +import net.adoptopenjdk.api.v3.models.DockerDownloadStatsDbEntry +import net.adoptopenjdk.api.v3.models.GithubDownloadStatsDbEntry +import net.adoptopenjdk.api.v3.models.StatsSource import net.adoptopenjdk.api.v3.stats.DockerStatsInterface import org.junit.Assert import org.junit.jupiter.api.BeforeAll import org.junit.jupiter.api.Test +import java.time.ZonedDateTime +import kotlin.test.assertEquals class DockerStatsInterfaceTest { @@ -30,5 +39,42 @@ class DockerStatsInterfaceTest { Assert.assertTrue(stats.size > 0) } } + + + @Test + fun onlyLastStatEntryPerDayIsRead() { + runBlocking { + val apiPersistanceMock = mockk() + coEvery { apiPersistanceMock.getGithubStats(any(), any()) } returns listOf( + GithubDownloadStatsDbEntry(ZonedDateTime.now().minusMinutes(10), 100, 8), + GithubDownloadStatsDbEntry(ZonedDateTime.now().minusMinutes(20), 90, 8), + GithubDownloadStatsDbEntry(ZonedDateTime.now().minusDays(1), 80, 8), + GithubDownloadStatsDbEntry(ZonedDateTime.now().minusMinutes(15), 20, 9) + ) + + coEvery { apiPersistanceMock.getDockerStats(any(), any()) } returns listOf( + DockerDownloadStatsDbEntry(ZonedDateTime.now().minusMinutes(10), 100, "a-stats-repo"), + DockerDownloadStatsDbEntry(ZonedDateTime.now().minusMinutes(20), 90, "a-stats-repo"), + DockerDownloadStatsDbEntry(ZonedDateTime.now().minusDays(1), 80, "a-stats-repo"), + DockerDownloadStatsDbEntry(ZonedDateTime.now().minusMinutes(15), 20, "a-different-stats-repo") + ) + + val downloadStatsInterface = DownloadStatsInterface(apiPersistanceMock) + + var stats = downloadStatsInterface.getTrackingStats(10, source = StatsSource.github, featureVersion = 8) + assertEquals(100, stats[0].total) + stats = downloadStatsInterface.getTrackingStats(10, source = StatsSource.github) + assertEquals(120, stats[0].total) + + stats = downloadStatsInterface.getTrackingStats(10, source = StatsSource.dockerhub, dockerRepo = "a-stats-repo") + assertEquals(100, stats[0].total) + stats = downloadStatsInterface.getTrackingStats(10, source = StatsSource.dockerhub) + assertEquals(120, stats[0].total) + + stats = downloadStatsInterface.getTrackingStats(10) + assertEquals(240, stats[0].total) + + } + } } From 129e0dc605d70d8a597f24536cbac62a492112c9 Mon Sep 17 00:00:00 2001 From: John Oliver <1615532+johnoliver@users.noreply.github.com> Date: Wed, 11 Mar 2020 13:45:23 +0000 Subject: [PATCH 3/3] add timeout to Http requests --- .../adoptopenjdk/api/v3/HttpClientFactory.kt | 11 +++++++++++ .../api/v3/dataSources/UpdaterHtmlClient.kt | 17 ++++++++++++----- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/adoptopenjdk-api-v3-updater/src/main/kotlin/net/adoptopenjdk/api/v3/HttpClientFactory.kt b/adoptopenjdk-api-v3-updater/src/main/kotlin/net/adoptopenjdk/api/v3/HttpClientFactory.kt index f681b944..65001e5b 100644 --- a/adoptopenjdk-api-v3-updater/src/main/kotlin/net/adoptopenjdk/api/v3/HttpClientFactory.kt +++ b/adoptopenjdk-api-v3-updater/src/main/kotlin/net/adoptopenjdk/api/v3/HttpClientFactory.kt @@ -3,6 +3,7 @@ package net.adoptopenjdk.api.v3 import org.apache.http.HttpRequest import org.apache.http.HttpResponse import org.apache.http.client.RedirectStrategy +import org.apache.http.client.config.RequestConfig import org.apache.http.client.methods.HttpUriRequest import org.apache.http.impl.NoConnectionReuseStrategy import org.apache.http.impl.nio.client.HttpAsyncClients @@ -13,10 +14,19 @@ object HttpClientFactory { private val client: HttpAsyncClient private val noRedirect: HttpAsyncClient + val REQUEST_CONFIG = RequestConfig + .copy(RequestConfig.DEFAULT) + .setConnectTimeout(5000) + .setSocketTimeout(5000) + .setConnectionRequestTimeout(5000) + .build()!! + init { + val client = HttpAsyncClients.custom() .setConnectionReuseStrategy(NoConnectionReuseStrategy()) .disableCookieManagement() + .setDefaultRequestConfig(REQUEST_CONFIG) .build() client.start() this.client = client @@ -33,6 +43,7 @@ object HttpClientFactory { }) .setConnectionReuseStrategy(NoConnectionReuseStrategy()) + .setDefaultRequestConfig(REQUEST_CONFIG) .build() noRedirect.start() this.noRedirect = noRedirect diff --git a/adoptopenjdk-api-v3-updater/src/main/kotlin/net/adoptopenjdk/api/v3/dataSources/UpdaterHtmlClient.kt b/adoptopenjdk-api-v3-updater/src/main/kotlin/net/adoptopenjdk/api/v3/dataSources/UpdaterHtmlClient.kt index 85899888..eba442d9 100644 --- a/adoptopenjdk-api-v3-updater/src/main/kotlin/net/adoptopenjdk/api/v3/dataSources/UpdaterHtmlClient.kt +++ b/adoptopenjdk-api-v3-updater/src/main/kotlin/net/adoptopenjdk/api/v3/dataSources/UpdaterHtmlClient.kt @@ -1,11 +1,12 @@ package net.adoptopenjdk.api.v3.dataSources import kotlinx.coroutines.delay +import kotlinx.coroutines.withTimeout import net.adoptopenjdk.api.v3.HttpClientFactory import net.adoptopenjdk.api.v3.dataSources.github.GithubAuth import org.apache.commons.io.IOUtils import org.apache.http.HttpResponse -import org.apache.http.client.methods.HttpGet +import org.apache.http.client.methods.RequestBuilder import org.apache.http.concurrent.FutureCallback import org.slf4j.LoggerFactory import java.io.StringWriter @@ -37,7 +38,7 @@ class DefaultUpdaterHtmlClient : UpdaterHtmlClient { @JvmStatic private val LOGGER = LoggerFactory.getLogger(this::class.java) private val TOKEN: String? = GithubAuth.readToken() - + private const val REQUEST_TIMEOUT = 12000L fun extractBody(response: HttpResponse?): String? { if (response == null) { @@ -98,7 +99,10 @@ class DefaultUpdaterHtmlClient : UpdaterHtmlClient { private fun getData(urlRequest: UrlRequest, continuation: Continuation) { try { val url = URL(urlRequest.url) - val request = HttpGet(url.toURI()) + val request = RequestBuilder + .get(url.toURI()) + .setConfig(HttpClientFactory.REQUEST_CONFIG) + .build() if (urlRequest.lastModified != null) { request.addHeader("If-Modified-Since", urlRequest.lastModified) @@ -116,6 +120,7 @@ class DefaultUpdaterHtmlClient : UpdaterHtmlClient { } client.execute(request, ResponseHandler(this, continuation, urlRequest)) + } catch (e: Exception) { continuation.resumeWith(Result.failure(e)) } @@ -127,8 +132,10 @@ class DefaultUpdaterHtmlClient : UpdaterHtmlClient { for (retryCount in 1..10) { try { LOGGER.info("Getting ${request.url} ${request.lastModified}") - val response: HttpResponse = suspendCoroutine { continuation -> - getData(request, continuation) + val response: HttpResponse = withTimeout(REQUEST_TIMEOUT) { + suspendCoroutine { continuation -> + getData(request, continuation) + } } LOGGER.info("Got ${request.url}") return response