diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java index cd2c6ce60..2c765161d 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java @@ -403,41 +403,42 @@ public class AccountsManager { public void delete(final Account account, final DeletionReason deletionReason) throws InterruptedException { try (final Timer.Context ignored = deleteTimer.time()) { - final CompletableFuture deleteStorageServiceDataFuture = secureStorageClient.deleteStoredData(account.getUuid()); - final CompletableFuture deleteBackupServiceDataFuture = secureBackupClient.deleteBackups(account.getUuid()); + deletedAccountsManager.lockAndPut(account.getNumber(), () -> { + final CompletableFuture deleteStorageServiceDataFuture = secureStorageClient.deleteStoredData(account.getUuid()); + final CompletableFuture deleteBackupServiceDataFuture = secureBackupClient.deleteBackups(account.getUuid()); - usernamesManager.delete(account.getUuid()); - directoryQueue.deleteAccount(account); - profilesManager.deleteAll(account.getUuid()); - keysDynamoDb.delete(account.getUuid()); - messagesManager.clear(account.getUuid()); + usernamesManager.delete(account.getUuid()); + directoryQueue.deleteAccount(account); + profilesManager.deleteAll(account.getUuid()); + keysDynamoDb.delete(account.getUuid()); + messagesManager.clear(account.getUuid()); - deleteStorageServiceDataFuture.join(); - deleteBackupServiceDataFuture.join(); + deleteStorageServiceDataFuture.join(); + deleteBackupServiceDataFuture.join(); - redisDelete(account); - databaseDelete(account); + redisDelete(account); + databaseDelete(account); - if (dynamoDeleteEnabled()) { + if (dynamoDeleteEnabled()) { try { dynamoDelete(account); } catch (final Exception e) { logger.error("Could not delete account {} from dynamo", account.getUuid().toString()); Metrics.counter(DYNAMO_MIGRATION_ERROR_COUNTER_NAME, "action", "delete").increment(); } - } - - deletedAccountsManager.addRecentlyDeletedAccount(account.getUuid(), account.getNumber()); + } + return account.getUuid(); + }); } catch (final RuntimeException | InterruptedException e) { logger.warn("Failed to delete account", e); throw e; } Metrics.counter(DELETE_COUNTER_NAME, - COUNTRY_CODE_TAG_NAME, Util.getCountryCode(account.getNumber()), - DELETION_REASON_TAG_NAME, deletionReason.tagValue) - .increment(); + COUNTRY_CODE_TAG_NAME, Util.getCountryCode(account.getNumber()), + DELETION_REASON_TAG_NAME, deletionReason.tagValue) + .increment(); } private String getAccountMapKey(String number) { diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/DeletedAccountsManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/DeletedAccountsManager.java index 6a3d05b67..2bb39114c 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/DeletedAccountsManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/DeletedAccountsManager.java @@ -20,6 +20,7 @@ import java.util.Set; import java.util.UUID; import java.util.concurrent.TimeUnit; import java.util.function.Consumer; +import java.util.function.Supplier; import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -60,10 +61,6 @@ public class DeletedAccountsManager { .build()); } - public void addRecentlyDeletedAccount(final UUID uuid, final String e164) throws InterruptedException { - withLock(e164, () -> deletedAccounts.put(uuid, e164, true)); - } - public void lockAndTake(final String e164, final Consumer> consumer) throws InterruptedException { withLock(e164, () -> { try { @@ -75,6 +72,16 @@ public class DeletedAccountsManager { }); } + public void lockAndPut(final String e164, final Supplier supplier) throws InterruptedException { + withLock(e164, () -> { + try { + deletedAccounts.put(supplier.get(), e164, true); + } catch (final Exception e) { + log.warn("Supplier threw an exception while holding lock on a deleted account record", e); + } + }); + } + private void withLock(final String e164, final Runnable task) throws InterruptedException { final LockItem lockItem = lockClient.acquireLock(AcquireLockOptions.builder(e164) .withAcquireReleasedLocksConsistently(true) diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/DeletedAccountsManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/DeletedAccountsManagerTest.java index 2661706c4..afe8d851d 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/DeletedAccountsManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/DeletedAccountsManagerTest.java @@ -79,7 +79,7 @@ class DeletedAccountsManagerTest { final UUID uuid = UUID.randomUUID(); final String e164 = "+18005551234"; - deletedAccountsManager.addRecentlyDeletedAccount(uuid, e164); + deletedAccounts.put(uuid, e164, true); deletedAccountsManager.lockAndTake(e164, maybeUuid -> assertEquals(Optional.of(uuid), maybeUuid)); assertEquals(Optional.empty(), deletedAccounts.findUuid(e164)); } @@ -89,7 +89,7 @@ class DeletedAccountsManagerTest { final UUID uuid = UUID.randomUUID(); final String e164 = "+18005551234"; - deletedAccountsManager.addRecentlyDeletedAccount(uuid, e164); + deletedAccounts.put(uuid, e164, true); deletedAccountsManager.lockAndTake(e164, maybeUuid -> { assertEquals(Optional.of(uuid), maybeUuid); @@ -100,7 +100,7 @@ class DeletedAccountsManagerTest { } @Test - void testReconciliationLockContention() throws ChunkProcessingFailedException, InterruptedException { + void testReconciliationLockContention() throws ChunkProcessingFailedException { final UUID[] uuids = new UUID[3]; final String[] e164s = new String[uuids.length]; @@ -113,7 +113,7 @@ class DeletedAccountsManagerTest { final Map expectedReconciledAccounts = new HashMap<>(); for (int i = 0; i < uuids.length; i++) { - deletedAccountsManager.addRecentlyDeletedAccount(uuids[i], e164s[i]); + deletedAccounts.put(uuids[i], e164s[i], true); expectedReconciledAccounts.put(e164s[i], uuids[i]); } @@ -122,7 +122,7 @@ class DeletedAccountsManagerTest { final Thread putThread = new Thread(() -> { try { - deletedAccountsManager.addRecentlyDeletedAccount(replacedUUID, e164s[0]); + deletedAccountsManager.lockAndPut(e164s[0], () -> replacedUUID); } catch (InterruptedException e) { throw new RuntimeException(e); }