enable up to 40 account updates per chunk in AccountCleaner
This commit is contained in:
parent
3c69f81a10
commit
6877b663f1
|
@ -33,6 +33,9 @@ public class AccountCleaner implements AccountDatabaseCrawlerListener {
|
||||||
private static final MetricRegistry metricRegistry = SharedMetricRegistries.getOrCreate(Constants.METRICS_NAME);
|
private static final MetricRegistry metricRegistry = SharedMetricRegistries.getOrCreate(Constants.METRICS_NAME);
|
||||||
private static final Meter expiredAccountsMeter = metricRegistry.meter(name(AccountCleaner.class, "expiredAccounts"));
|
private static final Meter expiredAccountsMeter = metricRegistry.meter(name(AccountCleaner.class, "expiredAccounts"));
|
||||||
|
|
||||||
|
@VisibleForTesting
|
||||||
|
public static final int MAX_ACCOUNT_UPDATES_PER_CHUNK = 40;
|
||||||
|
|
||||||
private final AccountsManager accountsManager;
|
private final AccountsManager accountsManager;
|
||||||
private final DirectoryQueue directoryQueue;
|
private final DirectoryQueue directoryQueue;
|
||||||
|
|
||||||
|
@ -47,7 +50,8 @@ public class AccountCleaner implements AccountDatabaseCrawlerListener {
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public void onCrawlChunk(Optional<String> fromNumber, List<Account> chunkAccounts) {
|
public void onCrawlChunk(Optional<String> fromNumber, List<Account> chunkAccounts) {
|
||||||
long nowMs = System.currentTimeMillis();
|
long nowMs = System.currentTimeMillis();
|
||||||
|
int accountUpdateCount = 0;
|
||||||
for (Account account : chunkAccounts) {
|
for (Account account : chunkAccounts) {
|
||||||
if (account.getMasterDevice().isPresent() &&
|
if (account.getMasterDevice().isPresent() &&
|
||||||
account.getMasterDevice().get().isActive() &&
|
account.getMasterDevice().get().isActive() &&
|
||||||
|
@ -60,7 +64,10 @@ public class AccountCleaner implements AccountDatabaseCrawlerListener {
|
||||||
masterDevice.setApnId(null);
|
masterDevice.setApnId(null);
|
||||||
masterDevice.setGcmId(null);
|
masterDevice.setGcmId(null);
|
||||||
|
|
||||||
// accountsManager.update(account);
|
if (accountUpdateCount < MAX_ACCOUNT_UPDATES_PER_CHUNK) {
|
||||||
|
accountUpdateCount++;
|
||||||
|
accountsManager.update(account);
|
||||||
|
}
|
||||||
directoryQueue.deleteRegisteredUser(account.getNumber());
|
directoryQueue.deleteRegisteredUser(account.getNumber());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -25,6 +25,7 @@ import org.whispersystems.textsecuregcm.storage.AccountsManager;
|
||||||
import org.whispersystems.textsecuregcm.storage.Device;
|
import org.whispersystems.textsecuregcm.storage.Device;
|
||||||
import org.whispersystems.textsecuregcm.tests.util.AuthHelper;
|
import org.whispersystems.textsecuregcm.tests.util.AuthHelper;
|
||||||
|
|
||||||
|
import java.util.ArrayList;
|
||||||
import java.util.Arrays;
|
import java.util.Arrays;
|
||||||
import java.util.HashSet;
|
import java.util.HashSet;
|
||||||
import java.util.Optional;
|
import java.util.Optional;
|
||||||
|
@ -34,8 +35,10 @@ import static org.junit.Assert.assertFalse;
|
||||||
import static org.junit.Assert.assertTrue;
|
import static org.junit.Assert.assertTrue;
|
||||||
import static org.mockito.ArgumentMatchers.eq;
|
import static org.mockito.ArgumentMatchers.eq;
|
||||||
import static org.mockito.ArgumentMatchers.isNull;
|
import static org.mockito.ArgumentMatchers.isNull;
|
||||||
|
import static org.mockito.Mockito.atLeast;
|
||||||
import static org.mockito.Mockito.atLeastOnce;
|
import static org.mockito.Mockito.atLeastOnce;
|
||||||
import static org.mockito.Mockito.mock;
|
import static org.mockito.Mockito.mock;
|
||||||
|
import static org.mockito.Mockito.times;
|
||||||
import static org.mockito.Mockito.verify;
|
import static org.mockito.Mockito.verify;
|
||||||
import static org.mockito.Mockito.verifyNoMoreInteractions;
|
import static org.mockito.Mockito.verifyNoMoreInteractions;
|
||||||
import static org.mockito.Mockito.when;
|
import static org.mockito.Mockito.when;
|
||||||
|
@ -135,7 +138,7 @@ public class AccountCleanerTest {
|
||||||
verify(activeExpiredDevice).setApnId(isNull());
|
verify(activeExpiredDevice).setApnId(isNull());
|
||||||
verify(activeExpiredDevice).setFetchesMessages(eq(false));
|
verify(activeExpiredDevice).setFetchesMessages(eq(false));
|
||||||
|
|
||||||
//verify(accountsManager).update(eq(activeExpiredAccount));
|
verify(accountsManager).update(eq(activeExpiredAccount));
|
||||||
verify(directoryQueue).deleteRegisteredUser(eq(AuthHelper.VALID_NUMBER));
|
verify(directoryQueue).deleteRegisteredUser(eq(AuthHelper.VALID_NUMBER));
|
||||||
|
|
||||||
verify(activeUnexpiredDevice, atLeastOnce()).isActive();
|
verify(activeUnexpiredDevice, atLeastOnce()).isActive();
|
||||||
|
@ -152,6 +155,46 @@ public class AccountCleanerTest {
|
||||||
verifyNoMoreInteractions(directoryQueue);
|
verifyNoMoreInteractions(directoryQueue);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testMaxAccountUpdates() {
|
||||||
|
ArrayList<Account> accounts = new ArrayList<>();
|
||||||
|
accounts.add(activeUnexpiredAccount);
|
||||||
|
|
||||||
|
int activeExpiredAccountCount = AccountCleaner.MAX_ACCOUNT_UPDATES_PER_CHUNK + 1;
|
||||||
|
for (int addedAccountCount = 0; addedAccountCount < activeExpiredAccountCount; addedAccountCount++) {
|
||||||
|
accounts.add(activeExpiredAccount);
|
||||||
|
}
|
||||||
|
|
||||||
|
accounts.add(inactiveUnexpiredAccount);
|
||||||
|
accounts.add(inactiveExpiredAccount);
|
||||||
|
|
||||||
|
AccountCleaner accountCleaner = new AccountCleaner(accountsManager, directoryQueue);
|
||||||
|
accountCleaner.onCrawlStart();
|
||||||
|
accountCleaner.onCrawlChunk(Optional.empty(), accounts);
|
||||||
|
accountCleaner.onCrawlEnd(Optional.empty());
|
||||||
|
|
||||||
|
verify(activeExpiredDevice, atLeast(0)).setGcmId(isNull());
|
||||||
|
verify(activeExpiredDevice, atLeast(0)).setApnId(isNull());
|
||||||
|
verify(activeExpiredDevice, atLeast(0)).setFetchesMessages(eq(false));
|
||||||
|
|
||||||
|
verify(accountsManager, times(AccountCleaner.MAX_ACCOUNT_UPDATES_PER_CHUNK)).update(eq(activeExpiredAccount));
|
||||||
|
verify(directoryQueue, times(activeExpiredAccountCount)).deleteRegisteredUser(eq(AuthHelper.VALID_NUMBER));
|
||||||
|
|
||||||
|
verify(activeUnexpiredDevice, atLeastOnce()).isActive();
|
||||||
|
verify(activeExpiredDevice, atLeastOnce()).isActive();
|
||||||
|
verify(inactiveUnexpiredDevice, atLeastOnce()).isActive();
|
||||||
|
verify(inactiveExpiredDevice, atLeastOnce()).isActive();
|
||||||
|
|
||||||
|
verifyNoMoreInteractions(activeUnexpiredDevice);
|
||||||
|
verifyNoMoreInteractions(activeExpiredDevice);
|
||||||
|
verifyNoMoreInteractions(inactiveUnexpiredDevice);
|
||||||
|
verifyNoMoreInteractions(inactiveExpiredDevice);
|
||||||
|
|
||||||
|
verifyNoMoreInteractions(accountsManager);
|
||||||
|
verifyNoMoreInteractions(directoryQueue);
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testIsAccountExpired() {
|
public void testIsAccountExpired() {
|
||||||
Account recentAccount = new Account("+14152222222", new HashSet<Device>() {{
|
Account recentAccount = new Account("+14152222222", new HashSet<Device>() {{
|
||||||
|
|
Loading…
Reference in New Issue