Remove hystrix from account manager

This commit is contained in:
Moxie Marlinspike 2018-11-06 05:20:47 -08:00
parent 79c05c37dd
commit feb7cd7bbf
5 changed files with 29 additions and 142 deletions

11
pom.xml
View File

@ -111,17 +111,6 @@
<version>0.5.0</version> <version>0.5.0</version>
</dependency> </dependency>
<dependency>
<groupId>com.netflix.hystrix</groupId>
<artifactId>hystrix-core</artifactId>
<version>${hystrix.version}</version>
</dependency>
<dependency>
<groupId>com.netflix.hystrix</groupId>
<artifactId>hystrix-codahale-metrics-publisher</artifactId>
<version>${hystrix.version}</version>
</dependency>
<dependency> <dependency>
<groupId>com.relayrides</groupId> <groupId>com.relayrides</groupId>
<artifactId>pushy</artifactId> <artifactId>pushy</artifactId>

View File

@ -229,7 +229,4 @@ public class WhisperServerConfiguration extends Configuration {
return results; return results;
} }
public Map<String, Object> getHystrixConfiguration() {
return hystrix;
}
} }

View File

@ -20,10 +20,6 @@ import com.codahale.metrics.SharedMetricRegistries;
import com.fasterxml.jackson.annotation.JsonAutoDetect; import com.fasterxml.jackson.annotation.JsonAutoDetect;
import com.fasterxml.jackson.annotation.PropertyAccessor; import com.fasterxml.jackson.annotation.PropertyAccessor;
import com.fasterxml.jackson.databind.DeserializationFeature; 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.bouncycastle.jce.provider.BouncyCastleProvider;
import org.eclipse.jetty.servlets.CrossOriginFilter; import org.eclipse.jetty.servlets.CrossOriginFilter;
import org.skife.jdbi.v2.DBI; import org.skife.jdbi.v2.DBI;
@ -106,9 +102,6 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
@Override @Override
public void initialize(Bootstrap<WhisperServerConfiguration> bootstrap) { public void initialize(Bootstrap<WhisperServerConfiguration> bootstrap) {
HystrixCodaHaleMetricsPublisher hystrixMetricsPublisher = new HystrixCodaHaleMetricsPublisher(bootstrap.getMetricRegistry());
HystrixPlugins.getInstance().registerMetricsPublisher(hystrixMetricsPublisher);
bootstrap.addCommand(new DirectoryCommand()); bootstrap.addCommand(new DirectoryCommand());
bootstrap.addCommand(new VacuumCommand()); bootstrap.addCommand(new VacuumCommand());
bootstrap.addCommand(new TrimMessagesCommand()); bootstrap.addCommand(new TrimMessagesCommand());
@ -144,8 +137,6 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
environment.getObjectMapper().setVisibility(PropertyAccessor.ALL, JsonAutoDetect.Visibility.NONE); environment.getObjectMapper().setVisibility(PropertyAccessor.ALL, JsonAutoDetect.Visibility.NONE);
environment.getObjectMapper().setVisibility(PropertyAccessor.FIELD, JsonAutoDetect.Visibility.ANY); environment.getObjectMapper().setVisibility(PropertyAccessor.FIELD, JsonAutoDetect.Visibility.ANY);
ConfigurationManager.install(new MapConfiguration(config.getHystrixConfiguration()));
DBIFactory dbiFactory = new DBIFactory(); DBIFactory dbiFactory = new DBIFactory();
DBI database = dbiFactory.build(environment, config.getDataSourceFactory(), "accountdb"); DBI database = dbiFactory.build(environment, config.getDataSourceFactory(), "accountdb");
DBI messagedb = dbiFactory.build(environment, config.getMessageStoreConfiguration(), "messagedb"); DBI messagedb = dbiFactory.build(environment, config.getMessageStoreConfiguration(), "messagedb");

View File

@ -22,15 +22,9 @@ import com.codahale.metrics.SharedMetricRegistries;
import com.codahale.metrics.Timer; import com.codahale.metrics.Timer;
import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectMapper;
import com.netflix.hystrix.HystrixCommand;
import com.netflix.hystrix.HystrixCommand.Setter;
import com.netflix.hystrix.HystrixCommandGroupKey;
import com.netflix.hystrix.HystrixCommandKey;
import com.netflix.hystrix.exception.HystrixBadRequestException;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import org.whispersystems.textsecuregcm.entities.ClientContact; import org.whispersystems.textsecuregcm.entities.ClientContact;
import org.whispersystems.textsecuregcm.hystrix.GroupKeys;
import org.whispersystems.textsecuregcm.redis.ReplicatedJedisPool; import org.whispersystems.textsecuregcm.redis.ReplicatedJedisPool;
import org.whispersystems.textsecuregcm.util.Constants; import org.whispersystems.textsecuregcm.util.Constants;
import org.whispersystems.textsecuregcm.util.SystemMapper; import org.whispersystems.textsecuregcm.util.SystemMapper;
@ -43,6 +37,7 @@ import java.util.Optional;
import static com.codahale.metrics.MetricRegistry.name; import static com.codahale.metrics.MetricRegistry.name;
import redis.clients.jedis.Jedis; import redis.clients.jedis.Jedis;
import redis.clients.jedis.exceptions.JedisException;
public class AccountsManager { public class AccountsManager {
@ -112,22 +107,13 @@ public class AccountsManager {
} }
private void updateDirectory(Account account) { private void updateDirectory(Account account) {
new HystrixCommand<Void>(Setter.withGroupKey(HystrixCommandGroupKey.Factory.asKey(GroupKeys.DIRECTORY_SERVICE)) if (account.isActive()) {
.andCommandKey(HystrixCommandKey.Factory.asKey(AccountsManager.class.getSimpleName() + ".updateDirectory"))) byte[] token = Util.getContactToken(account.getNumber());
{ ClientContact clientContact = new ClientContact(token, null, true, true);
@Override directory.add(clientContact);
protected Void run() { } else {
if (account.isActive()) { directory.remove(account.getNumber());
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();
} }
private String getKey(String number) { private String getKey(String number) {
@ -135,87 +121,41 @@ public class AccountsManager {
} }
private void redisSet(String number, Account account, boolean optional) { private void redisSet(String number, Account account, boolean optional) {
new HystrixCommand<Boolean>(Setter.withGroupKey(HystrixCommandGroupKey.Factory.asKey(GroupKeys.REDIS_CACHE)) try (Jedis jedis = cacheClient.getWriteResource();
.andCommandKey(HystrixCommandKey.Factory.asKey("accountsRedisSet"))) Timer.Context timer = redisSetTimer.time())
{ {
@Override jedis.set(getKey(number), mapper.writeValueAsString(account));
protected Boolean run() { } catch (JsonProcessingException e) {
try (Jedis jedis = cacheClient.getWriteResource(); throw new IllegalStateException(e);
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();
} }
private Optional<Account> redisGet(String number) { private Optional<Account> redisGet(String number) {
return new HystrixCommand<Optional<Account>>(Setter.withGroupKey(HystrixCommandGroupKey.Factory.asKey(GroupKeys.REDIS_CACHE)) try (Jedis jedis = cacheClient.getReadResource();
.andCommandKey(HystrixCommandKey.Factory.asKey("accountsRedisGet"))) Timer.Context timer = redisGetTimer.time())
{ {
@Override String json = jedis.get(getKey(number));
protected Optional<Account> run() {
try (Jedis jedis = cacheClient.getReadResource();
Timer.Context timer = redisGetTimer.time())
{
String json = jedis.get(getKey(number));
if (json != null) return Optional.of(mapper.readValue(json, Account.class)); if (json != null) return Optional.of(mapper.readValue(json, Account.class));
else return Optional.empty(); else return Optional.empty();
} catch (IOException e) { } catch (IOException e) {
logger.warn("AccountsManager", "Deserialization error", e); logger.warn("AccountsManager", "Deserialization error", e);
return Optional.empty(); return Optional.empty();
} } catch (JedisException e) {
} logger.warn("Redis failure", e);
return Optional.empty();
@Override }
protected Optional<Account> getFallback() {
return Optional.empty();
}
}.execute();
} }
private Optional<Account> databaseGet(String number) { private Optional<Account> databaseGet(String number) {
return new HystrixCommand<Optional<Account>>(Setter.withGroupKey(HystrixCommandGroupKey.Factory.asKey(GroupKeys.DATABASE_ACCOUNTS)) return Optional.ofNullable(accounts.get(number));
.andCommandKey(HystrixCommandKey.Factory.asKey("accountsDatabaseGet")))
{
@Override
protected Optional<Account> run() {
return Optional.ofNullable(accounts.get(number));
}
}.execute();
} }
private boolean databaseCreate(Account account) { private boolean databaseCreate(Account account) {
return new HystrixCommand<Boolean>(Setter.withGroupKey(HystrixCommandGroupKey.Factory.asKey(GroupKeys.DATABASE_ACCOUNTS)) return accounts.create(account);
.andCommandKey(HystrixCommandKey.Factory.asKey("accountsDatabaseCreate")))
{
@Override
protected Boolean run() {
return accounts.create(account);
}
}.execute();
} }
private void databaseUpdate(Account account) { private void databaseUpdate(Account account) {
new HystrixCommand<Void>(Setter.withGroupKey(HystrixCommandGroupKey.Factory.asKey(GroupKeys.DATABASE_ACCOUNTS)) accounts.update(account);
.andCommandKey(HystrixCommandKey.Factory.asKey("accountsDatabaseUpdate")))
{
@Override
protected Void run() {
accounts.update(account);
return null;
}
}.execute();
} }
} }

View File

@ -1,8 +1,6 @@
package org.whispersystems.textsecuregcm.tests.storage; package org.whispersystems.textsecuregcm.tests.storage;
import com.netflix.hystrix.exception.HystrixRuntimeException;
import org.junit.Test; import org.junit.Test;
import org.skife.jdbi.v2.exceptions.UnableToObtainConnectionException;
import org.whispersystems.textsecuregcm.redis.ReplicatedJedisPool; import org.whispersystems.textsecuregcm.redis.ReplicatedJedisPool;
import org.whispersystems.textsecuregcm.storage.Account; import org.whispersystems.textsecuregcm.storage.Account;
import org.whispersystems.textsecuregcm.storage.Accounts; import org.whispersystems.textsecuregcm.storage.Accounts;
@ -101,34 +99,6 @@ public class AccountsManagerTest {
verifyNoMoreInteractions(accounts); 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<Account> 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);
}
}
} }