From 909bda14000957e380e4e0414948693d122ee3e8 Mon Sep 17 00:00:00 2001 From: munishchouhan Date: Mon, 15 Jul 2024 14:26:07 +0200 Subject: [PATCH 01/12] added deleteAllMatchingEntries Signed-off-by: munishchouhan --- .../counter/AbstractCounterStore.groovy | 5 +++++ .../wave/service/counter/CounterStore.groovy | 6 +++++ .../counter/impl/CounterProvider.groovy | 8 +++++++ .../counter/impl/LocalCounterProvider.groovy | 12 ++++++++++ .../counter/impl/RedisCounterProvider.groovy | 13 +++++++++++ .../impl/LocalCounterProviderTest.groovy | 22 +++++++++++++++++++ .../impl/RedisCounterProviderTest.groovy | 22 +++++++++++++++++++ 7 files changed, 88 insertions(+) diff --git a/src/main/groovy/io/seqera/wave/service/counter/AbstractCounterStore.groovy b/src/main/groovy/io/seqera/wave/service/counter/AbstractCounterStore.groovy index 3e51f771c..232fb5990 100644 --- a/src/main/groovy/io/seqera/wave/service/counter/AbstractCounterStore.groovy +++ b/src/main/groovy/io/seqera/wave/service/counter/AbstractCounterStore.groovy @@ -55,4 +55,9 @@ abstract class AbstractCounterStore implements CounterStore { Map getAllMatchingEntries(String pattern) { provider.getAllMatchingEntries(getPrefix(), pattern) } + + @Override + void deleteAllMatchingEntries(String pattern) { + provider.getAllMatchingEntries(getPrefix(), pattern) + } } diff --git a/src/main/groovy/io/seqera/wave/service/counter/CounterStore.groovy b/src/main/groovy/io/seqera/wave/service/counter/CounterStore.groovy index 41e64c4e0..c91b46067 100644 --- a/src/main/groovy/io/seqera/wave/service/counter/CounterStore.groovy +++ b/src/main/groovy/io/seqera/wave/service/counter/CounterStore.groovy @@ -34,4 +34,10 @@ interface CounterStore { * @return all the entries whose field matches 'pattern' */ Map getAllMatchingEntries(String pattern) + + /** + * @param pattern + * @return void + */ + void deleteAllMatchingEntries(String pattern) } diff --git a/src/main/groovy/io/seqera/wave/service/counter/impl/CounterProvider.groovy b/src/main/groovy/io/seqera/wave/service/counter/impl/CounterProvider.groovy index c13f8f4e2..64823b435 100644 --- a/src/main/groovy/io/seqera/wave/service/counter/impl/CounterProvider.groovy +++ b/src/main/groovy/io/seqera/wave/service/counter/impl/CounterProvider.groovy @@ -35,4 +35,12 @@ interface CounterProvider { * @return all the entries whose field matches 'pattern' */ Map getAllMatchingEntries(String key, String pattern) + + /** + * + * @param key + * @param pattern + * @return void + */ + void deleteAllMatchingEntries(String key, String pattern) } diff --git a/src/main/groovy/io/seqera/wave/service/counter/impl/LocalCounterProvider.groovy b/src/main/groovy/io/seqera/wave/service/counter/impl/LocalCounterProvider.groovy index c7ea99ac6..b94c07cff 100644 --- a/src/main/groovy/io/seqera/wave/service/counter/impl/LocalCounterProvider.groovy +++ b/src/main/groovy/io/seqera/wave/service/counter/impl/LocalCounterProvider.groovy @@ -61,4 +61,16 @@ class LocalCounterProvider implements CounterProvider { } return result } + + @Override + void deleteAllMatchingEntries(String key, String pattern) { + Pattern compiledPattern = Pattern.compile(pattern.replace('*', '.*')) + Map keyStore = store.get(key) + if (keyStore){ + def matchingPairs = keyStore.findAll {entry -> compiledPattern.matcher(entry.key).matches()} + matchingPairs.each { k, v -> + keyStore.remove(k) + } + } + } } diff --git a/src/main/groovy/io/seqera/wave/service/counter/impl/RedisCounterProvider.groovy b/src/main/groovy/io/seqera/wave/service/counter/impl/RedisCounterProvider.groovy index 5e28467bf..c0e9bb1d9 100644 --- a/src/main/groovy/io/seqera/wave/service/counter/impl/RedisCounterProvider.groovy +++ b/src/main/groovy/io/seqera/wave/service/counter/impl/RedisCounterProvider.groovy @@ -70,4 +70,17 @@ class RedisCounterProvider implements CounterProvider { return result } } + + @Override + void deleteAllMatchingEntries(String key, String pattern) { + try(Jedis conn=pool.getResource() ) { + final scanResult = conn.hscan(key, "0", new ScanParams().match(pattern).count(hscanCount)) + if( !scanResult ) + return + for(String entry : scanResult.result) { + final parts = entry.tokenize('=') + conn.hdel(key, parts[0]) + } + } + } } diff --git a/src/test/groovy/io/seqera/wave/service/counter/impl/LocalCounterProviderTest.groovy b/src/test/groovy/io/seqera/wave/service/counter/impl/LocalCounterProviderTest.groovy index 1787b61f5..54d6ec088 100644 --- a/src/test/groovy/io/seqera/wave/service/counter/impl/LocalCounterProviderTest.groovy +++ b/src/test/groovy/io/seqera/wave/service/counter/impl/LocalCounterProviderTest.groovy @@ -73,4 +73,26 @@ class LocalCounterProviderTest extends Specification { ['pulls/o/abc.com.au/d/2024-05-30':1] } + def 'should delete records with given pattern' () { + when: + localCounterProvider.inc('metrics/v1', 'builds/o/foo.com', 1) + localCounterProvider.inc('metrics/v1', 'builds/o/bar.io', 1) + localCounterProvider.inc('metrics/v1', 'builds/o/abc.org', 2) + localCounterProvider.inc('metrics/v1', 'pulls/o/foo.it', 1) + localCounterProvider.inc('metrics/v1', 'pulls/o/bar.es', 2) + localCounterProvider.inc('metrics/v1', 'pulls/o/abc.in', 3) + localCounterProvider.inc('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-15', 1) + localCounterProvider.inc('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-14', 1) + localCounterProvider.inc('metrics/v1', 'builds/o/abc.com.au/d/2024-07-14', 1) + localCounterProvider.inc('metrics/v1', 'builds/o/abc.com.au/d/2024-07-14', 1) + + then:'delete all pull counter keys' + localCounterProvider.deleteAllMatchingEntries('metrics/v1', 'pulls/o/*') + localCounterProvider.getAllMatchingEntries('metrics/v1', 'pulls/o/*') == [:] + localCounterProvider.getAllMatchingEntries('metrics/v1', 'pulls/o/*/d/2024-05-30') == [:] + and:'delete only build per specific date counter keys' + localCounterProvider.deleteAllMatchingEntries('metrics/v1', 'builds/o/abc.com.au/d/2024-07-14') + localCounterProvider.getAllMatchingEntries('metrics/v1', 'builds/o/*') == ['builds/o/foo.com':1, 'builds/o/bar.io':1, 'builds/o/abc.org':2] + } + } diff --git a/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy b/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy index d592171a9..5a7d56970 100644 --- a/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy +++ b/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy @@ -82,4 +82,26 @@ class RedisCounterProviderTest extends Specification implements RedisTestContain redisCounterProvider.getAllMatchingEntries('metrics/v1', 'pulls/o/*/d/2024-05-30') == ['pulls/o/abc.com.au/d/2024-05-30':1] } + + def 'should delete records with given pattern' () { + when: + redisCounterProvider.inc('metrics/v1', 'builds/o/foo.com', 1) + redisCounterProvider.inc('metrics/v1', 'builds/o/bar.io', 1) + redisCounterProvider.inc('metrics/v1', 'builds/o/abc.org', 2) + redisCounterProvider.inc('metrics/v1', 'pulls/o/foo.it', 1) + redisCounterProvider.inc('metrics/v1', 'pulls/o/bar.es', 2) + redisCounterProvider.inc('metrics/v1', 'pulls/o/abc.in', 3) + redisCounterProvider.inc('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-15', 1) + redisCounterProvider.inc('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-14', 1) + redisCounterProvider.inc('metrics/v1', 'builds/o/abc.com.au/d/2024-07-14', 1) + redisCounterProvider.inc('metrics/v1', 'builds/o/abc.com.au/d/2024-07-14', 1) + + then:'delete all pull counter keys' + redisCounterProvider.deleteAllMatchingEntries('metrics/v1', 'pulls/o/*') + redisCounterProvider.getAllMatchingEntries('metrics/v1', 'pulls/o/*') == [:] + redisCounterProvider.getAllMatchingEntries('metrics/v1', 'pulls/o/*/d/2024-05-30') == [:] + and:'delete only build per specific date counter keys' + redisCounterProvider.deleteAllMatchingEntries('metrics/v1', 'builds/o/abc.com.au/d/2024-07-14') + redisCounterProvider.getAllMatchingEntries('metrics/v1', 'builds/o/*') == ['builds/o/foo.com':1, 'builds/o/bar.io':1, 'builds/o/abc.org':2] + } } From 6da3b49cfedfb64deb638ea5ee5f9d6ed4727ede Mon Sep 17 00:00:00 2001 From: munishchouhan Date: Tue, 16 Jul 2024 13:47:20 +0200 Subject: [PATCH 02/12] test for expiry Signed-off-by: munishchouhan --- .../counter/impl/RedisCounterProvider.groovy | 9 +++++- .../impl/RedisCounterProviderTest.groovy | 29 +++++++++---------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/main/groovy/io/seqera/wave/service/counter/impl/RedisCounterProvider.groovy b/src/main/groovy/io/seqera/wave/service/counter/impl/RedisCounterProvider.groovy index c0e9bb1d9..25887e4ef 100644 --- a/src/main/groovy/io/seqera/wave/service/counter/impl/RedisCounterProvider.groovy +++ b/src/main/groovy/io/seqera/wave/service/counter/impl/RedisCounterProvider.groovy @@ -18,6 +18,8 @@ package io.seqera.wave.service.counter.impl +import java.time.Duration + import groovy.transform.CompileStatic import io.micronaut.context.annotation.Requires import io.micronaut.context.annotation.Value @@ -42,10 +44,15 @@ class RedisCounterProvider implements CounterProvider { @Value('${redis.hscan.count:10000}') private Integer hscanCount + @Value('${redis.key.expiry:1s}') + private Duration keyExpiry + @Override long inc(String key, String field, long value) { try(Jedis conn=pool.getResource() ) { - return conn.hincrBy(key, field, value) + long count = conn.hincrBy(key, field, value) + conn.expire(key, keyExpiry.toSeconds()) + return count } } diff --git a/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy b/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy index 5a7d56970..3f91ccdd1 100644 --- a/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy +++ b/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy @@ -64,7 +64,7 @@ class RedisCounterProviderTest extends Specification implements RedisTestContain redisCounterProvider.get('metrics-x', 'foo') == 1 } - def 'should get correct org count' () { + def 'should get and delete metrics counter' () { when: redisCounterProvider.inc('metrics/v1', 'builds/o/foo.com', 1) redisCounterProvider.inc('metrics/v1', 'builds/o/bar.io', 1) @@ -81,27 +81,24 @@ class RedisCounterProviderTest extends Specification implements RedisTestContain and: redisCounterProvider.getAllMatchingEntries('metrics/v1', 'pulls/o/*/d/2024-05-30') == ['pulls/o/abc.com.au/d/2024-05-30':1] - } - - def 'should delete records with given pattern' () { - when: - redisCounterProvider.inc('metrics/v1', 'builds/o/foo.com', 1) - redisCounterProvider.inc('metrics/v1', 'builds/o/bar.io', 1) - redisCounterProvider.inc('metrics/v1', 'builds/o/abc.org', 2) - redisCounterProvider.inc('metrics/v1', 'pulls/o/foo.it', 1) - redisCounterProvider.inc('metrics/v1', 'pulls/o/bar.es', 2) - redisCounterProvider.inc('metrics/v1', 'pulls/o/abc.in', 3) - redisCounterProvider.inc('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-15', 1) - redisCounterProvider.inc('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-14', 1) - redisCounterProvider.inc('metrics/v1', 'builds/o/abc.com.au/d/2024-07-14', 1) - redisCounterProvider.inc('metrics/v1', 'builds/o/abc.com.au/d/2024-07-14', 1) - then:'delete all pull counter keys' + and:'delete all pull counter keys' redisCounterProvider.deleteAllMatchingEntries('metrics/v1', 'pulls/o/*') redisCounterProvider.getAllMatchingEntries('metrics/v1', 'pulls/o/*') == [:] redisCounterProvider.getAllMatchingEntries('metrics/v1', 'pulls/o/*/d/2024-05-30') == [:] + and:'delete only build per specific date counter keys' redisCounterProvider.deleteAllMatchingEntries('metrics/v1', 'builds/o/abc.com.au/d/2024-07-14') redisCounterProvider.getAllMatchingEntries('metrics/v1', 'builds/o/*') == ['builds/o/foo.com':1, 'builds/o/bar.io':1, 'builds/o/abc.org':2] } + + def 'should expire the hash'(){ + when: + redisCounterProvider.inc('build-x', 'foo', 1) + sleep(500) + redisCounterProvider.inc('build-x', 'foo', 1) + sleep(500) + then: + redisCounterProvider.get('build-x', 'foo') == 1 + } } From e787bc6a48709bffda34299e93524873c05a5274 Mon Sep 17 00:00:00 2001 From: munishchouhan Date: Tue, 16 Jul 2024 13:51:53 +0200 Subject: [PATCH 03/12] test for expiry Signed-off-by: munishchouhan --- .../wave/service/counter/impl/RedisCounterProviderTest.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy b/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy index 3f91ccdd1..900a599ce 100644 --- a/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy +++ b/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy @@ -96,9 +96,9 @@ class RedisCounterProviderTest extends Specification implements RedisTestContain when: redisCounterProvider.inc('build-x', 'foo', 1) sleep(500) - redisCounterProvider.inc('build-x', 'foo', 1) + redisCounterProvider.inc('build-x', 'bar', 1) sleep(500) - then: + then:'this value should be one, because foo should be expired' redisCounterProvider.get('build-x', 'foo') == 1 } } From f58f28c6d963534e6f62b87031f0e82dbb8614be Mon Sep 17 00:00:00 2001 From: munishchouhan Date: Tue, 16 Jul 2024 18:04:39 +0200 Subject: [PATCH 04/12] corrected tests Signed-off-by: munishchouhan --- .../impl/LocalCounterProviderTest.groovy | 22 --------------- .../impl/RedisCounterProviderTest.groovy | 27 ++++++++----------- 2 files changed, 11 insertions(+), 38 deletions(-) diff --git a/src/test/groovy/io/seqera/wave/service/counter/impl/LocalCounterProviderTest.groovy b/src/test/groovy/io/seqera/wave/service/counter/impl/LocalCounterProviderTest.groovy index 54d6ec088..1787b61f5 100644 --- a/src/test/groovy/io/seqera/wave/service/counter/impl/LocalCounterProviderTest.groovy +++ b/src/test/groovy/io/seqera/wave/service/counter/impl/LocalCounterProviderTest.groovy @@ -73,26 +73,4 @@ class LocalCounterProviderTest extends Specification { ['pulls/o/abc.com.au/d/2024-05-30':1] } - def 'should delete records with given pattern' () { - when: - localCounterProvider.inc('metrics/v1', 'builds/o/foo.com', 1) - localCounterProvider.inc('metrics/v1', 'builds/o/bar.io', 1) - localCounterProvider.inc('metrics/v1', 'builds/o/abc.org', 2) - localCounterProvider.inc('metrics/v1', 'pulls/o/foo.it', 1) - localCounterProvider.inc('metrics/v1', 'pulls/o/bar.es', 2) - localCounterProvider.inc('metrics/v1', 'pulls/o/abc.in', 3) - localCounterProvider.inc('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-15', 1) - localCounterProvider.inc('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-14', 1) - localCounterProvider.inc('metrics/v1', 'builds/o/abc.com.au/d/2024-07-14', 1) - localCounterProvider.inc('metrics/v1', 'builds/o/abc.com.au/d/2024-07-14', 1) - - then:'delete all pull counter keys' - localCounterProvider.deleteAllMatchingEntries('metrics/v1', 'pulls/o/*') - localCounterProvider.getAllMatchingEntries('metrics/v1', 'pulls/o/*') == [:] - localCounterProvider.getAllMatchingEntries('metrics/v1', 'pulls/o/*/d/2024-05-30') == [:] - and:'delete only build per specific date counter keys' - localCounterProvider.deleteAllMatchingEntries('metrics/v1', 'builds/o/abc.com.au/d/2024-07-14') - localCounterProvider.getAllMatchingEntries('metrics/v1', 'builds/o/*') == ['builds/o/foo.com':1, 'builds/o/bar.io':1, 'builds/o/abc.org':2] - } - } diff --git a/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy b/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy index 900a599ce..e95c5d776 100644 --- a/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy +++ b/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy @@ -35,7 +35,8 @@ class RedisCounterProviderTest extends Specification implements RedisTestContain def setup() { applicationContext = ApplicationContext.run([ REDIS_HOST : redisHostName, - REDIS_PORT : redisPort + REDIS_PORT : redisPort, + 'redis.key.expiry': '1s' ], 'test', 'redis') redisCounterProvider = applicationContext.getBean(RedisCounterProvider) sleep(500) // workaround to wait for Redis connection @@ -77,28 +78,22 @@ class RedisCounterProviderTest extends Specification implements RedisTestContain then: redisCounterProvider.getAllMatchingEntries('metrics/v1', 'pulls/o/*') == - ['pulls/o/abc.in':3, 'pulls/o/bar.es':2, 'pulls/o/foo.it':1, 'pulls/o/abc.com.au/d/2024-05-30':1, 'pulls/o/abc.com.au/d/2024-05-31':1] + ['pulls/o/abc.in': 3, 'pulls/o/bar.es': 2, 'pulls/o/foo.it': 1, 'pulls/o/abc.com.au/d/2024-05-30': 1, 'pulls/o/abc.com.au/d/2024-05-31': 1] and: redisCounterProvider.getAllMatchingEntries('metrics/v1', 'pulls/o/*/d/2024-05-30') == - ['pulls/o/abc.com.au/d/2024-05-30':1] - - and:'delete all pull counter keys' - redisCounterProvider.deleteAllMatchingEntries('metrics/v1', 'pulls/o/*') - redisCounterProvider.getAllMatchingEntries('metrics/v1', 'pulls/o/*') == [:] - redisCounterProvider.getAllMatchingEntries('metrics/v1', 'pulls/o/*/d/2024-05-30') == [:] - - and:'delete only build per specific date counter keys' - redisCounterProvider.deleteAllMatchingEntries('metrics/v1', 'builds/o/abc.com.au/d/2024-07-14') - redisCounterProvider.getAllMatchingEntries('metrics/v1', 'builds/o/*') == ['builds/o/foo.com':1, 'builds/o/bar.io':1, 'builds/o/abc.org':2] + ['pulls/o/abc.com.au/d/2024-05-30': 1] } def 'should expire the hash'(){ when: - redisCounterProvider.inc('build-x', 'foo', 1) - sleep(500) - redisCounterProvider.inc('build-x', 'bar', 1) + redisCounterProvider.inc('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-14', 1) sleep(500) + redisCounterProvider.inc('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-15', 1) + sleep(1000) then:'this value should be one, because foo should be expired' - redisCounterProvider.get('build-x', 'foo') == 1 + redisCounterProvider.get('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-14') == null + sleep(500) + and: + redisCounterProvider.get('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-15') == null } } From 636a90f4db6373b5ceaf46c2cd85392a10d88fad Mon Sep 17 00:00:00 2001 From: munishchouhan Date: Tue, 16 Jul 2024 18:27:49 +0200 Subject: [PATCH 05/12] bump redis.clients:jedis:5.1.3 Signed-off-by: munishchouhan --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 4d3be2206..7f858e1b7 100644 --- a/build.gradle +++ b/build.gradle @@ -60,7 +60,7 @@ dependencies { implementation 'com.fasterxml.jackson.dataformat:jackson-dataformat-yaml' implementation 'com.squareup.moshi:moshi:1.14.0' implementation 'com.squareup.moshi:moshi-adapters:1.14.0' - implementation 'redis.clients:jedis:5.0.2' + implementation 'redis.clients:jedis:5.1.3' implementation "io.github.resilience4j:resilience4j-ratelimiter:0.17.0" // caching deps implementation("io.micronaut.cache:micronaut-cache-core") From ac6dc7dce62677f1553ce0427e78b09476ee3439 Mon Sep 17 00:00:00 2001 From: munishchouhan Date: Wed, 17 Jul 2024 16:25:32 +0200 Subject: [PATCH 06/12] added failing and success expire test Signed-off-by: munishchouhan --- .../counter/impl/RedisCounterProviderTest.groovy | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy b/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy index e95c5d776..cbbce2c72 100644 --- a/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy +++ b/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy @@ -84,7 +84,20 @@ class RedisCounterProviderTest extends Specification implements RedisTestContain ['pulls/o/abc.com.au/d/2024-05-30': 1] } - def 'should expire the hash'(){ + def 'failing: should expire the hash'(){ + when: + redisCounterProvider.inc('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-14', 1) + sleep(500) + redisCounterProvider.inc('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-15', 1) + sleep(500) + then:'this value should be one, because foo should be expired' + redisCounterProvider.get('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-14') == null + sleep(500) + and: + redisCounterProvider.get('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-15') == null + } + + def 'successful: should expire the hash'(){ when: redisCounterProvider.inc('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-14', 1) sleep(500) From abd522a5b9caa26cc22f7405bd7b36f45c7567e6 Mon Sep 17 00:00:00 2001 From: munishchouhan Date: Wed, 17 Jul 2024 16:27:19 +0200 Subject: [PATCH 07/12] minor change [ci skip] Signed-off-by: munishchouhan --- .../wave/service/counter/impl/RedisCounterProviderTest.groovy | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy b/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy index cbbce2c72..e693d785c 100644 --- a/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy +++ b/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy @@ -105,8 +105,6 @@ class RedisCounterProviderTest extends Specification implements RedisTestContain sleep(1000) then:'this value should be one, because foo should be expired' redisCounterProvider.get('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-14') == null - sleep(500) - and: redisCounterProvider.get('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-15') == null } } From d695686144c33debd348acb7e03496889de8f81d Mon Sep 17 00:00:00 2001 From: munishchouhan Date: Wed, 14 Aug 2024 16:36:19 +0200 Subject: [PATCH 08/12] added hexpire and bumped jedis to 5.2.0-beta4 Signed-off-by: munishchouhan --- .../counter/impl/CounterProvider.groovy | 7 ------- .../counter/impl/LocalCounterProvider.groovy | 11 ----------- .../counter/impl/RedisCounterProvider.groovy | 14 +------------- .../impl/RedisCounterProviderTest.groovy | 19 +++---------------- .../wave/test/RedisTestContainer.groovy | 2 +- 5 files changed, 5 insertions(+), 48 deletions(-) diff --git a/src/main/groovy/io/seqera/wave/service/counter/impl/CounterProvider.groovy b/src/main/groovy/io/seqera/wave/service/counter/impl/CounterProvider.groovy index 64823b435..9fc868ab4 100644 --- a/src/main/groovy/io/seqera/wave/service/counter/impl/CounterProvider.groovy +++ b/src/main/groovy/io/seqera/wave/service/counter/impl/CounterProvider.groovy @@ -36,11 +36,4 @@ interface CounterProvider { */ Map getAllMatchingEntries(String key, String pattern) - /** - * - * @param key - * @param pattern - * @return void - */ - void deleteAllMatchingEntries(String key, String pattern) } diff --git a/src/main/groovy/io/seqera/wave/service/counter/impl/LocalCounterProvider.groovy b/src/main/groovy/io/seqera/wave/service/counter/impl/LocalCounterProvider.groovy index b94c07cff..8234e7dba 100644 --- a/src/main/groovy/io/seqera/wave/service/counter/impl/LocalCounterProvider.groovy +++ b/src/main/groovy/io/seqera/wave/service/counter/impl/LocalCounterProvider.groovy @@ -62,15 +62,4 @@ class LocalCounterProvider implements CounterProvider { return result } - @Override - void deleteAllMatchingEntries(String key, String pattern) { - Pattern compiledPattern = Pattern.compile(pattern.replace('*', '.*')) - Map keyStore = store.get(key) - if (keyStore){ - def matchingPairs = keyStore.findAll {entry -> compiledPattern.matcher(entry.key).matches()} - matchingPairs.each { k, v -> - keyStore.remove(k) - } - } - } } diff --git a/src/main/groovy/io/seqera/wave/service/counter/impl/RedisCounterProvider.groovy b/src/main/groovy/io/seqera/wave/service/counter/impl/RedisCounterProvider.groovy index 25887e4ef..5d3adb480 100644 --- a/src/main/groovy/io/seqera/wave/service/counter/impl/RedisCounterProvider.groovy +++ b/src/main/groovy/io/seqera/wave/service/counter/impl/RedisCounterProvider.groovy @@ -51,7 +51,7 @@ class RedisCounterProvider implements CounterProvider { long inc(String key, String field, long value) { try(Jedis conn=pool.getResource() ) { long count = conn.hincrBy(key, field, value) - conn.expire(key, keyExpiry.toSeconds()) + conn.hexpire(key, keyExpiry.toSeconds(), field) return count } } @@ -78,16 +78,4 @@ class RedisCounterProvider implements CounterProvider { } } - @Override - void deleteAllMatchingEntries(String key, String pattern) { - try(Jedis conn=pool.getResource() ) { - final scanResult = conn.hscan(key, "0", new ScanParams().match(pattern).count(hscanCount)) - if( !scanResult ) - return - for(String entry : scanResult.result) { - final parts = entry.tokenize('=') - conn.hdel(key, parts[0]) - } - } - } } diff --git a/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy b/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy index e693d785c..18995f49b 100644 --- a/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy +++ b/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy @@ -65,7 +65,7 @@ class RedisCounterProviderTest extends Specification implements RedisTestContain redisCounterProvider.get('metrics-x', 'foo') == 1 } - def 'should get and delete metrics counter' () { + def 'should get correct org count' () { when: redisCounterProvider.inc('metrics/v1', 'builds/o/foo.com', 1) redisCounterProvider.inc('metrics/v1', 'builds/o/bar.io', 1) @@ -78,13 +78,10 @@ class RedisCounterProviderTest extends Specification implements RedisTestContain then: redisCounterProvider.getAllMatchingEntries('metrics/v1', 'pulls/o/*') == - ['pulls/o/abc.in': 3, 'pulls/o/bar.es': 2, 'pulls/o/foo.it': 1, 'pulls/o/abc.com.au/d/2024-05-30': 1, 'pulls/o/abc.com.au/d/2024-05-31': 1] - and: - redisCounterProvider.getAllMatchingEntries('metrics/v1', 'pulls/o/*/d/2024-05-30') == - ['pulls/o/abc.com.au/d/2024-05-30': 1] + ['pulls/o/abc.in':3, 'pulls/o/bar.es':2, 'pulls/o/foo.it':1, 'pulls/o/abc.com.au/d/2024-05-30':1, 'pulls/o/abc.com.au/d/2024-05-31':1] } - def 'failing: should expire the hash'(){ + def 'should expire the hash'(){ when: redisCounterProvider.inc('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-14', 1) sleep(500) @@ -97,14 +94,4 @@ class RedisCounterProviderTest extends Specification implements RedisTestContain redisCounterProvider.get('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-15') == null } - def 'successful: should expire the hash'(){ - when: - redisCounterProvider.inc('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-14', 1) - sleep(500) - redisCounterProvider.inc('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-15', 1) - sleep(1000) - then:'this value should be one, because foo should be expired' - redisCounterProvider.get('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-14') == null - redisCounterProvider.get('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-15') == null - } } diff --git a/src/test/groovy/io/seqera/wave/test/RedisTestContainer.groovy b/src/test/groovy/io/seqera/wave/test/RedisTestContainer.groovy index f38dc516e..b0e4f9bf0 100644 --- a/src/test/groovy/io/seqera/wave/test/RedisTestContainer.groovy +++ b/src/test/groovy/io/seqera/wave/test/RedisTestContainer.groovy @@ -45,7 +45,7 @@ trait RedisTestContainer { def setupSpec() { log.debug "Starting Redis test container" - redisContainer = new GenericContainer(DockerImageName.parse("redis:7.0.4-alpine")) + redisContainer = new GenericContainer(DockerImageName.parse("redis:7.4.0-alpine")) .withExposedPorts(6379) .waitingFor(Wait.forLogMessage(".*Ready to accept connections.*\\n", 1)) redisContainer.start() From 3399bd2f45e0d0493c2e55971897ce5e51daf3bb Mon Sep 17 00:00:00 2001 From: munishchouhan Date: Wed, 14 Aug 2024 16:36:59 +0200 Subject: [PATCH 09/12] bump jedis to 5.2.0-beta4 Signed-off-by: munishchouhan --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 9d72c8b93..a9a715ec6 100644 --- a/build.gradle +++ b/build.gradle @@ -60,7 +60,7 @@ dependencies { implementation 'com.fasterxml.jackson.dataformat:jackson-dataformat-yaml' implementation 'com.squareup.moshi:moshi:1.14.0' implementation 'com.squareup.moshi:moshi-adapters:1.14.0' - implementation 'redis.clients:jedis:5.1.3' + implementation 'redis.clients:jedis:5.2.0-beta4' implementation "io.github.resilience4j:resilience4j-ratelimiter:0.17.0" // caching deps implementation("io.micronaut.cache:micronaut-cache-core") From f8849d709dbe4635bc0d84929791fa6178c8a15f Mon Sep 17 00:00:00 2001 From: munishchouhan Date: Wed, 14 Aug 2024 16:41:49 +0200 Subject: [PATCH 10/12] only date metrics expire Signed-off-by: munishchouhan --- .../wave/service/counter/impl/RedisCounterProvider.groovy | 8 +++++++- .../service/counter/impl/RedisCounterProviderTest.groovy | 3 +++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/main/groovy/io/seqera/wave/service/counter/impl/RedisCounterProvider.groovy b/src/main/groovy/io/seqera/wave/service/counter/impl/RedisCounterProvider.groovy index 5d3adb480..e540acfb8 100644 --- a/src/main/groovy/io/seqera/wave/service/counter/impl/RedisCounterProvider.groovy +++ b/src/main/groovy/io/seqera/wave/service/counter/impl/RedisCounterProvider.groovy @@ -47,11 +47,17 @@ class RedisCounterProvider implements CounterProvider { @Value('${redis.key.expiry:1s}') private Duration keyExpiry + final private static String DATE_PATTERN = /\b\d{4}-\d{2}-\d{2}\b/ + + static boolean isMetricPerDate(String field) { + return field =~ DATE_PATTERN + } @Override long inc(String key, String field, long value) { try(Jedis conn=pool.getResource() ) { long count = conn.hincrBy(key, field, value) - conn.hexpire(key, keyExpiry.toSeconds(), field) + if(isMetricPerDate(field)) + conn.hexpire(key, keyExpiry.toSeconds(), field) return count } } diff --git a/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy b/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy index 18995f49b..5becdb169 100644 --- a/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy +++ b/src/test/groovy/io/seqera/wave/service/counter/impl/RedisCounterProviderTest.groovy @@ -83,6 +83,7 @@ class RedisCounterProviderTest extends Specification implements RedisTestContain def 'should expire the hash'(){ when: + redisCounterProvider.inc('metrics/v1', 'pulls/o/abc.com.au', 1) redisCounterProvider.inc('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-14', 1) sleep(500) redisCounterProvider.inc('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-15', 1) @@ -92,6 +93,8 @@ class RedisCounterProviderTest extends Specification implements RedisTestContain sleep(500) and: redisCounterProvider.get('metrics/v1', 'pulls/o/abc.com.au/d/2024-07-15') == null + and: 'this value should be one, because org metric should not expire' + redisCounterProvider.get('metrics/v1', 'pulls/o/abc.com.au') == 1 } } From 8e963c870ed6b2be0ee184176a4a75003f6218b9 Mon Sep 17 00:00:00 2001 From: munishchouhan Date: Tue, 24 Sep 2024 22:31:26 +0200 Subject: [PATCH 11/12] bump redis.clients:jedis:5.2.0-beta5 Signed-off-by: munishchouhan --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index a9a715ec6..000b5f41b 100644 --- a/build.gradle +++ b/build.gradle @@ -60,7 +60,7 @@ dependencies { implementation 'com.fasterxml.jackson.dataformat:jackson-dataformat-yaml' implementation 'com.squareup.moshi:moshi:1.14.0' implementation 'com.squareup.moshi:moshi-adapters:1.14.0' - implementation 'redis.clients:jedis:5.2.0-beta4' + implementation 'redis.clients:jedis:5.2.0-beta5' implementation "io.github.resilience4j:resilience4j-ratelimiter:0.17.0" // caching deps implementation("io.micronaut.cache:micronaut-cache-core") From afb801b9650e03c230cc47b6aac70eaebe40a8e1 Mon Sep 17 00:00:00 2001 From: munishchouhan Date: Wed, 2 Oct 2024 15:46:59 +0200 Subject: [PATCH 12/12] bump redis.clients:jedis:5.2.0 Signed-off-by: munishchouhan --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index f31f80774..01bc4608e 100644 --- a/build.gradle +++ b/build.gradle @@ -60,7 +60,7 @@ dependencies { implementation 'com.fasterxml.jackson.dataformat:jackson-dataformat-yaml' implementation 'com.squareup.moshi:moshi:1.14.0' implementation 'com.squareup.moshi:moshi-adapters:1.14.0' - implementation 'redis.clients:jedis:5.2.0-beta5' + implementation 'redis.clients:jedis:5.2.0' implementation "io.github.resilience4j:resilience4j-ratelimiter:0.17.0" // caching deps implementation("io.micronaut.cache:micronaut-cache-core")