diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountCleaner.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountCleaner.java index 137fe7326..87bc26c3c 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountCleaner.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountCleaner.java @@ -4,33 +4,29 @@ */ package org.whispersystems.textsecuregcm.storage; -import com.codahale.metrics.Histogram; -import com.codahale.metrics.Meter; -import com.codahale.metrics.MetricRegistry; -import com.codahale.metrics.SharedMetricRegistries; import com.google.common.annotations.VisibleForTesting; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.whispersystems.textsecuregcm.util.Constants; -import org.whispersystems.textsecuregcm.util.Util; - import java.util.List; import java.util.Optional; import java.util.UUID; import java.util.concurrent.TimeUnit; +import io.micrometer.core.instrument.Metrics; +import io.micrometer.core.instrument.Tag; +import io.micrometer.core.instrument.Tags; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.whispersystems.textsecuregcm.util.Util; -import static com.codahale.metrics.MetricRegistry.name; +import static org.whispersystems.textsecuregcm.metrics.MetricsUtil.name; public class AccountCleaner extends AccountDatabaseCrawlerListener { private static final Logger log = LoggerFactory.getLogger(AccountCleaner.class); - private static final MetricRegistry metricRegistry = SharedMetricRegistries.getOrCreate(Constants.METRICS_NAME); - private static final Meter expiredAccountsMeter = metricRegistry.meter(name(AccountCleaner.class, "expiredAccounts")); - private static final Histogram deletableAccountHistogram = metricRegistry.histogram(name(AccountCleaner.class, "deletableAccountsPerChunk")); + private static final String DELETED_ACCOUNT_COUNTER_NAME = name(AccountCleaner.class, "deletedAccounts"); + private static final String DELETION_REASON_TAG_NAME = "reason"; @VisibleForTesting - static final int MAX_ACCOUNT_UPDATES_PER_CHUNK = 40; + static final int MAX_ACCOUNT_DELETIONS_PER_CHUNK = 40; private final AccountsManager accountsManager; @@ -48,29 +44,30 @@ public class AccountCleaner extends AccountDatabaseCrawlerListener { @Override protected void onCrawlChunk(Optional fromUuid, List chunkAccounts) { - int accountUpdateCount = 0; - int deletableAccountCount = 0; + int accountUpdateCount = 0; for (Account account : chunkAccounts) { - if (isExpired(account)) { - deletableAccountCount++; - } + if (isExpired(account) || needsExplicitRemoval(account)) { + final Tag deletionReason; - if (needsExplicitRemoval(account)) { - expiredAccountsMeter.mark(); + if (isExpired(account)) { + deletionReason = Tag.of(DELETION_REASON_TAG_NAME, "previouslyExpired"); + } else { + deletionReason = Tag.of(DELETION_REASON_TAG_NAME, "newlyExpired"); + } - if (accountUpdateCount < MAX_ACCOUNT_UPDATES_PER_CHUNK) { + if (accountUpdateCount < MAX_ACCOUNT_DELETIONS_PER_CHUNK) { try { accountsManager.delete(account, AccountsManager.DeletionReason.EXPIRED); accountUpdateCount++; + + Metrics.counter(DELETED_ACCOUNT_COUNTER_NAME, Tags.of(deletionReason)).increment(); } catch (final Exception e) { log.warn("Failed to delete account {}", account.getUuid(), e); } } } } - - deletableAccountHistogram.update(deletableAccountCount); } private boolean needsExplicitRemoval(Account account) { diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountCleanerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountCleanerTest.java index 1ddc0e457..7c4396474 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountCleanerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountCleanerTest.java @@ -21,11 +21,7 @@ import java.util.UUID; import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.BeforeEach; -import org.whispersystems.textsecuregcm.storage.Account; -import org.whispersystems.textsecuregcm.storage.AccountCleaner; -import org.whispersystems.textsecuregcm.storage.AccountDatabaseCrawlerRestartException; -import org.whispersystems.textsecuregcm.storage.AccountsManager; -import org.whispersystems.textsecuregcm.storage.Device; +import org.whispersystems.textsecuregcm.storage.AccountsManager.DeletionReason; class AccountCleanerTest { @@ -77,8 +73,8 @@ class AccountCleanerTest { accountCleaner.timeAndProcessCrawlChunk(Optional.empty(), Arrays.asList(deletedDisabledAccount, undeletedDisabledAccount, undeletedEnabledAccount)); accountCleaner.onCrawlEnd(Optional.empty()); - verify(accountsManager, never()).delete(eq(deletedDisabledAccount), any()); - verify(accountsManager).delete(undeletedDisabledAccount, AccountsManager.DeletionReason.EXPIRED); + verify(accountsManager).delete(deletedDisabledAccount, DeletionReason.EXPIRED); + verify(accountsManager).delete(undeletedDisabledAccount, DeletionReason.EXPIRED); verify(accountsManager, never()).delete(eq(undeletedEnabledAccount), any()); verifyNoMoreInteractions(accountsManager); @@ -89,7 +85,7 @@ class AccountCleanerTest { List accounts = new LinkedList<>(); accounts.add(undeletedEnabledAccount); - int activeExpiredAccountCount = AccountCleaner.MAX_ACCOUNT_UPDATES_PER_CHUNK + 1; + int activeExpiredAccountCount = AccountCleaner.MAX_ACCOUNT_DELETIONS_PER_CHUNK + 1; for (int addedAccountCount = 0; addedAccountCount < activeExpiredAccountCount; addedAccountCount++) { accounts.add(undeletedDisabledAccount); } @@ -101,7 +97,7 @@ class AccountCleanerTest { accountCleaner.timeAndProcessCrawlChunk(Optional.empty(), accounts); accountCleaner.onCrawlEnd(Optional.empty()); - verify(accountsManager, times(AccountCleaner.MAX_ACCOUNT_UPDATES_PER_CHUNK)).delete(undeletedDisabledAccount, AccountsManager.DeletionReason.EXPIRED); + verify(accountsManager, times(AccountCleaner.MAX_ACCOUNT_DELETIONS_PER_CHUNK)).delete(undeletedDisabledAccount, AccountsManager.DeletionReason.EXPIRED); verifyNoMoreInteractions(accountsManager); }