From b04667fef68c36692da437b755fcbad2692df8fc Mon Sep 17 00:00:00 2001 From: Nadia Mayor Date: Mon, 26 Feb 2024 17:02:27 -0300 Subject: [PATCH 1/9] [SDKS-8067] Fix deadlock when calling sendUniqueKeys --- .../io/split/client/impressions/UniqueKeysTrackerImp.java | 4 ++-- .../split/client/impressions/filters/BloomFilterImp.java | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/client/src/main/java/io/split/client/impressions/UniqueKeysTrackerImp.java b/client/src/main/java/io/split/client/impressions/UniqueKeysTrackerImp.java index b6694a9be..81b313473 100644 --- a/client/src/main/java/io/split/client/impressions/UniqueKeysTrackerImp.java +++ b/client/src/main/java/io/split/client/impressions/UniqueKeysTrackerImp.java @@ -46,7 +46,7 @@ public UniqueKeysTrackerImp(TelemetrySynchronizer telemetrySynchronizer, int uni } @Override - public synchronized boolean track(String featureFlagName, String key) { + public boolean track(String featureFlagName, String key) { if (!filterAdapter.add(featureFlagName, key)) { _logger.debug("The feature flag " + featureFlagName + " and key " + key + " exist in the UniqueKeysTracker"); return false; @@ -106,7 +106,7 @@ public HashMap> popAll(){ return toReturn; } - private void sendUniqueKeys(){ + private synchronized void sendUniqueKeys(){ if (uniqueKeysTracker.size() == 0) { _log.warn("The Unique Keys Tracker is empty"); return; diff --git a/client/src/main/java/io/split/client/impressions/filters/BloomFilterImp.java b/client/src/main/java/io/split/client/impressions/filters/BloomFilterImp.java index 554964d16..9bf82f81e 100644 --- a/client/src/main/java/io/split/client/impressions/filters/BloomFilterImp.java +++ b/client/src/main/java/io/split/client/impressions/filters/BloomFilterImp.java @@ -6,7 +6,7 @@ public class BloomFilterImp implements Filter { - private BloomFilter bloomFilter; + private BloomFilter bloomFilter; private final int size; private final double errorMargin; @@ -17,17 +17,17 @@ public BloomFilterImp(int size, double errorMargin) { } @Override - public synchronized boolean add(String data) { + public boolean add(String data) { return bloomFilter.put(data); } @Override - public synchronized boolean contains(String data) { + public boolean contains(String data) { return bloomFilter.mightContain(data); } @Override - public synchronized void clear() { + public void clear() { bloomFilter = BloomFilter.create(Funnels.stringFunnel(Charsets.UTF_16), size, errorMargin); } From 2bca1c83a76a62394d4e5edfe44954e5e960a218 Mon Sep 17 00:00:00 2001 From: Nadia Mayor Date: Mon, 26 Feb 2024 18:09:29 -0300 Subject: [PATCH 2/9] Update client version --- client/pom.xml | 2 +- pluggable-storage/pom.xml | 2 +- pom.xml | 2 +- redis-wrapper/pom.xml | 2 +- testing/pom.xml | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/client/pom.xml b/client/pom.xml index 0b4f93bd7..06ec6a81a 100644 --- a/client/pom.xml +++ b/client/pom.xml @@ -5,7 +5,7 @@ io.split.client java-client-parent - 4.11.0 + 4.11.1-rc1 java-client jar diff --git a/pluggable-storage/pom.xml b/pluggable-storage/pom.xml index 0d6bd2656..8b212e1ed 100644 --- a/pluggable-storage/pom.xml +++ b/pluggable-storage/pom.xml @@ -6,7 +6,7 @@ java-client-parent io.split.client - 4.11.0 + 4.11.1-rc1 2.1.0 diff --git a/pom.xml b/pom.xml index d186fdd63..2bd329249 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ 4.0.0 io.split.client java-client-parent - 4.11.0 + 4.11.1-rc1 diff --git a/redis-wrapper/pom.xml b/redis-wrapper/pom.xml index f33f560a7..911a9ed82 100644 --- a/redis-wrapper/pom.xml +++ b/redis-wrapper/pom.xml @@ -6,7 +6,7 @@ java-client-parent io.split.client - 4.11.0 + 4.11.1-rc1 redis-wrapper 3.1.0 diff --git a/testing/pom.xml b/testing/pom.xml index a7f75d1bc..f32ae3701 100644 --- a/testing/pom.xml +++ b/testing/pom.xml @@ -5,7 +5,7 @@ io.split.client java-client-parent - 4.11.0 + 4.11.1-rc1 java-client-testing jar From dfc5065ff4ffb3568a13c989115eef6b3c469908 Mon Sep 17 00:00:00 2001 From: Nadia Mayor Date: Tue, 27 Feb 2024 12:34:19 -0300 Subject: [PATCH 3/9] Update UniqueKeysTracker to use traffic lights when clean Filter --- client/pom.xml | 2 +- .../impressions/UniqueKeysTrackerImp.java | 45 ++++++++++++------- pluggable-storage/pom.xml | 2 +- pom.xml | 2 +- redis-wrapper/pom.xml | 2 +- testing/pom.xml | 2 +- 6 files changed, 33 insertions(+), 22 deletions(-) diff --git a/client/pom.xml b/client/pom.xml index 06ec6a81a..0dd79b071 100644 --- a/client/pom.xml +++ b/client/pom.xml @@ -5,7 +5,7 @@ io.split.client java-client-parent - 4.11.1-rc1 + 4.11.1-rc2 java-client jar diff --git a/client/src/main/java/io/split/client/impressions/UniqueKeysTrackerImp.java b/client/src/main/java/io/split/client/impressions/UniqueKeysTrackerImp.java index 81b313473..0e2bb60d6 100644 --- a/client/src/main/java/io/split/client/impressions/UniqueKeysTrackerImp.java +++ b/client/src/main/java/io/split/client/impressions/UniqueKeysTrackerImp.java @@ -14,10 +14,12 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; public class UniqueKeysTrackerImp implements UniqueKeysTracker{ private static final Logger _log = LoggerFactory.getLogger(UniqueKeysTrackerImp.class); @@ -31,6 +33,7 @@ public class UniqueKeysTrackerImp implements UniqueKeysTracker{ private final ConcurrentHashMap> uniqueKeysTracker; private final int _uniqueKeysRefreshRate; private final int _filterRefreshRate; + private final AtomicBoolean sendGuard = new AtomicBoolean(false); private static final Logger _logger = LoggerFactory.getLogger(UniqueKeysTrackerImp.class); public UniqueKeysTrackerImp(TelemetrySynchronizer telemetrySynchronizer, int uniqueKeysRefreshRate, int filterRefreshRate, @@ -51,14 +54,14 @@ public boolean track(String featureFlagName, String key) { _logger.debug("The feature flag " + featureFlagName + " and key " + key + " exist in the UniqueKeysTracker"); return false; } - HashSet value = new HashSet<>(); - if(uniqueKeysTracker.containsKey(featureFlagName)){ - value = uniqueKeysTracker.get(featureFlagName); - } - value.add(key); - uniqueKeysTracker.put(featureFlagName, value); + uniqueKeysTracker.compute(featureFlagName, + (feature, current) -> { + HashSet keysByFeature = Optional.ofNullable(current).orElse(new HashSet<>()); + keysByFeature.add(key); + return keysByFeature; + }); _logger.debug("The feature flag " + featureFlagName + " and key " + key + " was added"); - if (uniqueKeysTracker.size() == MAX_AMOUNT_OF_TRACKED_UNIQUE_KEYS){ + if (uniqueKeysTracker.size() >= MAX_AMOUNT_OF_TRACKED_UNIQUE_KEYS){ _logger.warn("The UniqueKeysTracker size reached the maximum limit"); try { sendUniqueKeys(); @@ -106,18 +109,26 @@ public HashMap> popAll(){ return toReturn; } - private synchronized void sendUniqueKeys(){ - if (uniqueKeysTracker.size() == 0) { - _log.warn("The Unique Keys Tracker is empty"); - return; + private void sendUniqueKeys(){ + if (!sendGuard.compareAndSet(false, true)) { + _log.debug("SendUniqueKeys already running"); + return; } - HashMap> uniqueKeysHashMap = popAll(); - List uniqueKeysFromPopAll = new ArrayList<>(); - for (String featureFlag : uniqueKeysHashMap.keySet()) { - UniqueKeys.UniqueKey uniqueKey = new UniqueKeys.UniqueKey(featureFlag, new ArrayList<>(uniqueKeysHashMap.get(featureFlag))); - uniqueKeysFromPopAll.add(uniqueKey); + try { + if (uniqueKeysTracker.size() == 0) { + _log.warn("The Unique Keys Tracker is empty"); + return; + } + HashMap> uniqueKeysHashMap = popAll(); + List uniqueKeysFromPopAll = new ArrayList<>(); + for (String feature : uniqueKeysHashMap.keySet()) { + UniqueKeys.UniqueKey uniqueKey = new UniqueKeys.UniqueKey(feature, new ArrayList<>(uniqueKeysHashMap.get(feature))); + uniqueKeysFromPopAll.add(uniqueKey); + } + _telemetrySynchronizer.synchronizeUniqueKeys(new UniqueKeys(uniqueKeysFromPopAll)); + } finally { + sendGuard.set(false); } - _telemetrySynchronizer.synchronizeUniqueKeys(new UniqueKeys(uniqueKeysFromPopAll)); } private interface ExecuteUniqueKeysAction{ diff --git a/pluggable-storage/pom.xml b/pluggable-storage/pom.xml index 8b212e1ed..670ac1335 100644 --- a/pluggable-storage/pom.xml +++ b/pluggable-storage/pom.xml @@ -6,7 +6,7 @@ java-client-parent io.split.client - 4.11.1-rc1 + 4.11.1-rc2 2.1.0 diff --git a/pom.xml b/pom.xml index 2bd329249..97e45a641 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ 4.0.0 io.split.client java-client-parent - 4.11.1-rc1 + 4.11.1-rc2 diff --git a/redis-wrapper/pom.xml b/redis-wrapper/pom.xml index 911a9ed82..81a08d6e9 100644 --- a/redis-wrapper/pom.xml +++ b/redis-wrapper/pom.xml @@ -6,7 +6,7 @@ java-client-parent io.split.client - 4.11.1-rc1 + 4.11.1-rc2 redis-wrapper 3.1.0 diff --git a/testing/pom.xml b/testing/pom.xml index f32ae3701..5c73198f6 100644 --- a/testing/pom.xml +++ b/testing/pom.xml @@ -5,7 +5,7 @@ io.split.client java-client-parent - 4.11.1-rc1 + 4.11.1-rc2 java-client-testing jar From 6c6705bc885f5c37cba59a255b22d33e45a55b5d Mon Sep 17 00:00:00 2001 From: Matias Melograno Date: Tue, 27 Feb 2024 18:01:48 -0300 Subject: [PATCH 4/9] logs --- client/pom.xml | 2 +- client/src/main/java/io/split/engine/common/SyncManagerImp.java | 2 +- client/src/main/java/io/split/engine/sse/AuthApiClientImp.java | 2 +- pluggable-storage/pom.xml | 2 +- pom.xml | 2 +- redis-wrapper/pom.xml | 2 +- testing/pom.xml | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/client/pom.xml b/client/pom.xml index 0dd79b071..6246c3d0e 100644 --- a/client/pom.xml +++ b/client/pom.xml @@ -5,7 +5,7 @@ io.split.client java-client-parent - 4.11.1-rc2 + 4.11.1-rc3 java-client jar diff --git a/client/src/main/java/io/split/engine/common/SyncManagerImp.java b/client/src/main/java/io/split/engine/common/SyncManagerImp.java index dcebbc100..4f5ce3714 100644 --- a/client/src/main/java/io/split/engine/common/SyncManagerImp.java +++ b/client/src/main/java/io/split/engine/common/SyncManagerImp.java @@ -29,7 +29,7 @@ import static io.split.client.utils.SplitExecutorFactory.buildExecutorService; public class SyncManagerImp implements SyncManager { - private static final Logger _log = LoggerFactory.getLogger(SyncManager.class); + private static final Logger _log = LoggerFactory.getLogger(SyncManagerImp.class); private final AtomicBoolean _streamingEnabledConfig; private final Synchronizer _synchronizer; diff --git a/client/src/main/java/io/split/engine/sse/AuthApiClientImp.java b/client/src/main/java/io/split/engine/sse/AuthApiClientImp.java index 4f39baa6c..9f750ada3 100644 --- a/client/src/main/java/io/split/engine/sse/AuthApiClientImp.java +++ b/client/src/main/java/io/split/engine/sse/AuthApiClientImp.java @@ -22,7 +22,7 @@ import static com.google.common.base.Preconditions.checkNotNull; public class AuthApiClientImp implements AuthApiClient { - private static final Logger _log = LoggerFactory.getLogger(AuthApiClient.class); + private static final Logger _log = LoggerFactory.getLogger(AuthApiClientImp.class); private final CloseableHttpClient _httpClient; private final String _target; diff --git a/pluggable-storage/pom.xml b/pluggable-storage/pom.xml index 670ac1335..dbb5705b3 100644 --- a/pluggable-storage/pom.xml +++ b/pluggable-storage/pom.xml @@ -6,7 +6,7 @@ java-client-parent io.split.client - 4.11.1-rc2 + 4.11.1-rc3 2.1.0 diff --git a/pom.xml b/pom.xml index 97e45a641..f484ed762 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ 4.0.0 io.split.client java-client-parent - 4.11.1-rc2 + 4.11.1-rc3 diff --git a/redis-wrapper/pom.xml b/redis-wrapper/pom.xml index 81a08d6e9..080686bce 100644 --- a/redis-wrapper/pom.xml +++ b/redis-wrapper/pom.xml @@ -6,7 +6,7 @@ java-client-parent io.split.client - 4.11.1-rc2 + 4.11.1-rc3 redis-wrapper 3.1.0 diff --git a/testing/pom.xml b/testing/pom.xml index 5c73198f6..f3019c10c 100644 --- a/testing/pom.xml +++ b/testing/pom.xml @@ -5,7 +5,7 @@ io.split.client java-client-parent - 4.11.1-rc2 + 4.11.1-rc3 java-client-testing jar From 0dd75fba1400e23893484822815dea8df58b63a0 Mon Sep 17 00:00:00 2001 From: Nadia Mayor Date: Wed, 28 Feb 2024 09:48:45 -0300 Subject: [PATCH 5/9] Update to remove smell code for sonar kube --- .../split/client/impressions/UniqueKeysTrackerImp.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/client/src/main/java/io/split/client/impressions/UniqueKeysTrackerImp.java b/client/src/main/java/io/split/client/impressions/UniqueKeysTrackerImp.java index 0e2bb60d6..bd2ff9185 100644 --- a/client/src/main/java/io/split/client/impressions/UniqueKeysTrackerImp.java +++ b/client/src/main/java/io/split/client/impressions/UniqueKeysTrackerImp.java @@ -14,6 +14,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ScheduledExecutorService; @@ -121,8 +122,8 @@ private void sendUniqueKeys(){ } HashMap> uniqueKeysHashMap = popAll(); List uniqueKeysFromPopAll = new ArrayList<>(); - for (String feature : uniqueKeysHashMap.keySet()) { - UniqueKeys.UniqueKey uniqueKey = new UniqueKeys.UniqueKey(feature, new ArrayList<>(uniqueKeysHashMap.get(feature))); + for (Map.Entry> uniqueKeyEntry : uniqueKeysHashMap.entrySet()) { + UniqueKeys.UniqueKey uniqueKey = new UniqueKeys.UniqueKey(uniqueKeyEntry.getKey(), new ArrayList<>(uniqueKeyEntry.getValue())); uniqueKeysFromPopAll.add(uniqueKey); } _telemetrySynchronizer.synchronizeUniqueKeys(new UniqueKeys(uniqueKeysFromPopAll)); @@ -149,4 +150,8 @@ public void execute() { sendUniqueKeys(); } } + + public AtomicBoolean getSendGuard() { + return sendGuard; + } } From fa807c4c38e91cfe248f44b59c795bbcd1ed4533 Mon Sep 17 00:00:00 2001 From: Nadia Mayor Date: Wed, 28 Feb 2024 10:41:52 -0300 Subject: [PATCH 6/9] Add test case for sendGuard --- .../io/split/client/impressions/UniqueKeysTrackerImpTest.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/client/src/test/java/io/split/client/impressions/UniqueKeysTrackerImpTest.java b/client/src/test/java/io/split/client/impressions/UniqueKeysTrackerImpTest.java index 5edd8e531..354c40e51 100644 --- a/client/src/test/java/io/split/client/impressions/UniqueKeysTrackerImpTest.java +++ b/client/src/test/java/io/split/client/impressions/UniqueKeysTrackerImpTest.java @@ -90,10 +90,13 @@ public void testStopSynchronization() throws Exception { TelemetrySynchronizer telemetrySynchronizer = Mockito.mock(TelemetryInMemorySubmitter.class); UniqueKeysTrackerImp uniqueKeysTrackerImp = new UniqueKeysTrackerImp(telemetrySynchronizer, 1, 2, null); uniqueKeysTrackerImp.start(); + Assert.assertFalse(uniqueKeysTrackerImp.getSendGuard().get()); Assert.assertTrue(uniqueKeysTrackerImp.track("feature1","key1")); Assert.assertTrue(uniqueKeysTrackerImp.track("feature1","key2")); Assert.assertTrue(uniqueKeysTrackerImp.track("feature2","key3")); + Thread.sleep(1000); + Assert.assertTrue(uniqueKeysTrackerImp.getSendGuard().get()); Thread.sleep(2100); Mockito.verify(telemetrySynchronizer, Mockito.times(1)).synchronizeUniqueKeys(Mockito.anyObject()); uniqueKeysTrackerImp.stop(); From 70b6096893863ffd79a892a985a5625609ccc976 Mon Sep 17 00:00:00 2001 From: Nadia Mayor Date: Wed, 28 Feb 2024 10:58:13 -0300 Subject: [PATCH 7/9] Update test case --- .../io/split/client/impressions/UniqueKeysTrackerImpTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/client/src/test/java/io/split/client/impressions/UniqueKeysTrackerImpTest.java b/client/src/test/java/io/split/client/impressions/UniqueKeysTrackerImpTest.java index 354c40e51..a15940110 100644 --- a/client/src/test/java/io/split/client/impressions/UniqueKeysTrackerImpTest.java +++ b/client/src/test/java/io/split/client/impressions/UniqueKeysTrackerImpTest.java @@ -95,8 +95,6 @@ public void testStopSynchronization() throws Exception { Assert.assertTrue(uniqueKeysTrackerImp.track("feature1","key2")); Assert.assertTrue(uniqueKeysTrackerImp.track("feature2","key3")); - Thread.sleep(1000); - Assert.assertTrue(uniqueKeysTrackerImp.getSendGuard().get()); Thread.sleep(2100); Mockito.verify(telemetrySynchronizer, Mockito.times(1)).synchronizeUniqueKeys(Mockito.anyObject()); uniqueKeysTrackerImp.stop(); From b07906d4d6d8ae1677fbe7d622db6e7cb10186aa Mon Sep 17 00:00:00 2001 From: Nadia Mayor Date: Wed, 28 Feb 2024 14:20:43 -0300 Subject: [PATCH 8/9] Update changelog --- CHANGES.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES.txt b/CHANGES.txt index 888b21421..6e21a62e5 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,3 +1,6 @@ +4.11.1 (Feb 28, 2024) +- Fixed deadlock in UniqueKeysTracker when sending Unique Keys. + 4.11.0 (Jan 9, 2024) - Added impressionsListener method in the IntegrationConfig builder to set Sync or Async Listener execution. - Fixed localhost to read files with yml ending. From 0d5267e8aa8409af2c55cdb558661eee5ceedf52 Mon Sep 17 00:00:00 2001 From: Nadia Mayor Date: Thu, 29 Feb 2024 13:31:19 -0300 Subject: [PATCH 9/9] Update version and logchange --- CHANGES.txt | 2 +- client/pom.xml | 2 +- pluggable-storage/pom.xml | 2 +- pom.xml | 2 +- redis-wrapper/pom.xml | 2 +- testing/pom.xml | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 6e21a62e5..ec4d34a39 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,4 @@ -4.11.1 (Feb 28, 2024) +4.11.1 (Feb 29, 2024) - Fixed deadlock in UniqueKeysTracker when sending Unique Keys. 4.11.0 (Jan 9, 2024) diff --git a/client/pom.xml b/client/pom.xml index 6246c3d0e..131fe248c 100644 --- a/client/pom.xml +++ b/client/pom.xml @@ -5,7 +5,7 @@ io.split.client java-client-parent - 4.11.1-rc3 + 4.11.1 java-client jar diff --git a/pluggable-storage/pom.xml b/pluggable-storage/pom.xml index dbb5705b3..b797e50fe 100644 --- a/pluggable-storage/pom.xml +++ b/pluggable-storage/pom.xml @@ -6,7 +6,7 @@ java-client-parent io.split.client - 4.11.1-rc3 + 4.11.1 2.1.0 diff --git a/pom.xml b/pom.xml index f484ed762..4c5c2871b 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ 4.0.0 io.split.client java-client-parent - 4.11.1-rc3 + 4.11.1 diff --git a/redis-wrapper/pom.xml b/redis-wrapper/pom.xml index 080686bce..2f9da5c8c 100644 --- a/redis-wrapper/pom.xml +++ b/redis-wrapper/pom.xml @@ -6,7 +6,7 @@ java-client-parent io.split.client - 4.11.1-rc3 + 4.11.1 redis-wrapper 3.1.0 diff --git a/testing/pom.xml b/testing/pom.xml index f3019c10c..d3f815e27 100644 --- a/testing/pom.xml +++ b/testing/pom.xml @@ -5,7 +5,7 @@ io.split.client java-client-parent - 4.11.1-rc3 + 4.11.1 java-client-testing jar