From feb7cd7bbfb145b770a44a38345d1ac05e60d0db Mon Sep 17 00:00:00 2001 From: Moxie Marlinspike Date: Tue, 6 Nov 2018 05:20:47 -0800 Subject: [PATCH] Remove hystrix from account manager --- pom.xml | 11 -- .../WhisperServerConfiguration.java | 3 - .../textsecuregcm/WhisperServerService.java | 9 -- .../storage/AccountsManager.java | 118 +++++------------- .../tests/storage/AccountsManagerTest.java | 30 ----- 5 files changed, 29 insertions(+), 142 deletions(-) diff --git a/pom.xml b/pom.xml index 89dcf1adb..4e8aedf52 100644 --- a/pom.xml +++ b/pom.xml @@ -111,17 +111,6 @@ 0.5.0 - - com.netflix.hystrix - hystrix-core - ${hystrix.version} - - - com.netflix.hystrix - hystrix-codahale-metrics-publisher - ${hystrix.version} - - com.relayrides pushy diff --git a/src/main/java/org/whispersystems/textsecuregcm/WhisperServerConfiguration.java b/src/main/java/org/whispersystems/textsecuregcm/WhisperServerConfiguration.java index 95219af28..0bcff36df 100644 --- a/src/main/java/org/whispersystems/textsecuregcm/WhisperServerConfiguration.java +++ b/src/main/java/org/whispersystems/textsecuregcm/WhisperServerConfiguration.java @@ -229,7 +229,4 @@ public class WhisperServerConfiguration extends Configuration { return results; } - public Map getHystrixConfiguration() { - return hystrix; - } } diff --git a/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index 66b652db8..8d138d821 100644 --- a/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -20,10 +20,6 @@ import com.codahale.metrics.SharedMetricRegistries; import com.fasterxml.jackson.annotation.JsonAutoDetect; import com.fasterxml.jackson.annotation.PropertyAccessor; import com.fasterxml.jackson.databind.DeserializationFeature; -import com.netflix.config.ConfigurationManager; -import com.netflix.hystrix.contrib.codahalemetricspublisher.HystrixCodaHaleMetricsPublisher; -import com.netflix.hystrix.strategy.HystrixPlugins; -import org.apache.commons.configuration.MapConfiguration; import org.bouncycastle.jce.provider.BouncyCastleProvider; import org.eclipse.jetty.servlets.CrossOriginFilter; import org.skife.jdbi.v2.DBI; @@ -106,9 +102,6 @@ public class WhisperServerService extends Application bootstrap) { - HystrixCodaHaleMetricsPublisher hystrixMetricsPublisher = new HystrixCodaHaleMetricsPublisher(bootstrap.getMetricRegistry()); - HystrixPlugins.getInstance().registerMetricsPublisher(hystrixMetricsPublisher); - bootstrap.addCommand(new DirectoryCommand()); bootstrap.addCommand(new VacuumCommand()); bootstrap.addCommand(new TrimMessagesCommand()); @@ -144,8 +137,6 @@ public class WhisperServerService extends Application(Setter.withGroupKey(HystrixCommandGroupKey.Factory.asKey(GroupKeys.DIRECTORY_SERVICE)) - .andCommandKey(HystrixCommandKey.Factory.asKey(AccountsManager.class.getSimpleName() + ".updateDirectory"))) - { - @Override - protected Void run() { - if (account.isActive()) { - byte[] token = Util.getContactToken(account.getNumber()); - ClientContact clientContact = new ClientContact(token, null, true, true); - directory.add(clientContact); - } else { - directory.remove(account.getNumber()); - } - - return null; - } - }.execute(); + if (account.isActive()) { + byte[] token = Util.getContactToken(account.getNumber()); + ClientContact clientContact = new ClientContact(token, null, true, true); + directory.add(clientContact); + } else { + directory.remove(account.getNumber()); + } } private String getKey(String number) { @@ -135,87 +121,41 @@ public class AccountsManager { } private void redisSet(String number, Account account, boolean optional) { - new HystrixCommand(Setter.withGroupKey(HystrixCommandGroupKey.Factory.asKey(GroupKeys.REDIS_CACHE)) - .andCommandKey(HystrixCommandKey.Factory.asKey("accountsRedisSet"))) + try (Jedis jedis = cacheClient.getWriteResource(); + Timer.Context timer = redisSetTimer.time()) { - @Override - protected Boolean run() { - try (Jedis jedis = cacheClient.getWriteResource(); - Timer.Context timer = redisSetTimer.time()) - { - jedis.set(getKey(number), mapper.writeValueAsString(account)); - } catch (JsonProcessingException e) { - throw new HystrixBadRequestException("Json processing error", e); - } - - return true; - } - - @Override - protected Boolean getFallback() { - if (optional) return true; - else return super.getFallback(); - } - }.execute(); + jedis.set(getKey(number), mapper.writeValueAsString(account)); + } catch (JsonProcessingException e) { + throw new IllegalStateException(e); + } } private Optional redisGet(String number) { - return new HystrixCommand>(Setter.withGroupKey(HystrixCommandGroupKey.Factory.asKey(GroupKeys.REDIS_CACHE)) - .andCommandKey(HystrixCommandKey.Factory.asKey("accountsRedisGet"))) + try (Jedis jedis = cacheClient.getReadResource(); + Timer.Context timer = redisGetTimer.time()) { - @Override - protected Optional run() { - try (Jedis jedis = cacheClient.getReadResource(); - Timer.Context timer = redisGetTimer.time()) - { - String json = jedis.get(getKey(number)); + String json = jedis.get(getKey(number)); - if (json != null) return Optional.of(mapper.readValue(json, Account.class)); - else return Optional.empty(); - } catch (IOException e) { - logger.warn("AccountsManager", "Deserialization error", e); - return Optional.empty(); - } - } - - @Override - protected Optional getFallback() { - return Optional.empty(); - } - }.execute(); + if (json != null) return Optional.of(mapper.readValue(json, Account.class)); + else return Optional.empty(); + } catch (IOException e) { + logger.warn("AccountsManager", "Deserialization error", e); + return Optional.empty(); + } catch (JedisException e) { + logger.warn("Redis failure", e); + return Optional.empty(); + } } private Optional databaseGet(String number) { - return new HystrixCommand>(Setter.withGroupKey(HystrixCommandGroupKey.Factory.asKey(GroupKeys.DATABASE_ACCOUNTS)) - .andCommandKey(HystrixCommandKey.Factory.asKey("accountsDatabaseGet"))) - { - @Override - protected Optional run() { - return Optional.ofNullable(accounts.get(number)); - } - }.execute(); + return Optional.ofNullable(accounts.get(number)); } private boolean databaseCreate(Account account) { - return new HystrixCommand(Setter.withGroupKey(HystrixCommandGroupKey.Factory.asKey(GroupKeys.DATABASE_ACCOUNTS)) - .andCommandKey(HystrixCommandKey.Factory.asKey("accountsDatabaseCreate"))) - { - @Override - protected Boolean run() { - return accounts.create(account); - } - }.execute(); + return accounts.create(account); } private void databaseUpdate(Account account) { - new HystrixCommand(Setter.withGroupKey(HystrixCommandGroupKey.Factory.asKey(GroupKeys.DATABASE_ACCOUNTS)) - .andCommandKey(HystrixCommandKey.Factory.asKey("accountsDatabaseUpdate"))) - { - @Override - protected Void run() { - accounts.update(account); - return null; - } - }.execute(); + accounts.update(account); } } diff --git a/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AccountsManagerTest.java b/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AccountsManagerTest.java index 719eb6d45..49942bbf0 100644 --- a/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AccountsManagerTest.java +++ b/src/test/java/org/whispersystems/textsecuregcm/tests/storage/AccountsManagerTest.java @@ -1,8 +1,6 @@ package org.whispersystems.textsecuregcm.tests.storage; -import com.netflix.hystrix.exception.HystrixRuntimeException; import org.junit.Test; -import org.skife.jdbi.v2.exceptions.UnableToObtainConnectionException; import org.whispersystems.textsecuregcm.redis.ReplicatedJedisPool; import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.Accounts; @@ -101,34 +99,6 @@ public class AccountsManagerTest { verifyNoMoreInteractions(accounts); } - @Test - public void testGetAccountEmptyCacheBrokenDatabase() { - ReplicatedJedisPool cacheClient = mock(ReplicatedJedisPool.class); - Jedis jedis = mock(Jedis.class ); - Accounts accounts = mock(Accounts.class ); - DirectoryManager directoryManager = mock(DirectoryManager.class ); - Account account = new Account("+14152222222", new HashSet<>(), new byte[16]); - - when(cacheClient.getReadResource()).thenReturn(jedis); - when(cacheClient.getWriteResource()).thenReturn(jedis); - when(jedis.get(eq("Account5+14152222222"))).thenReturn(null); - when(accounts.get(eq("+14152222222"))).thenThrow(new UnableToObtainConnectionException(new Exception())); - - AccountsManager accountsManager = new AccountsManager(accounts, directoryManager, cacheClient); - - try { - Optional retrieved = accountsManager.get("+14152222222"); - throw new AssertionError("Should not have succeeded!"); - } catch (HystrixRuntimeException e) { - // good - verify(jedis, times(1)).get(eq("Account5+14152222222")); - verify(jedis, times(1)).close(); - verifyNoMoreInteractions(jedis); - - verify(accounts, times(1)).get(eq("+14152222222")); - verifyNoMoreInteractions(accounts); - } - } }