diff --git a/litmus-cache/src/main/java/com/navi/medici/cache/RedisCache.java b/litmus-cache/src/main/java/com/navi/medici/cache/RedisCache.java index a98a1cb..2d2a87e 100644 --- a/litmus-cache/src/main/java/com/navi/medici/cache/RedisCache.java +++ b/litmus-cache/src/main/java/com/navi/medici/cache/RedisCache.java @@ -2,15 +2,10 @@ package com.navi.medici.cache; import com.navi.medici.command.CacheCommands; import com.navi.medici.config.RedisConfiguration; -import com.navi.medici.request.v1.LitmusExperiment; import lombok.RequiredArgsConstructor; -import org.redisson.api.RBloomFilter; -import org.redisson.api.RList; -import org.redisson.api.RMap; -import org.redisson.api.RedissonClient; +import org.redisson.api.*; import org.springframework.stereotype.Component; -import java.util.List; @Component @RequiredArgsConstructor @@ -62,15 +57,4 @@ public class RedisCache implements CacheCommands { RMap rm = redissonClient.getMap(map); rm.remove(key); } - - public void updateEnabledExperiments(List litmusExperiments) { - RList list =redissonClient.getList("enabledExperiments"); - list.clear(); - list.addAll(litmusExperiments); - } - - public List getEnabledExperiments() { - RList list = redissonClient.getList("enabledExperiments"); - return list.readAll(); - } } diff --git a/litmus-cache/src/main/java/com/navi/medici/command/CacheCommands.java b/litmus-cache/src/main/java/com/navi/medici/command/CacheCommands.java index 4c8c9af..c5eb179 100644 --- a/litmus-cache/src/main/java/com/navi/medici/command/CacheCommands.java +++ b/litmus-cache/src/main/java/com/navi/medici/command/CacheCommands.java @@ -1,9 +1,7 @@ package com.navi.medici.command; -import com.navi.medici.request.v1.LitmusExperiment; import org.redisson.api.RBloomFilter; -import java.util.List; public interface CacheCommands { Boolean reserveBF(String name); @@ -22,7 +20,4 @@ public interface CacheCommands { void delete(String map, String key); - void updateEnabledExperiments(List litmusExperiments); - - List getEnabledExperiments(); } diff --git a/litmus-core/src/main/java/com/navi/medici/config/LitmusCoreConfig.java b/litmus-core/src/main/java/com/navi/medici/config/LitmusCoreConfig.java index dfa7fcc..3970d31 100644 --- a/litmus-core/src/main/java/com/navi/medici/config/LitmusCoreConfig.java +++ b/litmus-core/src/main/java/com/navi/medici/config/LitmusCoreConfig.java @@ -75,4 +75,7 @@ public class LitmusCoreConfig { @Value("${query.day.interval.for.total.users}") int queryDayIntervalForTotalUsers; + + @Value("${litmus.fetch.experiment.from.cache}") + boolean fetchExperimentFromCache; } diff --git a/litmus-core/src/main/java/com/navi/medici/scheduler/ExperimentMapUpdater.java b/litmus-core/src/main/java/com/navi/medici/scheduler/EnabledLitmusExperimentUpdater.java similarity index 55% rename from litmus-core/src/main/java/com/navi/medici/scheduler/ExperimentMapUpdater.java rename to litmus-core/src/main/java/com/navi/medici/scheduler/EnabledLitmusExperimentUpdater.java index fb16b53..64c3cfe 100644 --- a/litmus-core/src/main/java/com/navi/medici/scheduler/ExperimentMapUpdater.java +++ b/litmus-core/src/main/java/com/navi/medici/scheduler/EnabledLitmusExperimentUpdater.java @@ -1,6 +1,5 @@ package com.navi.medici.scheduler; -import com.navi.medici.command.CacheCommands; import com.navi.medici.entity.ExperimentEntity; import com.navi.medici.factories.StaticFactoryObjects; import com.navi.medici.query.experiment.IExperimentQuery; @@ -8,28 +7,36 @@ import com.navi.medici.request.v1.LitmusExperiment; import com.navi.medici.util.JacksonUtils; import lombok.RequiredArgsConstructor; import lombok.extern.log4j.Log4j2; -import net.javacrumbs.shedlock.core.SchedulerLock; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Service; +import javax.annotation.PostConstruct; import java.util.List; +import java.util.stream.Collectors; @Service @RequiredArgsConstructor @Log4j2 -public class ExperimentMapUpdater { +public class EnabledLitmusExperimentUpdater { - private final CacheCommands cacheCommands; private final IExperimentQuery iExperimentQuery; private final JacksonUtils jacksonUtils; + private List litmusExperiments; - @Scheduled(cron = "${fetch.enabled.experiments.cron}") - @SchedulerLock(name = "Fetch_Enabled_Experiments_Updater", lockAtMostForString = "PT10S") - public void updateEnabledExperiments() { - log.info("Refreshing enabled experiments in Redis cache"); - List experimentEntities = iExperimentQuery.findByEnabled(true); - List litmusExperiments = experimentEntities.stream().map(experimentEntity -> StaticFactoryObjects.fromExperimentEntity(experimentEntity, jacksonUtils)).toList(); - cacheCommands.updateEnabledExperiments(litmusExperiments); + @PostConstruct + private void init() { + log.info("Running EnabledLitmusExperimentUpdater on startup"); + updateEnabledExperiments(); } + @Scheduled(cron = "${fetch.enabled.experiments.cron}") + public void updateEnabledExperiments() { + log.info("Refreshing enabled experiments in InMemory cache"); + List experimentEntities = iExperimentQuery.findByEnabled(true); + litmusExperiments = experimentEntities.stream().map(experimentEntity -> StaticFactoryObjects.fromExperimentEntity(experimentEntity, jacksonUtils)).collect(Collectors.toList()); + } + + public List getLitmusExperiments() { + return litmusExperiments; + } } diff --git a/litmus-core/src/main/java/com/navi/medici/service/experiment/ExperimentServiceImpl.java b/litmus-core/src/main/java/com/navi/medici/service/experiment/ExperimentServiceImpl.java index c132015..3679276 100644 --- a/litmus-core/src/main/java/com/navi/medici/service/experiment/ExperimentServiceImpl.java +++ b/litmus-core/src/main/java/com/navi/medici/service/experiment/ExperimentServiceImpl.java @@ -1,7 +1,6 @@ package com.navi.medici.service.experiment; import com.navi.medici.Constants; -import com.navi.medici.command.CacheCommands; import com.navi.medici.config.LitmusCoreConfig; import com.navi.medici.dto.DoughnutSectionDTO; import com.navi.medici.dto.Dropdown; @@ -28,6 +27,7 @@ import com.navi.medici.exceptions.MetricNotFoundException; import com.navi.medici.exceptions.UnableToChangeExperimentStatusException; import com.navi.medici.factories.StaticFactoryObjects; import com.navi.medici.mapper.ExperimentMapper; +import com.navi.medici.metrics.MetricsUtils; import com.navi.medici.query.experiment.IExperimentQuery; import com.navi.medici.query.experimentaudittrail.IExperimentAuditTrailQuery; import com.navi.medici.query.experimentinfo.IExperimentInfoQuery; @@ -47,6 +47,7 @@ import com.navi.medici.response.DashboardExperimentResponse; import com.navi.medici.response.ExperimentResponse; import com.navi.medici.response.LitmusExperimentCollection; import com.navi.medici.response.PaginatedSearchResponse; +import com.navi.medici.scheduler.EnabledLitmusExperimentUpdater; import com.navi.medici.specification.ExperimentSpecification; import com.navi.medici.stats.ChiSquaredTest; import com.navi.medici.stats.SampleSizeCalculator; @@ -55,6 +56,7 @@ import com.navi.medici.util.DateTimeUtil; import com.navi.medici.util.JacksonUtils; import com.navi.medici.validator.ExperimentValidator; import com.navi.medici.variants.VariantDefinition; +import io.micrometer.core.instrument.MeterRegistry; import lombok.RequiredArgsConstructor; import lombok.extern.log4j.Log4j2; import org.apache.commons.lang3.StringUtils; @@ -66,7 +68,6 @@ import org.springframework.stereotype.Service; import javax.transaction.Transactional; import java.time.LocalDateTime; import java.util.ArrayList; -import java.util.Collections; import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; @@ -101,7 +102,8 @@ public class ExperimentServiceImpl implements ExperimentService { private final IExperimentMetricResultQuery experimentMetricResultQuery; private final IExperimentMetricMappingQuery experimentMetricMappingQuery; private final IExperimentPopulationResultQuery experimentPopulationResultQuery; - private final CacheCommands cacheCommands; + private final EnabledLitmusExperimentUpdater enabledLitmusExperimentUpdater; + private final MeterRegistry meterRegistry; @Override @Transactional @@ -279,9 +281,28 @@ public class ExperimentServiceImpl implements ExperimentService { @Override public LitmusExperimentCollection fetchAllExperiments() { - List litmusExperiments = cacheCommands.getEnabledExperiments(); + List litmusExperiments; + if (litmusCoreConfig.isFetchExperimentFromCache()) { + litmusExperiments = enabledLitmusExperimentUpdater.getLitmusExperiments(); + } else { + List experimentEntities = experimentQuery.findByEnabled(true); + log.info("Fetched experiments from DB: {}", experimentEntities.size()); + MetricsUtils.customCounterMetrics("litmus_core_fetch_all_experiments_from_db") + .tag("experimentCount", String.valueOf(experimentEntities.size())) + .register(meterRegistry) + .increment(); + List litmusExperimentsFromCache = enabledLitmusExperimentUpdater.getLitmusExperiments(); + log.info("Fetched experiments from cache: {}", litmusExperimentsFromCache.size()); + MetricsUtils.customCounterMetrics("litmus_core_fetch_all_experiments_from_cache") + .tag("experimentCount", String.valueOf(litmusExperimentsFromCache.size())) + .register(meterRegistry) + .increment(); + litmusExperiments = experimentEntities.stream() + .map(experimentEntity -> StaticFactoryObjects.fromExperimentEntity(experimentEntity, jacksonUtils)) + .collect(Collectors.toList()); + } - litmusExperiments = litmusExperiments.stream().peek(le -> { + litmusExperiments = litmusExperiments.stream().peek(le -> { //This is to reduce sending strategy string over network if (RELEASE.equals(le.getType()) || KILL_SWITCH.equals(le.getType())) { le.setStrategies(new ArrayList<>()); diff --git a/litmus-core/src/main/resources/application.properties b/litmus-core/src/main/resources/application.properties index fa1667b..dc41978 100644 --- a/litmus-core/src/main/resources/application.properties +++ b/litmus-core/src/main/resources/application.properties @@ -62,4 +62,5 @@ experiment.metric.fetch.limit.per.interval=${EXPERIMENT_METRIC_FETCH_LIMIT_PER_I experiment.population.fetch.limit.per.interval=${EXPERIMENT_POPULATION_FETCH_LIMIT_PER_INTERVAL:5} population.graph.data.days.interval=${POPULATION_GRAPH_DATA_DAYS_INTERVAL:3} query.schedule.and.end.time.diff.hours=${QUERY_SCHEDULE_AND_END_TIME_DIFF_HOURS:2} -query.day.interval.for.total.users=${QUERY_DAY_INTERVAL_FOR_TOTAL_USERS:30} \ No newline at end of file +query.day.interval.for.total.users=${QUERY_DAY_INTERVAL_FOR_TOTAL_USERS:30} +litmus.fetch.experiment.from.cache=${LITMUS_FETCH_EXPERIMENT_FROM_CACHE:true} \ No newline at end of file diff --git a/litmus-core/src/test/java/com/navi/medici/service/experiment/ExperimentServiceImplTest.java b/litmus-core/src/test/java/com/navi/medici/service/experiment/ExperimentServiceImplTest.java index 33a61ef..0b75f5b 100644 --- a/litmus-core/src/test/java/com/navi/medici/service/experiment/ExperimentServiceImplTest.java +++ b/litmus-core/src/test/java/com/navi/medici/service/experiment/ExperimentServiceImplTest.java @@ -2,7 +2,6 @@ package com.navi.medici.service.experiment; import com.fasterxml.jackson.core.JsonProcessingException; import com.navi.medici.TestUtils; -import com.navi.medici.command.CacheCommands; import com.navi.medici.config.LitmusCoreConfig; import com.navi.medici.dto.Dropdown; import com.navi.medici.dto.ExperimentAuditTrailDTO; @@ -34,14 +33,18 @@ import com.navi.medici.response.DashboardExperimentResponse; import com.navi.medici.response.ExperimentResponse; import com.navi.medici.response.LitmusExperimentCollection; import com.navi.medici.response.PaginatedSearchResponse; +import com.navi.medici.scheduler.EnabledLitmusExperimentUpdater; import com.navi.medici.util.JacksonUtils; import com.navi.medici.validator.ExperimentValidator; import com.navi.medici.variants.VariantDefinition; +import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.Mockito; +import org.mockito.Spy; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; @@ -78,7 +81,7 @@ class ExperimentServiceImplTest { private IExperimentInfoQuery experimentInfoQuery; @Mock - private CacheCommands cacheCommands; + private EnabledLitmusExperimentUpdater experimentMapUpdater; @Mock private IMetricQuery metricQuery; @@ -99,6 +102,9 @@ class ExperimentServiceImplTest { @Mock private IExperimentMetricResultQuery experimentMetricResultQuery; + @Spy + private MeterRegistry meterRegistry = new SimpleMeterRegistry(); + @Test void shouldCreateExperiment() { @@ -152,13 +158,50 @@ class ExperimentServiceImplTest { } @Test - void shouldFetchExperiments() { + void shouldFetchExperimentsWillFetchExperimentFromBothCacheAndDBIfFetchExperimentFromCacheIsFalse() { LitmusExperiment litmusExperiment = TestUtils.getLitmusExperiment(); litmusExperiment.setType(ExperimentType.RELEASE); - Mockito.when(cacheCommands.getEnabledExperiments()).thenReturn(List.of(litmusExperiment)); + ExperimentEntity experimentEntity = TestUtils.getExperimentEntity(); + experimentEntity.setType(ExperimentType.RELEASE); + Mockito.when(litmusCoreConfig.isFetchExperimentFromCache()).thenReturn(false); + Mockito.when(experimentQuery.findByEnabled(true)).thenReturn(List.of(experimentEntity)); + Mockito.when(experimentMapUpdater.getLitmusExperiments()).thenReturn(List.of(litmusExperiment)); LitmusExperimentCollection litmusExperimentCollection = experimentService.fetchAllExperiments(); assertEquals(1, litmusExperimentCollection.getLitmusExperiments().size()); assertEquals("test experiment", litmusExperimentCollection.getLitmusExperiments().iterator().next().getName()); + Mockito.verify(experimentQuery, Mockito.times(1)).findByEnabled(true); + Mockito.verify(experimentMapUpdater, Mockito.times(1)).getLitmusExperiments(); + } + + @Test + void shouldFetchExperimentsWillFetchExperimentFromBothCacheAndDBButHonoursTheDataFetchedFromDBIfFetchExperimentFromCacheIsFalse() { + LitmusExperiment litmusExperiment = TestUtils.getLitmusExperiment(); + litmusExperiment.setType(ExperimentType.RELEASE); + ExperimentEntity experimentEntity = TestUtils.getExperimentEntity(); + experimentEntity.setType(ExperimentType.RELEASE); + ExperimentEntity experimentEntity2 = TestUtils.getExperimentEntity(); + experimentEntity2.setType(ExperimentType.EXPERIMENT); + Mockito.when(litmusCoreConfig.isFetchExperimentFromCache()).thenReturn(false); + Mockito.when(experimentQuery.findByEnabled(true)).thenReturn(List.of(experimentEntity, experimentEntity2)); + Mockito.when(experimentMapUpdater.getLitmusExperiments()).thenReturn(List.of(litmusExperiment)); + LitmusExperimentCollection litmusExperimentCollection = experimentService.fetchAllExperiments(); + assertEquals(2, litmusExperimentCollection.getLitmusExperiments().size()); + assertEquals("test experiment", litmusExperimentCollection.getLitmusExperiments().iterator().next().getName()); + Mockito.verify(experimentQuery, Mockito.times(1)).findByEnabled(true); + Mockito.verify(experimentMapUpdater, Mockito.times(1)).getLitmusExperiments(); + } + + @Test + void shouldFetchExperimentsWillFetchExperimentOnlyFromCacheIfFetchExperimentFromCacheIsTrue() { + LitmusExperiment litmusExperiment = TestUtils.getLitmusExperiment(); + litmusExperiment.setType(ExperimentType.RELEASE); + Mockito.when(litmusCoreConfig.isFetchExperimentFromCache()).thenReturn(true); + Mockito.when(experimentMapUpdater.getLitmusExperiments()).thenReturn(List.of(litmusExperiment)); + LitmusExperimentCollection litmusExperimentCollection = experimentService.fetchAllExperiments(); + assertEquals(1, litmusExperimentCollection.getLitmusExperiments().size()); + assertEquals("test experiment", litmusExperimentCollection.getLitmusExperiments().iterator().next().getName()); + Mockito.verify(experimentQuery, Mockito.never()).findByEnabled(true); + Mockito.verify(experimentMapUpdater, Mockito.times(1)).getLitmusExperiments(); } @Test diff --git a/litmus-util/src/main/java/com/navi/medici/metrics/MetricsUtils.java b/litmus-util/src/main/java/com/navi/medici/metrics/MetricsUtils.java index 5453122..cd2c1b0 100644 --- a/litmus-util/src/main/java/com/navi/medici/metrics/MetricsUtils.java +++ b/litmus-util/src/main/java/com/navi/medici/metrics/MetricsUtils.java @@ -10,4 +10,8 @@ public class MetricsUtils { public static Counter.Builder tesseractMetrics(String name) { return Counter.builder(name); } + + public static Counter.Builder customCounterMetrics(String name) { + return Counter.builder(name); + } }