TP-62059 || Store enabled experiments in InMemoryCache instead of Redis (#103)

* TP-62059 || Store enabled experiments in InMemoryCache instead of Redis * TP-62059 || Added tests, Added feature flag to enable reading only from cache. Added metrics to compare during shadow duration.
2024-04-12 11:45:43 +05:30
parent cf97a7464d
commit 2e3889323a
8 changed files with 101 additions and 43 deletions
--- 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<String, String> rm = redissonClient.getMap(map);
        rm.remove(key);
    }
-
-    public void updateEnabledExperiments(List<LitmusExperiment> litmusExperiments) {
-        RList<LitmusExperiment> list =redissonClient.getList("enabledExperiments");
-        list.clear();
-        list.addAll(litmusExperiments);
-    }
-
-    public List<LitmusExperiment> getEnabledExperiments() {
-        RList<LitmusExperiment> list = redissonClient.getList("enabledExperiments");
-        return list.readAll();
-    }
 }
--- 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<LitmusExperiment> litmusExperiments);
-
-    List<LitmusExperiment> getEnabledExperiments();
 }
--- 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;
 }
--- a/litmus-core/src/main/java/com/navi/medici/scheduler/EnabledLitmusExperimentUpdater.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<LitmusExperiment> 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<ExperimentEntity> experimentEntities = iExperimentQuery.findByEnabled(true);
-        List<LitmusExperiment> 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<ExperimentEntity> experimentEntities = iExperimentQuery.findByEnabled(true);
+        litmusExperiments = experimentEntities.stream().map(experimentEntity -> StaticFactoryObjects.fromExperimentEntity(experimentEntity, jacksonUtils)).collect(Collectors.toList());
+    }
+
+    public List<LitmusExperiment> getLitmusExperiments() {
+        return litmusExperiments;
+    }
 }
--- 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<LitmusExperiment> litmusExperiments = cacheCommands.getEnabledExperiments();
+        List<LitmusExperiment> litmusExperiments;
+        if (litmusCoreConfig.isFetchExperimentFromCache()) {
+            litmusExperiments = enabledLitmusExperimentUpdater.getLitmusExperiments();
+        } else {
+            List<ExperimentEntity> 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<LitmusExperiment> 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<>());
--- a/litmus-core/src/main/resources/application.properties
+++ b/litmus-core/src/main/resources/application.properties
@@ -63,3 +63,4 @@ experiment.population.fetch.limit.per.interval=${EXPERIMENT_POPULATION_FETCH_LIM
 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}
+litmus.fetch.experiment.from.cache=${LITMUS_FETCH_EXPERIMENT_FROM_CACHE:true}
--- 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
--- 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);
+    }
 }