You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by dp...@apache.org on 2019/08/06 14:13:29 UTC

[ignite-teamcity-bot] branch master updated: Fixed race between caches updates: interceptor based cache was returning stale builds history.

This is an automated email from the ASF dual-hosted git repository.

dpavlov pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ignite-teamcity-bot.git


The following commit(s) were added to refs/heads/master by this push:
     new 1e998ae  Fixed race between caches updates: interceptor based cache was returning stale builds history.
1e998ae is described below

commit 1e998ae180a6e54a6f173f82ace760a48c7459bb
Author: Dmitriy Pavlov <dp...@apache.org>
AuthorDate: Tue Aug 6 17:13:23 2019 +0300

    Fixed race between caches updates: interceptor based cache was returning stale builds history.
---
 .../org/apache/ignite/ci/web/model/Version.java    |  2 +-
 .../IgnitedTcInMemoryIntegrationTest.java          | 56 +++++++++++++++++++++-
 .../ci/teamcity/ignited/runhist/RunHistKey.java    | 13 ++---
 .../ignite/tcignited/buildref/BuildRefDao.java     | 43 +++++++++++++----
 4 files changed, 97 insertions(+), 17 deletions(-)

diff --git a/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/web/model/Version.java b/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/web/model/Version.java
index 97b2e42..32536a9 100644
--- a/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/web/model/Version.java
+++ b/ignite-tc-helper-web/src/main/java/org/apache/ignite/ci/web/model/Version.java
@@ -28,7 +28,7 @@ package org.apache.ignite.ci.web.model;
     public static final String GITHUB_REF = "https://github.com/apache/ignite-teamcity-bot";
 
     /** TC Bot Version. */
-    public static final String VERSION = "20190805";
+    public static final String VERSION = "20190806";
 
     /** Java version, where Web App is running. */
     public String javaVer;
diff --git a/ignite-tc-helper-web/src/test/java/org/apache/ignite/tcignited/IgnitedTcInMemoryIntegrationTest.java b/ignite-tc-helper-web/src/test/java/org/apache/ignite/tcignited/IgnitedTcInMemoryIntegrationTest.java
index 715f7d2..3a19844 100644
--- a/ignite-tc-helper-web/src/test/java/org/apache/ignite/tcignited/IgnitedTcInMemoryIntegrationTest.java
+++ b/ignite-tc-helper-web/src/test/java/org/apache/ignite/tcignited/IgnitedTcInMemoryIntegrationTest.java
@@ -27,6 +27,7 @@ import java.io.FileWriter;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
+import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -34,9 +35,15 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.LongAdder;
+import java.util.concurrent.locks.LockSupport;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import javax.xml.bind.JAXBException;
 import org.apache.ignite.Ignite;
 import org.apache.ignite.IgniteCache;
@@ -53,6 +60,8 @@ import org.apache.ignite.jiraservice.IJiraIntegrationProvider;
 import org.apache.ignite.spi.discovery.tcp.TcpDiscoverySpi;
 import org.apache.ignite.tcbot.common.conf.IDataSourcesConfigSupplier;
 import org.apache.ignite.tcbot.common.conf.ITcServerConfig;
+import org.apache.ignite.tcbot.common.interceptor.GuavaCachedModule;
+import org.apache.ignite.tcbot.common.util.FutureUtil;
 import org.apache.ignite.tcbot.engine.conf.ITcBotConfig;
 import org.apache.ignite.tcbot.engine.conf.TcBotJsonConfig;
 import org.apache.ignite.tcbot.persistence.IStringCompactor;
@@ -734,7 +743,7 @@ public class IgnitedTcInMemoryIntegrationTest {
     public void testCachesInvalidation() {
         TeamcityIgnitedModule module = new TeamcityIgnitedModule();
 
-        Injector injector = Guice.createInjector(module, new IgniteAndSchedulerTestModule());
+        Injector injector = Guice.createInjector(module, new GuavaCachedModule(), new IgniteAndSchedulerTestModule());
 
         IStringCompactor c = injector.getInstance(IStringCompactor.class);
         BuildRefDao storage = injector.getInstance(BuildRefDao.class);
@@ -758,7 +767,50 @@ public class IgnitedTcInMemoryIntegrationTest {
         assertEquals(2, storage.getAllBuildsCompacted(srvId, buildTypeId, branchList).size());
 
         storage.save(srvId, ref.withId(idGen.incrementAndGet()).branchName(branch));
-        assertEquals(3, storage.getAllBuildsCompacted(srvId, buildTypeId, branchList).size());
+        int present = 3;
+        assertEquals(present, storage.getAllBuildsCompacted(srvId, buildTypeId, branchList).size());
+
+        LongAdder savedCnt = new LongAdder();
+        ExecutorService svc = Executors.newFixedThreadPool(10);
+        List<Future<Void>> futures = new ArrayList<>();
+        int tasks = 100;
+        int buildPerTask = 100;
+        for (int i = 0; i < tasks; i++) {
+            int finalI = i;
+            futures.add(svc.submit(() -> {
+                List<BuildRefCompacted> collect = Stream.generate(() -> new BuildRefCompacted()
+                    .withId(idGen.incrementAndGet())
+                    .state(10000 + finalI).status(1032)
+                    .branchName(branch)
+                    .buildTypeId(buildTypeId)).limit(buildPerTask).collect(Collectors.toList());
+
+                collect.forEach(bRef -> {
+                    storage.save(srvId, bRef);
+                    savedCnt.add(1);
+                });
+
+                return null;
+            }));
+        }
+
+        long ms = System.currentTimeMillis();
+        do {
+            int saved = savedCnt.intValue() + present;
+            int size = storage.getAllBuildsCompacted(srvId, buildTypeId, branchList).size();
+
+            System.out.println("Builds available " + size + " save completed for " + saved);
+            assertTrue(size >= saved);
+
+            if (savedCnt.intValue() >= tasks * buildPerTask)
+                break;
+
+            LockSupport.parkNanos(Duration.ofMillis(10).toNanos());
+        }
+        while (System.currentTimeMillis() - ms < 5000);
+
+        FutureUtil.getResults(futures);
+        svc.shutdown();
+
     }
 
     /**
diff --git a/tcbot-teamcity-ignited/src/main/java/org/apache/ignite/ci/teamcity/ignited/runhist/RunHistKey.java b/tcbot-teamcity-ignited/src/main/java/org/apache/ignite/ci/teamcity/ignited/runhist/RunHistKey.java
index a62577b..0d020db 100644
--- a/tcbot-teamcity-ignited/src/main/java/org/apache/ignite/ci/teamcity/ignited/runhist/RunHistKey.java
+++ b/tcbot-teamcity-ignited/src/main/java/org/apache/ignite/ci/teamcity/ignited/runhist/RunHistKey.java
@@ -64,17 +64,18 @@ public class RunHistKey {
         return Objects.hashCode(srvId, testOrSuiteName, branch);
     }
 
-    /**
-     *
-     */
+    /**  */
     public int testNameOrSuite() {
         return testOrSuiteName;
     }
 
-    /**
-     *
-     */
+    /** */
     public int srvId() {
         return srvId;
     }
+
+    /** */
+    public int branch() {
+        return branch;
+    }
 }
diff --git a/tcbot-teamcity-ignited/src/main/java/org/apache/ignite/tcignited/buildref/BuildRefDao.java b/tcbot-teamcity-ignited/src/main/java/org/apache/ignite/tcignited/buildref/BuildRefDao.java
index 91c73b0..654b08c 100644
--- a/tcbot-teamcity-ignited/src/main/java/org/apache/ignite/tcignited/buildref/BuildRefDao.java
+++ b/tcbot-teamcity-ignited/src/main/java/org/apache/ignite/tcignited/buildref/BuildRefDao.java
@@ -50,7 +50,6 @@ import org.apache.ignite.lang.IgnitePredicate;
 import org.apache.ignite.tcbot.common.conf.TcBotSystemProperties;
 import org.apache.ignite.tcbot.common.exeption.ExceptionUtil;
 import org.apache.ignite.tcbot.common.interceptor.AutoProfiling;
-import org.apache.ignite.tcbot.common.interceptor.GuavaCached;
 import org.apache.ignite.tcbot.persistence.CacheConfigs;
 import org.apache.ignite.tcbot.persistence.IStringCompactor;
 import org.apache.ignite.tcservice.model.hist.BuildRef;
@@ -78,11 +77,19 @@ public class BuildRefDao {
         = CacheBuilder.newBuilder()
         .maximumSize(Boolean.valueOf(System.getProperty(TcBotSystemProperties.DEV_MODE)) ? 1000 : 8000)
         .expireAfterAccess(16, TimeUnit.MINUTES)
-        .expireAfterWrite(17, TimeUnit.MINUTES) //workaround for stale records
+        .expireAfterWrite(45, TimeUnit.MINUTES) //workaround for stale records, enforcing to ask persistence sometimes.
         .softValues()
         .build();
 
-    //todo check if invalidation may be missed in this cache
+
+    /** Build refs in mem cache for all branch. Mostly this cache exist for fast building replace with recent builds. */
+    private final com.google.common.cache.Cache<Long, List<BuildRefCompacted>> buildRefsInMemCacheForAllBranch
+        = CacheBuilder.newBuilder()
+        .maximumSize(Boolean.valueOf(System.getProperty(TcBotSystemProperties.DEV_MODE)) ? 200 : 2000)
+        .expireAfterAccess(2, TimeUnit.MINUTES)
+        .expireAfterWrite(4, TimeUnit.MINUTES) //workaround for stale records
+        .softValues()
+        .build();
 
     /** */
     public BuildRefDao init() {
@@ -162,12 +169,17 @@ public class BuildRefDao {
     }
 
     public void invalidateHistoryInMem(int srvId, Stream<BuildRefCompacted> stream) {
-        Iterable<RunHistKey> objects =
-            stream
-                .map(b -> new RunHistKey(srvId, b.buildTypeId(), b.branchName()))
+        Set<RunHistKey> setOfHistToClear = stream
+            .map(b -> new RunHistKey(srvId, b.buildTypeId(), b.branchName()))
+            .collect(Collectors.toSet());
+
+        Iterable<Long> cacheForAllBranch =
+            setOfHistToClear.stream()
+                .map(b -> branchNameToHistCacheKey(srvId, b.branch()))
                 .collect(Collectors.toSet());
 
-        buildRefsInMemCache.invalidateAll(objects);
+        buildRefsInMemCacheForAllBranch.invalidateAll(cacheForAllBranch);
+        buildRefsInMemCache.invalidateAll(setOfHistToClear);
     }
 
     /**
@@ -252,6 +264,11 @@ public class BuildRefDao {
             .collect(Collectors.toList());
     }
 
+    private static long branchNameToHistCacheKey(long srvId, int branchName) {
+        return (long)branchName | srvId << 32;
+    }
+
+
     /**
      * Collects all builds for branch. Short-term cached because builds from same branch may be queued several times.
      *
@@ -259,8 +276,18 @@ public class BuildRefDao {
      * @param branchNameId  Branch name - IDs from compactor.
      */
     @AutoProfiling
-    @GuavaCached(softValues = true, maximumSize = 10000, expireAfterWriteSecs = 90)
     public List<BuildRefCompacted> getBuildsForBranch(int srvId, int branchNameId) {
+        long branchKey = branchNameToHistCacheKey(srvId, branchNameId);
+
+        try {
+            return buildRefsInMemCacheForAllBranch.get(branchKey, () -> getBuildsForBranchNonCached(srvId, branchNameId));
+        }
+        catch (ExecutionException e) {
+            throw ExceptionUtil.propagateException(e);
+        }
+    }
+
+    public List<BuildRefCompacted> getBuildsForBranchNonCached(int srvId, int branchNameId) {
         List<BuildRefCompacted> list = new ArrayList<>();
 
         try (QueryCursor<Cache.Entry<Long, BuildRefCompacted>> qryCursor = buildRefsCache.query(