Skip to content

Commit

Permalink
[FSSDK-8739] refact: Implements a warning log for polling interval le…
Browse files Browse the repository at this point in the history
…ss than 30s (#528)

* Added warning upon setting polling time period below 30 ms

* Updated test and changed expecting time to lower than 30 seconds
  • Loading branch information
mnoman09 authored Aug 3, 2023
1 parent f5528d5 commit 65b3f43
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,9 @@ public PollingProjectConfigManager(long period, TimeUnit timeUnit, long blocking
this.blockingTimeoutPeriod = blockingTimeoutPeriod;
this.blockingTimeoutUnit = blockingTimeoutUnit;
this.notificationCenter = notificationCenter;

if (TimeUnit.SECONDS.convert(period, this.timeUnit) < 30) {
logger.warn("Polling intervals below 30 seconds are not recommended.");
}
final ThreadFactory threadFactory = Executors.defaultThreadFactory();
this.scheduledExecutorService = Executors.newSingleThreadScheduledExecutor(runnable -> {
Thread thread = threadFactory.newThread(runnable);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@
*/
package com.optimizely.ab.config;

import ch.qos.logback.classic.Level;
import com.optimizely.ab.internal.LogbackVerifier;
import com.optimizely.ab.internal.NotificationRegistry;
import com.optimizely.ab.notification.NotificationCenter;
import com.optimizely.ab.notification.UpdateConfigNotification;
import org.junit.After;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import org.junit.*;
import org.junit.rules.ExpectedException;
import org.junit.rules.RuleChain;

import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CountDownLatch;
Expand All @@ -41,6 +43,13 @@ public class PollingProjectConfigManagerTest {
private static final TimeUnit POLLING_UNIT = TimeUnit.MILLISECONDS;
private static final int PROJECT_CONFIG_DELAY = 100;

public ExpectedException thrown = ExpectedException.none();
public LogbackVerifier logbackVerifier = new LogbackVerifier();

@Rule
@SuppressFBWarnings("URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
public RuleChain ruleChain = RuleChain.outerRule(thrown)
.around(logbackVerifier);
private TestProjectConfigManager testProjectConfigManager;
private ProjectConfig projectConfig;

Expand Down Expand Up @@ -228,6 +237,13 @@ public void testUpdateConfigNotificationGetsTriggered() throws InterruptedExcept
assertTrue(countDownLatch.await(500, TimeUnit.MILLISECONDS));
}

@Test
public void testSettingUpLowerPollingPeriodResultsInWarning() throws InterruptedException {
long pollingPeriod = 29;
new TestProjectConfigManager(projectConfig, pollingPeriod, TimeUnit.SECONDS, pollingPeriod / 2, TimeUnit.SECONDS, new NotificationCenter());
logbackVerifier.expectMessage(Level.WARN, "Polling intervals below 30 seconds are not recommended.");
}

@Test
public void testUpdateConfigNotificationDoesNotResultInDeadlock() throws Exception {
NotificationCenter notificationCenter = new NotificationCenter();
Expand Down Expand Up @@ -257,7 +273,11 @@ private TestProjectConfigManager(ProjectConfig projectConfig) {
}

private TestProjectConfigManager(ProjectConfig projectConfig, long blockPeriod, NotificationCenter notificationCenter) {
super(POLLING_PERIOD, POLLING_UNIT, blockPeriod, POLLING_UNIT, notificationCenter);
this(projectConfig, POLLING_PERIOD, POLLING_UNIT, blockPeriod, POLLING_UNIT, notificationCenter);
}

private TestProjectConfigManager(ProjectConfig projectConfig, long pollingPeriod, TimeUnit pollingUnit, long blockPeriod, TimeUnit blockingUnit, NotificationCenter notificationCenter) {
super(pollingPeriod, pollingUnit, blockPeriod, blockingUnit, notificationCenter);
this.projectConfig = projectConfig;
}

Expand Down

0 comments on commit 65b3f43

Please sign in to comment.