From 8d72515a3098b5c634c19442a88aab17b9ac9508 Mon Sep 17 00:00:00 2001 From: Jeffrey Griffin Date: Thu, 20 Sep 2018 00:37:05 -0700 Subject: [PATCH] Use config option to tune reconciliation instead of auto-tuning the COUNT query on the accounts db is too heavyweight and risky to justify an auto-tuning reconciliation cycle --- config/sample.yml | 9 +++-- .../textsecuregcm/WhisperServerService.java | 4 +- .../DirectoryServerConfiguration.java | 14 +++++++ .../storage/DirectoryReconciler.java | 34 ++++++---------- .../storage/DirectoryReconciliationCache.java | 23 ----------- .../storage/DirectoryReconcilerTest.java | 39 +------------------ 6 files changed, 34 insertions(+), 89 deletions(-) diff --git a/config/sample.yml b/config/sample.yml index bae920a13..cd4a92d7c 100644 --- a/config/sample.yml +++ b/config/sample.yml @@ -40,10 +40,11 @@ directory: accessSecret: # AWS SQS accessSecret queueUrl: # AWS SQS queue url server: - replicationUrl: # CDS replication endpoint base url - replicationPassword: # CDS replication endpoint password - replicationCaCertificate: # CDS replication endpoint TLS certificate trust root - + replicationUrl: # CDS replication endpoint base url + replicationPassword: # CDS replication endpoint password + replicationCaCertificate: # CDS replication endpoint TLS certificate trust root + reconciliationChunkSize: # CDS reconciliation chunk size + reconciliationChunkIntervalMs: # CDS reconciliation chunk interval, in milliseconds messageCache: # Redis server configuration for message store cache url: diff --git a/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index a1c264d18..171a1c897 100644 --- a/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -204,7 +204,9 @@ public class WhisperServerService extends Application cachedCount = reconciliationCache.getCachedAccountCount(); - - if (cachedCount.isPresent()) { - return cachedCount.get(); - } - - long count = accounts.getCount(); - reconciliationCache.setCachedAccountCount(count); - return count; - } - private synchronized boolean sleepWhileRunning(long delayMs) { long startTimeMs = System.currentTimeMillis(); while (running && delayMs > 0) { @@ -170,7 +158,7 @@ public class DirectoryReconciler implements Managed, Runnable { } private long getBoundedChunkInterval(long intervalMs) { - return Math.max(Math.min(intervalMs, MAXIMUM_CHUNK_INTERVAL), MINIMUM_CHUNK_INTERVAL); + return Math.max(Math.min(intervalMs, chunkIntervalMs), MINIMUM_CHUNK_INTERVAL); } private long getDelayWithJitter(long delayMs) { @@ -180,7 +168,7 @@ public class DirectoryReconciler implements Managed, Runnable { private boolean processChunk() { Optional fromNumber = reconciliationCache.getLastNumber(); - List chunkAccounts = readChunk(fromNumber, CHUNK_SIZE); + List chunkAccounts = readChunk(fromNumber, chunkSize); writeChunktoDirectoryCache(chunkAccounts); diff --git a/src/main/java/org/whispersystems/textsecuregcm/storage/DirectoryReconciliationCache.java b/src/main/java/org/whispersystems/textsecuregcm/storage/DirectoryReconciliationCache.java index 37cf30f60..a44c0dcff 100644 --- a/src/main/java/org/whispersystems/textsecuregcm/storage/DirectoryReconciliationCache.java +++ b/src/main/java/org/whispersystems/textsecuregcm/storage/DirectoryReconciliationCache.java @@ -29,10 +29,8 @@ public class DirectoryReconciliationCache { private static final String ACTIVE_WORKER_KEY = "directory_reconciliation_active_worker"; private static final String LAST_NUMBER_KEY = "directory_reconciliation_last_number"; - private static final String CACHED_COUNT_KEY = "directory_reconciliation_cached_count"; private static final String ACCELERATE_KEY = "directory_reconciliation_accelerate"; - private static final long CACHED_COUNT_TTL_MS = 21600_000L; private static final long LAST_NUMBER_TTL_MS = 86400_000L; private final ReplicatedJedisPool jedisPool; @@ -78,27 +76,6 @@ public class DirectoryReconciliationCache { } } - public Optional getCachedAccountCount() { - try (Jedis jedis = jedisPool.getWriteResource()) { - Optional cachedAccountCount = Optional.fromNullable(jedis.get(CACHED_COUNT_KEY)); - if (!cachedAccountCount.isPresent()) { - return Optional.absent(); - } - - try { - return Optional.of(Long.parseUnsignedLong(cachedAccountCount.get())); - } catch (NumberFormatException ex) { - return Optional.absent(); - } - } - } - - public void setCachedAccountCount(long accountCount) { - try (Jedis jedis = jedisPool.getWriteResource()) { - jedis.psetex(CACHED_COUNT_KEY, CACHED_COUNT_TTL_MS, Long.toString(accountCount)); - } - } - public static class UnlockOperation { private final LuaScript luaScript; diff --git a/src/test/java/org/whispersystems/textsecuregcm/tests/storage/DirectoryReconcilerTest.java b/src/test/java/org/whispersystems/textsecuregcm/tests/storage/DirectoryReconcilerTest.java index d90d225b8..b225272ba 100644 --- a/src/test/java/org/whispersystems/textsecuregcm/tests/storage/DirectoryReconcilerTest.java +++ b/src/test/java/org/whispersystems/textsecuregcm/tests/storage/DirectoryReconcilerTest.java @@ -35,7 +35,6 @@ public class DirectoryReconcilerTest { private static final String VALID_NUMBER = "valid"; private static final String INACTIVE_NUMBER = "inactive"; - private static final long ACCOUNT_COUNT = 0L; private static final long INTERVAL_MS = 30_000L; private final Account account = mock(Account.class); @@ -45,7 +44,7 @@ public class DirectoryReconcilerTest { private final DirectoryManager directoryManager = mock(DirectoryManager.class); private final DirectoryReconciliationClient reconciliationClient = mock(DirectoryReconciliationClient.class); private final DirectoryReconciliationCache reconciliationCache = mock(DirectoryReconciliationCache.class); - private final DirectoryReconciler directoryReconciler = new DirectoryReconciler(reconciliationClient, reconciliationCache, directoryManager, accounts); + private final DirectoryReconciler directoryReconciler = new DirectoryReconciler(reconciliationClient, reconciliationCache, directoryManager, accounts, 1000, INTERVAL_MS); private final DirectoryReconciliationResponse successResponse = new DirectoryReconciliationResponse(DirectoryReconciliationResponse.Status.OK); private final DirectoryReconciliationResponse notFoundResponse = new DirectoryReconciliationResponse(DirectoryReconciliationResponse.Status.MISSING); @@ -64,7 +63,6 @@ public class DirectoryReconcilerTest { when(accounts.getAllFrom(anyInt())).thenReturn(Arrays.asList(account, inactiveAccount)); when(accounts.getAllFrom(eq(VALID_NUMBER), anyInt())).thenReturn(Arrays.asList(inactiveAccount)); when(accounts.getAllFrom(eq(INACTIVE_NUMBER), anyInt())).thenReturn(Collections.emptyList()); - when(accounts.getCount()).thenReturn(ACCOUNT_COUNT); when(reconciliationClient.sendChunk(any())).thenReturn(successResponse); @@ -73,41 +71,6 @@ public class DirectoryReconcilerTest { when(reconciliationCache.isAccelerated()).thenReturn(false); } - @Test - public void testGetUncachedAccountCount() { - when(reconciliationCache.getCachedAccountCount()).thenReturn(Optional.absent()); - - long accountCount = directoryReconciler.getAccountCount(); - - assertThat(accountCount).isEqualTo(ACCOUNT_COUNT); - - verify(accounts, times(1)).getCount(); - - verify(reconciliationCache, times(1)).getCachedAccountCount(); - verify(reconciliationCache, times(1)).setCachedAccountCount(eq(ACCOUNT_COUNT)); - - verifyNoMoreInteractions(directoryManager); - verifyNoMoreInteractions(accounts); - verifyNoMoreInteractions(reconciliationClient); - verifyNoMoreInteractions(reconciliationCache); - } - - @Test - public void testGetCachedAccountCount() { - when(reconciliationCache.getCachedAccountCount()).thenReturn(Optional.of(ACCOUNT_COUNT)); - - long accountCount = directoryReconciler.getAccountCount(); - - assertThat(accountCount).isEqualTo(ACCOUNT_COUNT); - - verify(reconciliationCache, times(1)).getCachedAccountCount(); - - verifyNoMoreInteractions(directoryManager); - verifyNoMoreInteractions(accounts); - verifyNoMoreInteractions(reconciliationClient); - verifyNoMoreInteractions(reconciliationCache); - } - @Test public void testValid() { long delayMs = directoryReconciler.doPeriodicWork(INTERVAL_MS);