Simplify optimistic write logic
This commit is contained in:
		
							parent
							
								
									23f9199439
								
							
						
					
					
						commit
						bcb89924b4
					
				| 
						 | 
					@ -293,47 +293,21 @@ public class AccountsManager {
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    try (Timer.Context ignored = updateTimer.time()) {
 | 
					    try (Timer.Context ignored = updateTimer.time()) {
 | 
				
			||||||
 | 
					
 | 
				
			||||||
      if (!updater.apply(account)) {
 | 
					      redisDelete(account);
 | 
				
			||||||
        return account;
 | 
					 | 
				
			||||||
      }
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
      {
 | 
					 | 
				
			||||||
        // optimistically increment version
 | 
					 | 
				
			||||||
        final int originalVersion = account.getVersion();
 | 
					 | 
				
			||||||
        account.setVersion(originalVersion + 1);
 | 
					 | 
				
			||||||
        redisSet(account);
 | 
					 | 
				
			||||||
        account.setVersion(originalVersion);
 | 
					 | 
				
			||||||
      }
 | 
					 | 
				
			||||||
 | 
					
 | 
				
			||||||
      final UUID uuid = account.getUuid();
 | 
					      final UUID uuid = account.getUuid();
 | 
				
			||||||
 | 
					
 | 
				
			||||||
      updatedAccount = updateWithRetries(account, updater, this::databaseUpdate, () -> databaseGet(uuid).get());
 | 
					      updatedAccount = updateWithRetries(account, updater, this::databaseUpdate, () -> databaseGet(uuid).get());
 | 
				
			||||||
 | 
					
 | 
				
			||||||
      if (dynamoWriteEnabled()) {
 | 
					      if (dynamoWriteEnabled()) {
 | 
				
			||||||
        runSafelyAndRecordMetrics(() -> {
 | 
					        runSafelyAndRecordMetrics(() -> dynamoGet(uuid).map(dynamoAccount ->
 | 
				
			||||||
 | 
					                updateWithRetries(dynamoAccount, updater, this::dynamoUpdate, () -> dynamoGet(uuid).get())),
 | 
				
			||||||
              final Optional<Account> dynamoAccount = dynamoGet(uuid);
 | 
					            Optional.of(uuid),
 | 
				
			||||||
              if (dynamoAccount.isPresent()) {
 | 
					            Optional.of(updatedAccount),
 | 
				
			||||||
 | 
					 | 
				
			||||||
                if (!updater.apply(dynamoAccount.get())) {
 | 
					 | 
				
			||||||
                  return dynamoAccount;
 | 
					 | 
				
			||||||
                }
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
                Account dynamoUpdatedAccount = updateWithRetries(dynamoAccount.get(),
 | 
					 | 
				
			||||||
                    updater,
 | 
					 | 
				
			||||||
                    this::dynamoUpdate,
 | 
					 | 
				
			||||||
                    () -> dynamoGet(uuid).get());
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
                return Optional.of(dynamoUpdatedAccount);
 | 
					 | 
				
			||||||
              }
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
              return Optional.empty();
 | 
					 | 
				
			||||||
            }, Optional.of(uuid), Optional.of(updatedAccount),
 | 
					 | 
				
			||||||
            this::compareAccounts,
 | 
					            this::compareAccounts,
 | 
				
			||||||
            "update");
 | 
					            "update");
 | 
				
			||||||
      }
 | 
					      }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
      // set the cache again, so that all updates are coalesced
 | 
					 | 
				
			||||||
      redisSet(updatedAccount);
 | 
					      redisSet(updatedAccount);
 | 
				
			||||||
    }
 | 
					    }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					@ -349,6 +323,10 @@ public class AccountsManager {
 | 
				
			||||||
  private Account updateWithRetries(Account account, Function<Account, Boolean> updater, Consumer<Account> persister,
 | 
					  private Account updateWithRetries(Account account, Function<Account, Boolean> updater, Consumer<Account> persister,
 | 
				
			||||||
      Supplier<Account> retriever) {
 | 
					      Supplier<Account> retriever) {
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    if (!updater.apply(account)) {
 | 
				
			||||||
 | 
					      return account;
 | 
				
			||||||
 | 
					    }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    final int maxTries = 10;
 | 
					    final int maxTries = 10;
 | 
				
			||||||
    int tries = 0;
 | 
					    int tries = 0;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -338,16 +338,14 @@ class AccountsManagerTest {
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    ArgumentCaptor<String> redisSetArgumentCapture = ArgumentCaptor.forClass(String.class);
 | 
					    ArgumentCaptor<String> redisSetArgumentCapture = ArgumentCaptor.forClass(String.class);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    verify(commands, times(4)).set(anyString(), redisSetArgumentCapture.capture());
 | 
					    verify(commands, times(2)).set(anyString(), redisSetArgumentCapture.capture());
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    Account firstAccountCached = JsonHelpers.fromJson(redisSetArgumentCapture.getAllValues().get(1), Account.class);
 | 
					    Account accountCached = JsonHelpers.fromJson(redisSetArgumentCapture.getAllValues().get(1), Account.class);
 | 
				
			||||||
    Account secondAccountCached = JsonHelpers.fromJson(redisSetArgumentCapture.getAllValues().get(3), Account.class);
 | 
					 | 
				
			||||||
 | 
					
 | 
				
			||||||
    // uuid is @JsonIgnore, so we need to set it for compareAccounts to work
 | 
					    // uuid is @JsonIgnore, so we need to set it for compareAccounts to work
 | 
				
			||||||
    firstAccountCached.setUuid(uuid);
 | 
					    accountCached.setUuid(uuid);
 | 
				
			||||||
    secondAccountCached.setUuid(uuid);
 | 
					 | 
				
			||||||
 | 
					
 | 
				
			||||||
    assertEquals(Optional.empty(), accountsManager.compareAccounts(Optional.of(firstAccountCached), Optional.of(secondAccountCached)));
 | 
					    assertEquals(Optional.empty(), accountsManager.compareAccounts(Optional.of(updatedAccount), Optional.of(accountCached)));
 | 
				
			||||||
  }
 | 
					  }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  @Test
 | 
					  @Test
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
		Reference in New Issue