diff --git a/atlasdb-config/src/main/java/com/palantir/atlasdb/config/AtlasDbRuntimeConfig.java b/atlasdb-config/src/main/java/com/palantir/atlasdb/config/AtlasDbRuntimeConfig.java index bcbcf0ed362..5fff51f0203 100644 --- a/atlasdb-config/src/main/java/com/palantir/atlasdb/config/AtlasDbRuntimeConfig.java +++ b/atlasdb-config/src/main/java/com/palantir/atlasdb/config/AtlasDbRuntimeConfig.java @@ -103,6 +103,11 @@ public StreamStorePersistenceConfiguration streamStorePersistence() { return StreamStorePersistenceConfiguration.DEFAULT_CONFIG; } + @Value.Default + public RemotingClientConfig remotingClient() { + return ImmutableRemotingClientConfig.builder().build(); + } + public static ImmutableAtlasDbRuntimeConfig defaultRuntimeConfig() { return ImmutableAtlasDbRuntimeConfig.builder().build(); } diff --git a/atlasdb-config/src/main/java/com/palantir/atlasdb/factory/Leaders.java b/atlasdb-config/src/main/java/com/palantir/atlasdb/factory/Leaders.java index ee849cec58d..4b6cf6a09bd 100644 --- a/atlasdb-config/src/main/java/com/palantir/atlasdb/factory/Leaders.java +++ b/atlasdb-config/src/main/java/com/palantir/atlasdb/factory/Leaders.java @@ -40,14 +40,16 @@ import com.google.common.collect.Sets; import com.google.common.net.HostAndPort; import com.google.common.util.concurrent.ThreadFactoryBuilder; +import com.palantir.atlasdb.config.AuxiliaryRemotingParameters; import com.palantir.atlasdb.config.LeaderConfig; import com.palantir.atlasdb.config.LeaderRuntimeConfig; import com.palantir.atlasdb.http.AtlasDbHttpClients; +import com.palantir.atlasdb.http.AtlasDbRemotingConstants; import com.palantir.atlasdb.http.NotCurrentLeaderExceptionMapper; -import com.palantir.atlasdb.http.UserAgents; import com.palantir.atlasdb.util.AtlasDbMetrics; import com.palantir.atlasdb.util.MetricsManager; import com.palantir.common.concurrent.PTExecutors; +import com.palantir.conjure.java.api.config.service.UserAgent; import com.palantir.conjure.java.config.ssl.TrustContext; import com.palantir.leader.AsyncLeadershipObserver; import com.palantir.leader.BatchingLeaderElectionService; @@ -75,17 +77,20 @@ private Leaders() { */ public static LeaderElectionService create(MetricsManager metricsManager, Consumer env, LeaderConfig config, Supplier runtime) { - return create(metricsManager, env, config, runtime, UserAgents.DEFAULT_USER_AGENT); + return create(metricsManager, env, config, runtime, AtlasDbRemotingConstants.DEFAULT_USER_AGENT); } public static LeaderElectionService create(MetricsManager metricsManager, - Consumer env, LeaderConfig config, Supplier runtime, String userAgent) { + Consumer env, LeaderConfig config, Supplier runtime, UserAgent userAgent) { return createAndRegisterLocalServices(metricsManager, env, config, runtime, userAgent).leaderElectionService(); } public static LocalPaxosServices createAndRegisterLocalServices( - MetricsManager metricsManager, Consumer env, LeaderConfig config, - Supplier runtime, String userAgent) { + MetricsManager metricsManager, + Consumer env, + LeaderConfig config, + Supplier runtime, + UserAgent userAgent) { LocalPaxosServices localPaxosServices = createInstrumentedLocalServices( metricsManager, config, runtime, userAgent); @@ -100,7 +105,7 @@ public static LocalPaxosServices createInstrumentedLocalServices( MetricsManager metricsManager, LeaderConfig config, Supplier runtime, - String userAgent) { + UserAgent userAgent) { Set remoteLeaderUris = Sets.newHashSet(config.leaders()); remoteLeaderUris.remove(config.localServer()); @@ -117,7 +122,7 @@ public static LocalPaxosServices createInstrumentedLocalServices( LeaderConfig config, Supplier runtime, RemotePaxosServerSpec remotePaxosServerSpec, - String userAgent) { + UserAgent userAgent) { UUID leaderUuid = UUID.randomUUID(); AsyncLeadershipObserver leadershipObserver = AsyncLeadershipObserver.create(); @@ -215,10 +220,20 @@ public static List createProxyAndLocalList( Set remoteUris, Optional trustContext, Class clazz, - String userAgent) { + UserAgent userAgent) { + // TODO (jkong): Enable runtime config for leader election services. List remotes = remoteUris.stream() - .map(uri -> AtlasDbHttpClients.createProxy(metrics, trustContext, uri, clazz, userAgent, false)) + .map(uri -> AtlasDbHttpClients.createProxy( + metrics, + trustContext, + uri, + clazz, + AuxiliaryRemotingParameters.builder() + .userAgent(userAgent) + .shouldLimitPayload(false) + .shouldRetry(true) + .build())) .collect(Collectors.toList()); return ImmutableList.copyOf(Iterables.concat( @@ -230,14 +245,23 @@ public static Map generatePingables( MetricsManager metricsManager, Collection remoteEndpoints, Optional trustContext, - String userAgent) { + UserAgent userAgent) { /* The interface used as a key here may be a proxy, which may have strange .equals() behavior. * This is circumvented by using an IdentityHashMap which will just use native == for equality. */ Map pingables = new IdentityHashMap<>(); for (String endpoint : remoteEndpoints) { - PingableLeader remoteInterface = AtlasDbHttpClients.createProxyWithoutRetrying(metricsManager.getRegistry(), - trustContext, endpoint, PingableLeader.class, userAgent, false); + PingableLeader remoteInterface = AtlasDbHttpClients.createProxy( + metricsManager.getRegistry(), + trustContext, + endpoint, + PingableLeader.class, + AuxiliaryRemotingParameters.builder() // TODO (jkong): Configurable remoting client config. + .shouldLimitPayload(false) + .userAgent(userAgent) + .shouldRetry(false) + .shouldLimitPayload(true) + .build()); HostAndPort hostAndPort = HostAndPort.fromString(endpoint); pingables.put(remoteInterface, hostAndPort); } diff --git a/atlasdb-config/src/main/java/com/palantir/atlasdb/factory/ServiceCreator.java b/atlasdb-config/src/main/java/com/palantir/atlasdb/factory/ServiceCreator.java index e59cca4d9e2..afb06f821a9 100644 --- a/atlasdb-config/src/main/java/com/palantir/atlasdb/factory/ServiceCreator.java +++ b/atlasdb-config/src/main/java/com/palantir/atlasdb/factory/ServiceCreator.java @@ -15,50 +15,44 @@ */ package com.palantir.atlasdb.factory; -import java.io.IOException; -import java.net.InetSocketAddress; -import java.net.Proxy; -import java.net.ProxySelector; -import java.net.SocketAddress; -import java.net.URI; -import java.util.List; import java.util.Optional; -import java.util.function.Function; import java.util.function.Supplier; import com.codahale.metrics.MetricRegistry; -import com.google.common.collect.ImmutableList; -import com.google.common.net.HostAndPort; +import com.palantir.atlasdb.config.AuxiliaryRemotingParameters; +import com.palantir.atlasdb.config.RemotingClientConfig; import com.palantir.atlasdb.config.ServerListConfig; import com.palantir.atlasdb.http.AtlasDbHttpClients; import com.palantir.atlasdb.util.AtlasDbMetrics; import com.palantir.atlasdb.util.MetricsManager; -import com.palantir.conjure.java.api.config.service.ProxyConfiguration; +import com.palantir.conjure.java.api.config.service.UserAgent; import com.palantir.conjure.java.api.config.ssl.SslConfiguration; import com.palantir.conjure.java.config.ssl.SslSocketFactories; import com.palantir.conjure.java.config.ssl.TrustContext; -import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; public final class ServiceCreator { private final MetricsManager metricsManager; - private final String userAgent; private final Supplier servers; - private final boolean limitPayload; + private final AuxiliaryRemotingParameters parameters; - private ServiceCreator(MetricsManager metricsManager, String userAgent, Supplier servers, - boolean limitPayload) { + private ServiceCreator(MetricsManager metricsManager, + Supplier servers, + AuxiliaryRemotingParameters parameters) { this.metricsManager = metricsManager; - this.userAgent = userAgent; this.servers = servers; - this.limitPayload = limitPayload; + this.parameters = parameters; } /** * Creates clients without client-side restrictions on payload size. */ - public static ServiceCreator noPayloadLimiter(MetricsManager metrics, String agent, - Supplier serverList) { - return new ServiceCreator(metrics, agent, serverList, false); + public static ServiceCreator noPayloadLimiter( + MetricsManager metrics, + Supplier serverList, + UserAgent userAgent, + Supplier remotingClientConfigSupplier) { + return new ServiceCreator( + metrics, serverList, toAuxiliaryRemotingParameters(userAgent, remotingClientConfigSupplier, false)); } /** @@ -66,20 +60,17 @@ public static ServiceCreator noPayloadLimiter(MetricsManager metrics, String age * {@link com.palantir.atlasdb.http.AtlasDbInterceptors#MAX_PAYLOAD_SIZE} bytes. This ServiceCreator should be used * for clients to servers that impose payload limits. */ - public static ServiceCreator withPayloadLimiter(MetricsManager metrics, String agent, - Supplier serverList) { - return new ServiceCreator(metrics, agent, serverList, true); + public static ServiceCreator withPayloadLimiter( + MetricsManager metrics, + Supplier serverList, + UserAgent userAgent, + Supplier remotingClientConfigSupplier) { + return new ServiceCreator( + metrics, serverList, toAuxiliaryRemotingParameters(userAgent, remotingClientConfigSupplier, true)); } public T createService(Class serviceClass) { - return create( - metricsManager, - servers, - SslSocketFactories::createTrustContext, - ServiceCreator::createProxySelector, - serviceClass, - userAgent, - limitPayload); + return create(metricsManager, servers, serviceClass, parameters); } /** @@ -93,19 +84,13 @@ public static Optional createTrustContext(Optional T create( MetricsManager metricsManager, Supplier serverListConfigSupplier, - Function trustContextCreator, - Function proxySelectorCreator, Class type, - String userAgent, - boolean limitPayload) { + AuxiliaryRemotingParameters parameters) { return AtlasDbHttpClients.createLiveReloadingProxyWithFailover( metricsManager.getTaggedRegistry(), serverListConfigSupplier, - trustContextCreator, - proxySelectorCreator, type, - userAgent, - limitPayload); + parameters); } public static T createInstrumentedService(MetricRegistry metricRegistry, T service, Class serviceClass) { @@ -116,36 +101,15 @@ public static T createInstrumentedService(MetricRegistry metricRegistry, T s MetricRegistry.name(serviceClass)); } - /** - * The code below is copied from http-remoting and should be removed when we switch the clients to use remoting. - */ - public static ProxySelector createProxySelector(ProxyConfiguration proxyConfig) { - switch (proxyConfig.type()) { - case DIRECT: - return fixedProxySelectorFor(Proxy.NO_PROXY); - case HTTP: - HostAndPort hostAndPort = HostAndPort.fromString(proxyConfig.hostAndPort() - .orElseThrow(() -> new SafeIllegalArgumentException( - "Expected to find proxy hostAndPort configuration for HTTP proxy"))); - InetSocketAddress addr = new InetSocketAddress(hostAndPort.getHost(), hostAndPort.getPort()); - return fixedProxySelectorFor(new Proxy(Proxy.Type.HTTP, addr)); - default: - // fall through - } - - throw new IllegalStateException("Failed to create ProxySelector for proxy configuration: " + proxyConfig); - } - - private static ProxySelector fixedProxySelectorFor(Proxy proxy) { - return new ProxySelector() { - @Override - public List select(URI uri) { - return ImmutableList.of(proxy); - } - - @Override - public void connectFailed(URI uri, SocketAddress sa, IOException ioe) {} - }; - + private static AuxiliaryRemotingParameters toAuxiliaryRemotingParameters( + UserAgent userAgent, + Supplier remotingClientConfigSupplier, + boolean shouldLimitPayload) { + return AuxiliaryRemotingParameters.builder() + .remotingClientConfig(remotingClientConfigSupplier) + .userAgent(userAgent) + .shouldLimitPayload(shouldLimitPayload) + .shouldRetry(true) + .build(); } } diff --git a/atlasdb-config/src/main/java/com/palantir/atlasdb/factory/TransactionManagers.java b/atlasdb-config/src/main/java/com/palantir/atlasdb/factory/TransactionManagers.java index ae3ff475c29..b758e3999dc 100644 --- a/atlasdb-config/src/main/java/com/palantir/atlasdb/factory/TransactionManagers.java +++ b/atlasdb-config/src/main/java/com/palantir/atlasdb/factory/TransactionManagers.java @@ -62,6 +62,7 @@ import com.palantir.atlasdb.config.ImmutableTimeLockClientConfig; import com.palantir.atlasdb.config.LeaderConfig; import com.palantir.atlasdb.config.LeaderRuntimeConfig; +import com.palantir.atlasdb.config.RemotingClientConfig; import com.palantir.atlasdb.config.ServerListConfig; import com.palantir.atlasdb.config.ServerListConfigs; import com.palantir.atlasdb.config.ShouldRunBackgroundSweepSupplier; @@ -74,7 +75,7 @@ import com.palantir.atlasdb.factory.timelock.TimestampCorroboratingTimelockService; import com.palantir.atlasdb.factory.timestamp.FreshTimestampSupplierAdapter; import com.palantir.atlasdb.http.AtlasDbFeignTargetFactory; -import com.palantir.atlasdb.http.UserAgents; +import com.palantir.atlasdb.http.AtlasDbRemotingConstants; import com.palantir.atlasdb.internalschema.InternalSchemaMetadata; import com.palantir.atlasdb.internalschema.TransactionSchemaInstaller; import com.palantir.atlasdb.internalschema.TransactionSchemaManager; @@ -134,6 +135,8 @@ import com.palantir.atlasdb.util.MetricsManagers; import com.palantir.common.annotation.Output; import com.palantir.common.time.Clock; +import com.palantir.conjure.java.api.config.service.UserAgent; +import com.palantir.conjure.java.api.config.service.UserAgents; import com.palantir.leader.LeaderElectionService; import com.palantir.leader.PingableLeader; import com.palantir.leader.proxy.AwaitingLeadershipProxy; @@ -208,8 +211,17 @@ boolean lockImmutableTsOnReadOnlyTransactions() { return false; } + /** + * @deprecated Please specify a {@link #structuredUserAgent()} instead. + */ + @Deprecated abstract String userAgent(); + @Value.Default + UserAgent structuredUserAgent() { + return UserAgents.tryParse(userAgent()); + } + abstract MetricRegistry globalMetricsRegistry(); abstract TaggedMetricRegistry globalTaggedMetricRegistry(); @@ -258,10 +270,11 @@ public static TransactionManager createInMemory(Set schemas) { AtlasDbConfig config = ImmutableAtlasDbConfig.builder().keyValueService(new InMemoryAtlasDbConfig()).build(); return builder() .config(config) - .userAgent(UserAgents.DEFAULT_USER_AGENT) + .userAgent(AtlasDbRemotingConstants.DEFAULT_USER_AGENT.toString()) .globalMetricsRegistry(new MetricRegistry()) .globalTaggedMetricRegistry(DefaultTaggedMetricRegistry.getDefault()) .addAllSchemas(schemas) + .structuredUserAgent(AtlasDbRemotingConstants.DEFAULT_USER_AGENT) .build() .serializable(); } @@ -321,7 +334,7 @@ private TransactionManager serializableInternal(@Output List clos () -> LockServiceImpl.create(lockServerOptions()), managedTimestampSupplier, atlasFactory.getTimestampStoreInvalidator(), - userAgent()); + structuredUserAgent()); adapter.setTimestampService(lockAndTimestampServices.timestamp()); KvsProfilingLogger.setSlowLogThresholdMillis(config().getKvsSlowLogThresholdMillis()); @@ -765,7 +778,7 @@ public static LockAndTimestampServices createLockAndTimestampServicesForCli( lock, time, invalidator, - userAgent); + UserAgents.tryParse(userAgent)); TimeLockClient timeLockClient = TimeLockClient.withSynchronousUnlocker(lockAndTimestampServices.timelock()); return ImmutableLockAndTimestampServices.builder() .from(lockAndTimestampServices) @@ -784,7 +797,7 @@ static LockAndTimestampServices createLockAndTimestampServices( Supplier lock, Supplier time, TimestampStoreInvalidator invalidator, - String userAgent) { + UserAgent userAgent) { LockAndTimestampServices lockAndTimestampServices = createRawInstrumentedServices( metricsManager, config, @@ -852,13 +865,13 @@ private static LockAndTimestampServices createRawInstrumentedServices( Supplier lock, Supplier time, TimestampStoreInvalidator invalidator, - String userAgent) { + UserAgent userAgent) { AtlasDbRuntimeConfig initialRuntimeConfig = runtimeConfigSupplier.get(); assertNoSpuriousTimeLockBlockInRuntimeConfig(config, initialRuntimeConfig); if (config.leader().isPresent()) { return createRawLeaderServices(metricsManager, config.leader().get(), env, lock, time, userAgent); } else if (config.timestamp().isPresent() && config.lock().isPresent()) { - return createRawRemoteServices(metricsManager, config, userAgent); + return createRawRemoteServices(metricsManager, config, runtimeConfigSupplier, userAgent); } else if (isUsingTimeLock(config, initialRuntimeConfig)) { return createRawServicesFromTimeLock(metricsManager, config, runtimeConfigSupplier, invalidator, userAgent); } else { @@ -888,14 +901,19 @@ private static LockAndTimestampServices createRawServicesFromTimeLock( AtlasDbConfig config, Supplier runtimeConfigSupplier, TimestampStoreInvalidator invalidator, - String userAgent) { + UserAgent userAgent) { Supplier serverListConfigSupplier = getServerListConfigSupplierForTimeLock(config, runtimeConfigSupplier); String timelockNamespace = OptionalResolver.resolve( config.timelock().flatMap(TimeLockClientConfig::client), config.namespace()); LockAndTimestampServices lockAndTimestampServices = - getLockAndTimestampServices(metricsManager, serverListConfigSupplier, userAgent, timelockNamespace); + getLockAndTimestampServices( + metricsManager, + serverListConfigSupplier, + () -> runtimeConfigSupplier.get().remotingClient(), + userAgent, + timelockNamespace); TimeLockMigrator migrator = TimeLockMigrator.create( lockAndTimestampServices.timestampManagement(), @@ -922,9 +940,11 @@ private static Supplier getServerListConfigSupplierForTimeLock private static LockAndTimestampServices getLockAndTimestampServices( MetricsManager metricsManager, Supplier timelockServerListConfig, - String userAgent, + Supplier remotingConfigSupplier, + UserAgent userAgent, String timelockNamespace) { - ServiceCreator creator = ServiceCreator.withPayloadLimiter(metricsManager, userAgent, timelockServerListConfig); + ServiceCreator creator = ServiceCreator.withPayloadLimiter( + metricsManager, timelockServerListConfig, userAgent, remotingConfigSupplier); LockService lockService = RemoteLockServiceAdapter.create( creator.createService(LockRpcClient.class), timelockNamespace); @@ -951,7 +971,7 @@ private static LockAndTimestampServices createRawLeaderServices( Consumer env, Supplier lock, Supplier time, - String userAgent) { + UserAgent userAgent) { // Create local services, that may or may not end up being registered in an Consumer. LeaderRuntimeConfig defaultRuntime = ImmutableLeaderRuntimeConfig.builder().build(); LocalPaxosServices localPaxosServices = Leaders.createAndRegisterLocalServices( @@ -987,7 +1007,8 @@ private static LockAndTimestampServices createRawLeaderServices( .servers(leaderConfig.leaders()) .sslConfiguration(leaderConfig.sslConfiguration()) .build(); - ServiceCreator creator = ServiceCreator.noPayloadLimiter(metricsManager, userAgent, () -> serverListConfig); + ServiceCreator creator = ServiceCreator.noPayloadLimiter( + metricsManager, () -> serverListConfig, userAgent, () -> RemotingClientConfig.DEFAULT); LockService remoteLock = creator.createService(LockService.class); TimestampService remoteTime = creator.createService(TimestampService.class); TimestampManagementService remoteManagement = creator.createService(TimestampManagementService.class); @@ -1055,8 +1076,15 @@ private static LockAndTimestampServices createRawLeaderServices( } private static LockAndTimestampServices createRawRemoteServices( - MetricsManager metricsManager, AtlasDbConfig config, String userAgent) { - ServiceCreator creator = ServiceCreator.noPayloadLimiter(metricsManager, userAgent, () -> config.lock().get()); + MetricsManager metricsManager, + AtlasDbConfig config, + Supplier runtimeConfigSupplier, + UserAgent userAgent) { + ServiceCreator creator = ServiceCreator.noPayloadLimiter( + metricsManager, + () -> config.lock().get(), + userAgent, + () -> runtimeConfigSupplier.get().remotingClient()); LockService lockService = creator.createService(LockService.class); TimestampService timeService = creator.createService(TimestampService.class); TimestampManagementService timestampManagementService = creator.createService(TimestampManagementService.class); diff --git a/atlasdb-config/src/main/java/com/palantir/atlasdb/factory/startup/TimeLockMigrator.java b/atlasdb-config/src/main/java/com/palantir/atlasdb/factory/startup/TimeLockMigrator.java index cac0eb22eb4..903d40536dc 100644 --- a/atlasdb-config/src/main/java/com/palantir/atlasdb/factory/startup/TimeLockMigrator.java +++ b/atlasdb-config/src/main/java/com/palantir/atlasdb/factory/startup/TimeLockMigrator.java @@ -15,13 +15,8 @@ */ package com.palantir.atlasdb.factory.startup; -import java.util.function.Supplier; - import com.palantir.async.initializer.AsyncInitializer; import com.palantir.atlasdb.AtlasDbConstants; -import com.palantir.atlasdb.config.ServerListConfig; -import com.palantir.atlasdb.factory.ServiceCreator; -import com.palantir.atlasdb.util.MetricsManager; import com.palantir.common.annotation.Idempotent; import com.palantir.common.exception.AtlasDbDependencyException; import com.palantir.timestamp.TimestampManagementService; @@ -73,14 +68,6 @@ public void migrate() { initialize(initializeAsync); } - private static TimestampManagementService createRemoteManagementService( - MetricsManager metricsManager, - Supplier serverListConfig, - String userAgent) { - return ServiceCreator.noPayloadLimiter(metricsManager, userAgent, serverListConfig) - .createService(TimestampManagementService.class); - } - @Override @SuppressWarnings({"CheckReturnValue", "ResultOfMethodCallIgnored"}) // errorprone doesn't pick up "when=NEVER" protected synchronized void tryInitialize() { diff --git a/atlasdb-config/src/main/java/com/palantir/atlasdb/http/AtlasDbHttpClients.java b/atlasdb-config/src/main/java/com/palantir/atlasdb/http/AtlasDbHttpClients.java index db57cf9775b..c67fb186f30 100644 --- a/atlasdb-config/src/main/java/com/palantir/atlasdb/http/AtlasDbHttpClients.java +++ b/atlasdb-config/src/main/java/com/palantir/atlasdb/http/AtlasDbHttpClients.java @@ -15,18 +15,14 @@ */ package com.palantir.atlasdb.http; -import java.net.ProxySelector; -import java.util.Collection; import java.util.Optional; -import java.util.function.Function; import java.util.function.Supplier; import com.codahale.metrics.MetricRegistry; import com.google.common.annotations.VisibleForTesting; +import com.palantir.atlasdb.config.AuxiliaryRemotingParameters; import com.palantir.atlasdb.config.ServerListConfig; import com.palantir.atlasdb.util.AtlasDbMetrics; -import com.palantir.conjure.java.api.config.service.ProxyConfiguration; -import com.palantir.conjure.java.api.config.ssl.SslConfiguration; import com.palantir.conjure.java.config.ssl.TrustContext; import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; @@ -38,43 +34,16 @@ private AtlasDbHttpClients() { // Utility class } - /** - * Constructs a dynamic proxy for the specified type, using the supplied SSL factory if is present, and the - * default Feign HTTP client. - */ - public static T createProxy( - MetricRegistry metricRegistry, - Optional trustContext, - String uri, - Class type) { - return createProxy(metricRegistry, trustContext, uri, type, UserAgents.DEFAULT_USER_AGENT, false); - } - public static T createProxy( MetricRegistry metricRegistry, Optional trustContext, String uri, Class type, - String userAgent, - boolean limitPayloadSize) { + AuxiliaryRemotingParameters parameters) { return AtlasDbMetrics.instrument( metricRegistry, type, - DEFAULT_TARGET_FACTORY.createProxy(trustContext, uri, type, userAgent, limitPayloadSize), - MetricRegistry.name(type)); - } - - public static T createProxyWithoutRetrying( - MetricRegistry metricRegistry, - Optional trustContext, - String uri, - Class type, - String userAgent, - boolean limitPayloadSize) { - return AtlasDbMetrics.instrument( - metricRegistry, - type, - DEFAULT_TARGET_FACTORY.createProxyWithoutRetrying(trustContext, uri, type, userAgent, limitPayloadSize), + DEFAULT_TARGET_FACTORY.createProxy(trustContext, uri, type, parameters).instance(), MetricRegistry.name(type)); } @@ -87,52 +56,33 @@ public static T createProxyWithoutRetrying( */ public static T createProxyWithFailover( MetricRegistry metricRegistry, - Optional trustContext, - Optional proxySelector, - Collection endpointUris, - String userAgent, - Class type) { + ServerListConfig serverListConfig, + Class type, + AuxiliaryRemotingParameters parameters) { return AtlasDbMetrics.instrument( metricRegistry, type, - DEFAULT_TARGET_FACTORY.createProxyWithFailover( - trustContext, - proxySelector, - endpointUris, - type, - userAgent, - false), + DEFAULT_TARGET_FACTORY.createProxyWithFailover(serverListConfig, type, parameters).instance(), MetricRegistry.name(type)); } public static T createLiveReloadingProxyWithFailover( TaggedMetricRegistry taggedMetricRegistry, Supplier serverListConfigSupplier, - Function trustContextCreator, - Function proxySelectorCreator, Class type, - String userAgent, - boolean limitPayload) { + AuxiliaryRemotingParameters clientParameters) { return VersionSelectingClients.createVersionSelectingClient( taggedMetricRegistry, // TODO (jkong): Replace the new client with the CJR one; also I wish there was a way to curry stuff - ImmutableInstanceAndVersion.of(DEFAULT_TARGET_FACTORY.createLiveReloadingProxyWithFailover( + DEFAULT_TARGET_FACTORY.createLiveReloadingProxyWithFailover( serverListConfigSupplier, - trustContextCreator, - proxySelectorCreator, type, - userAgent, - limitPayload), - DEFAULT_TARGET_FACTORY.getClientVersion()), - ImmutableInstanceAndVersion.of(DEFAULT_TARGET_FACTORY.createLiveReloadingProxyWithFailover( + clientParameters), + DEFAULT_TARGET_FACTORY.createLiveReloadingProxyWithFailover( serverListConfigSupplier, - trustContextCreator, - proxySelectorCreator, type, - userAgent, - limitPayload), - DEFAULT_TARGET_FACTORY.getClientVersion()), - () -> 0.0, + clientParameters), + () -> clientParameters.remotingClientConfig().get().maximumConjureRemotingProbability(), type); } @@ -140,40 +90,31 @@ public static T createLiveReloadingProxyWithFailover( static T createLiveReloadingProxyWithQuickFailoverForTesting( MetricRegistry metricRegistry, Supplier serverListConfigSupplier, - Function trustContextCreator, - Function proxySelectorCreator, Class type, - String userAgent) { + AuxiliaryRemotingParameters parameters) { return AtlasDbMetrics.instrument( metricRegistry, type, TESTING_TARGET_FACTORY.createLiveReloadingProxyWithFailover( serverListConfigSupplier, - trustContextCreator, - proxySelectorCreator, type, - userAgent, - false), + parameters).instance(), MetricRegistry.name(type)); } @VisibleForTesting static T createProxyWithQuickFailoverForTesting( MetricRegistry metricRegistry, - Optional trustContext, - Optional proxySelector, - Collection endpointUris, - Class type) { + ServerListConfig serverListConfig, + Class type, + AuxiliaryRemotingParameters parameters) { return AtlasDbMetrics.instrument( metricRegistry, type, TESTING_TARGET_FACTORY.createProxyWithFailover( - trustContext, - proxySelector, - endpointUris, + serverListConfig, type, - UserAgents.DEFAULT_USER_AGENT, - false), + parameters).instance(), MetricRegistry.name(type, "atlasdb-testing")); } } diff --git a/atlasdb-config/src/main/java/com/palantir/atlasdb/http/VersionSelectingClients.java b/atlasdb-config/src/main/java/com/palantir/atlasdb/http/VersionSelectingClients.java index 6eff2d12fbf..25487fd048a 100644 --- a/atlasdb-config/src/main/java/com/palantir/atlasdb/http/VersionSelectingClients.java +++ b/atlasdb-config/src/main/java/com/palantir/atlasdb/http/VersionSelectingClients.java @@ -19,8 +19,6 @@ import java.util.concurrent.ThreadLocalRandom; import java.util.function.DoubleSupplier; -import org.immutables.value.Value; - import com.codahale.metrics.MetricRegistry; import com.google.common.collect.ImmutableMap; import com.palantir.atlasdb.util.AtlasDbMetrics; @@ -42,8 +40,8 @@ private VersionSelectingClients() { static T createVersionSelectingClient( TaggedMetricRegistry taggedMetricRegistry, - InstanceAndVersion newClient, - InstanceAndVersion legacyClient, + TargetFactory.InstanceAndVersion newClient, + TargetFactory.InstanceAndVersion legacyClient, DoubleSupplier newClientProbabilitySupplier, Class clazz) { T instrumentedNewClient = instrumentWithClientVersionTag( @@ -60,22 +58,14 @@ static T createVersionSelectingClient( private static T instrumentWithClientVersionTag( TaggedMetricRegistry taggedMetricRegistry, - InstanceAndVersion client, + TargetFactory.InstanceAndVersion client, Class clazz) { return AtlasDbMetrics.instrumentWithTaggedMetrics( taggedMetricRegistry, clazz, - client.client(), + client.instance(), MetricRegistry.name(clazz), $ -> ImmutableMap.of(CLIENT_VERSION, client.version())); } - @Value.Immutable - interface InstanceAndVersion { - @Value.Parameter - T client(); - - @Value.Parameter - String version(); - } } diff --git a/atlasdb-config/src/test/java/com/palantir/atlasdb/factory/LeadersTest.java b/atlasdb-config/src/test/java/com/palantir/atlasdb/factory/LeadersTest.java index aee5ac2b34b..dd109d15101 100644 --- a/atlasdb-config/src/test/java/com/palantir/atlasdb/factory/LeadersTest.java +++ b/atlasdb-config/src/test/java/com/palantir/atlasdb/factory/LeadersTest.java @@ -34,7 +34,7 @@ import com.codahale.metrics.MetricRegistry; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; -import com.palantir.atlasdb.http.UserAgents; +import com.palantir.atlasdb.http.AtlasDbRemotingConstants; import com.palantir.paxos.PaxosAcceptor; import com.palantir.paxos.PaxosLearner; import com.palantir.paxos.PaxosValue; @@ -55,7 +55,7 @@ public void canCreateProxyAndLocalListOfPaxosLearners() { REMOTE_SERVICE_ADDRESSES, Optional.empty(), PaxosLearner.class, - UserAgents.DEFAULT_USER_AGENT); + AtlasDbRemotingConstants.DEFAULT_USER_AGENT); MatcherAssert.assertThat(paxosLearners.size(), is(REMOTE_SERVICE_ADDRESSES.size() + 1)); paxosLearners.forEach(object -> MatcherAssert.assertThat(object, not(nullValue()))); @@ -75,7 +75,7 @@ public void canCreateProxyAndLocalListOfPaxosAcceptors() { REMOTE_SERVICE_ADDRESSES, Optional.empty(), PaxosAcceptor.class, - UserAgents.DEFAULT_USER_AGENT); + AtlasDbRemotingConstants.DEFAULT_USER_AGENT); MatcherAssert.assertThat(paxosAcceptors.size(), is(REMOTE_SERVICE_ADDRESSES.size() + 1)); paxosAcceptors.forEach(object -> MatcherAssert.assertThat(object, not(nullValue()))); @@ -96,7 +96,7 @@ public void createProxyAndLocalListCreatesSingletonListIfNoRemoteAddressesProvid ImmutableSet.of(), Optional.empty(), PaxosAcceptor.class, - UserAgents.DEFAULT_USER_AGENT); + AtlasDbRemotingConstants.DEFAULT_USER_AGENT); MatcherAssert.assertThat(paxosAcceptors.size(), is(1)); @@ -115,7 +115,7 @@ public void createProxyAndLocalListThrowsIfCreatingObjectsWithoutHttpMethodAnnot REMOTE_SERVICE_ADDRESSES, Optional.empty(), BigInteger.class, - UserAgents.DEFAULT_USER_AGENT); + AtlasDbRemotingConstants.DEFAULT_USER_AGENT); } @Test(expected = NullPointerException.class) @@ -128,6 +128,6 @@ public void createProxyAndLocalListThrowsIfNullClassProvided() { REMOTE_SERVICE_ADDRESSES, Optional.empty(), null, - UserAgents.DEFAULT_USER_AGENT); + AtlasDbRemotingConstants.DEFAULT_USER_AGENT); } } diff --git a/atlasdb-config/src/test/java/com/palantir/atlasdb/factory/TransactionManagersTest.java b/atlasdb-config/src/test/java/com/palantir/atlasdb/factory/TransactionManagersTest.java index 5f352fa9386..cd8b4f0d5da 100644 --- a/atlasdb-config/src/test/java/com/palantir/atlasdb/factory/TransactionManagersTest.java +++ b/atlasdb-config/src/test/java/com/palantir/atlasdb/factory/TransactionManagersTest.java @@ -79,10 +79,11 @@ import com.palantir.atlasdb.config.ImmutableTimeLockClientConfig; import com.palantir.atlasdb.config.ImmutableTimeLockRuntimeConfig; import com.palantir.atlasdb.config.ImmutableTimestampClientConfig; +import com.palantir.atlasdb.config.RemotingClientConfig; import com.palantir.atlasdb.config.ServerListConfig; import com.palantir.atlasdb.config.TimeLockClientConfig; import com.palantir.atlasdb.factory.startup.TimeLockMigrator; -import com.palantir.atlasdb.http.UserAgents; +import com.palantir.atlasdb.http.AtlasDbRemotingConstants; import com.palantir.atlasdb.keyvalue.api.KeyValueService; import com.palantir.atlasdb.keyvalue.api.TableReference; import com.palantir.atlasdb.keyvalue.impl.SweepStatsKeyValueService; @@ -98,6 +99,8 @@ import com.palantir.atlasdb.transaction.api.TransactionManager; import com.palantir.atlasdb.util.MetricsManager; import com.palantir.atlasdb.util.MetricsManagers; +import com.palantir.conjure.java.api.config.service.UserAgent; +import com.palantir.conjure.java.api.config.service.UserAgents; import com.palantir.exception.NotInitializedException; import com.palantir.leader.PingableLeader; import com.palantir.lock.LockMode; @@ -119,7 +122,10 @@ public class TransactionManagersTest { private static final String CLIENT = "testClient"; private static final String USER_AGENT_NAME = "user-agent"; - private static final String USER_AGENT = USER_AGENT_NAME + " (3.14159265)"; + private static final String USER_AGENT_VERSION = "3.1415926.5358979"; + private static final UserAgent USER_AGENT = UserAgent.of(UserAgent.Agent.of(USER_AGENT_NAME, USER_AGENT_VERSION)); + private static final String EXPECTED_USER_AGENT_STRING = UserAgents.format(USER_AGENT.addAgent( + AtlasDbRemotingConstants.LEGACY_ATLASDB_HTTP_CLIENT_AGENT)); private static final String USER_AGENT_HEADER = "User-Agent"; private static final long EMBEDDED_BOUND = 3; @@ -214,6 +220,7 @@ public void setup() throws JsonProcessingException { runtimeConfig = mock(AtlasDbRuntimeConfig.class); when(runtimeConfig.timestampClient()).thenReturn(ImmutableTimestampClientConfig.of(false)); when(runtimeConfig.timelockRuntime()).thenReturn(Optional.empty()); + when(runtimeConfig.remotingClient()).thenReturn(RemotingClientConfig.DEFAULT); environment = mock(Consumer.class); @@ -257,7 +264,7 @@ public void userAgentsPresentOnRequestsWithLeaderBlockConfigured() throws IOExce } @Test - public void remoteCallsStillMadeIfPingableLeader404s() throws IOException, InterruptedException { + public void remoteCallsStillMadeIfPingableLeader404s() throws IOException { setUpForRemoteServices(); setUpLeaderBlockInConfig(); @@ -268,13 +275,13 @@ public void remoteCallsStillMadeIfPingableLeader404s() throws IOException, Inter lockAndTimestamp.lock().currentTimeMillis(); availableServer.verify(postRequestedFor(urlMatching(TIMESTAMP_PATH)) - .withHeader(USER_AGENT_HEADER, WireMock.equalTo(USER_AGENT))); + .withHeader(USER_AGENT_HEADER, WireMock.equalTo(EXPECTED_USER_AGENT_STRING))); availableServer.verify(postRequestedFor(urlMatching(LOCK_PATH)) - .withHeader(USER_AGENT_HEADER, WireMock.equalTo(USER_AGENT))); + .withHeader(USER_AGENT_HEADER, WireMock.equalTo(EXPECTED_USER_AGENT_STRING))); } @Test - public void remoteCallsElidedIfTalkingToLocalServer() throws IOException, InterruptedException { + public void remoteCallsElidedIfTalkingToLocalServer() throws IOException { setUpForLocalServices(); setUpLeaderBlockInConfig(); @@ -285,9 +292,9 @@ public void remoteCallsElidedIfTalkingToLocalServer() throws IOException, Interr lockAndTimestamp.lock().currentTimeMillis(); availableServer.verify(0, postRequestedFor(urlMatching(TIMESTAMP_PATH)) - .withHeader(USER_AGENT_HEADER, WireMock.equalTo(USER_AGENT))); + .withHeader(USER_AGENT_HEADER, WireMock.equalTo(EXPECTED_USER_AGENT_STRING))); availableServer.verify(0, postRequestedFor(urlMatching(LOCK_PATH)) - .withHeader(USER_AGENT_HEADER, WireMock.equalTo(USER_AGENT))); + .withHeader(USER_AGENT_HEADER, WireMock.equalTo(EXPECTED_USER_AGENT_STRING))); } @Test @@ -333,7 +340,7 @@ public void canDropTablesWhenSweepQueueWritesAreDisabled() { .build(); KeyValueService kvs = TransactionManagers.builder() .config(inMemoryNoQueueWrites) - .userAgent(UserAgents.DEFAULT_USER_AGENT) + .userAgent(EXPECTED_USER_AGENT_STRING) .globalMetricsRegistry(new MetricRegistry()) .globalTaggedMetricRegistry(DefaultTaggedMetricRegistry.getDefault()) .build() @@ -753,9 +760,9 @@ private void verifyUserAgentOnTimelockTimestampAndLockRequests() { verifyUserAgentOnTimestampAndLockRequests(TIMELOCK_TIMESTAMP_PATH, TIMELOCK_LOCK_PATH); verify(invalidator, times(1)).backupAndInvalidate(); availableServer.verify(getRequestedFor(urlEqualTo(TIMELOCK_PING_PATH)) - .withHeader(USER_AGENT_HEADER, WireMock.equalTo(USER_AGENT))); + .withHeader(USER_AGENT_HEADER, WireMock.equalTo(EXPECTED_USER_AGENT_STRING))); availableServer.verify(postRequestedFor(urlEqualTo(TIMELOCK_FF_PATH)) - .withHeader(USER_AGENT_HEADER, WireMock.equalTo(USER_AGENT))); + .withHeader(USER_AGENT_HEADER, WireMock.equalTo(EXPECTED_USER_AGENT_STRING))); } private void verifyUserAgentOnTimestampAndLockRequests(String timestampPath, String lockPath) { @@ -774,9 +781,9 @@ private void verifyUserAgentOnTimestampAndLockRequests(String timestampPath, Str lockAndTimestamp.timelock().currentTimeMillis(); availableServer.verify(postRequestedFor(urlMatching(timestampPath)) - .withHeader(USER_AGENT_HEADER, WireMock.equalTo(USER_AGENT))); + .withHeader(USER_AGENT_HEADER, WireMock.equalTo(EXPECTED_USER_AGENT_STRING))); availableServer.verify(postRequestedFor(urlMatching(lockPath)) - .withHeader(USER_AGENT_HEADER, WireMock.equalTo(USER_AGENT))); + .withHeader(USER_AGENT_HEADER, WireMock.equalTo(EXPECTED_USER_AGENT_STRING))); } private void assertGetLockAndTimestampServicesThrows() { diff --git a/atlasdb-config/src/test/java/com/palantir/atlasdb/http/AtlasDbHttpClientsTest.java b/atlasdb-config/src/test/java/com/palantir/atlasdb/http/AtlasDbHttpClientsTest.java index 56a63a699fd..c922121e152 100644 --- a/atlasdb-config/src/test/java/com/palantir/atlasdb/http/AtlasDbHttpClientsTest.java +++ b/atlasdb-config/src/test/java/com/palantir/atlasdb/http/AtlasDbHttpClientsTest.java @@ -27,7 +27,6 @@ import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; import static com.github.tomakehurst.wiremock.client.WireMock.urlMatching; -import java.net.ProxySelector; import java.util.List; import java.util.Optional; import java.util.Set; @@ -53,12 +52,12 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; import com.google.common.util.concurrent.Uninterruptibles; +import com.palantir.atlasdb.config.AuxiliaryRemotingParameters; import com.palantir.atlasdb.config.ImmutableServerListConfig; import com.palantir.atlasdb.config.ServerListConfig; -import com.palantir.atlasdb.factory.ServiceCreator; import com.palantir.common.remoting.ServiceNotAvailableException; import com.palantir.conjure.java.api.config.service.ProxyConfiguration; -import com.palantir.conjure.java.config.ssl.SslSocketFactories; +import com.palantir.conjure.java.api.config.service.UserAgent; import com.palantir.conjure.java.config.ssl.TrustContext; public class AtlasDbHttpClientsTest { @@ -69,6 +68,20 @@ public class AtlasDbHttpClientsTest { private static final MappingBuilder POST_MAPPING = post(urlEqualTo(POST_ENDPOINT)); private static final int TEST_NUMBER = 12; + private static final String DEFAULT_USER_AGENT = "bla/0.1.2 atlasdb-http-client/1.0"; + private static final AuxiliaryRemotingParameters AUXILIARY_REMOTING_PARAMETERS_NO_PAYLOAD_LIMIT + = AuxiliaryRemotingParameters.builder() + .shouldLimitPayload(false) + .userAgent(UserAgent.of(UserAgent.Agent.of("bla", "0.1.2"))) + .shouldRetry(true) + .build(); + private static final AuxiliaryRemotingParameters AUXILIARY_REMOTING_PARAMETERS_WITH_PAYLOAD_LIMIT + = AuxiliaryRemotingParameters.builder() + .shouldLimitPayload(true) + .userAgent(UserAgent.of(UserAgent.Agent.of("bla", "0.1.2"))) + .shouldRetry(true) + .build(); + private int availablePort; private int unavailablePort; private int proxyPort; @@ -114,8 +127,12 @@ public void setup() { @Test public void payloadLimitingClientThrowsOnRequestThatIsTooLarge() { - TestResource client = AtlasDbHttpClients.createProxy(new MetricRegistry(), NO_SSL, getUriForPort(availablePort), - TestResource.class, UserAgents.DEFAULT_USER_AGENT, true); + TestResource client = AtlasDbHttpClients.createProxy( + new MetricRegistry(), + NO_SSL, + getUriForPort(availablePort), + TestResource.class, + AUXILIARY_REMOTING_PARAMETERS_WITH_PAYLOAD_LIMIT); assertThat(client.postRequest(new byte[AtlasDbInterceptors.MAX_PAYLOAD_SIZE / 2])) .as("Request with payload size below limit succeeds") .isTrue(); @@ -127,8 +144,13 @@ public void payloadLimitingClientThrowsOnRequestThatIsTooLarge() { @Test public void regularClientDoesNotThrowOnRequestThatIsTooLarge() { - TestResource client = AtlasDbHttpClients.createProxy(new MetricRegistry(), NO_SSL, getUriForPort(availablePort), - TestResource.class); + TestResource client + = AtlasDbHttpClients.createProxy( + new MetricRegistry(), + NO_SSL, + getUriForPort(availablePort), + TestResource.class, + AUXILIARY_REMOTING_PARAMETERS_NO_PAYLOAD_LIMIT); assertThat(client.postRequest(new byte[AtlasDbInterceptors.MAX_PAYLOAD_SIZE])) .as("Request with payload size exceeding limit succeeds when not limiting payload size") .isTrue(); @@ -138,9 +160,11 @@ public void regularClientDoesNotThrowOnRequestThatIsTooLarge() { public void ifOneServerResponds503WithNoRetryHeaderTheRequestIsRerouted() { unavailableServer.stubFor(GET_MAPPING.willReturn(aResponse().withStatus(503))); - TestResource client = AtlasDbHttpClients.createProxyWithFailover(new MetricRegistry(), - NO_SSL, - Optional.empty(), bothUris, UserAgents.DEFAULT_USER_AGENT, TestResource.class); + TestResource client = AtlasDbHttpClients.createProxyWithFailover( + new MetricRegistry(), + ImmutableServerListConfig.builder().addAllServers(bothUris).build(), + TestResource.class, + AUXILIARY_REMOTING_PARAMETERS_NO_PAYLOAD_LIMIT); int response = client.getTestNumber(); assertThat(response, equalTo(TEST_NUMBER)); @@ -149,50 +173,48 @@ public void ifOneServerResponds503WithNoRetryHeaderTheRequestIsRerouted() { @Test public void userAgentIsPresentOnClientRequests() { - TestResource client = - AtlasDbHttpClients.createProxy( - new MetricRegistry(), NO_SSL, getUriForPort(availablePort), TestResource.class); + TestResource client = AtlasDbHttpClients.createProxy( + new MetricRegistry(), + NO_SSL, + getUriForPort(availablePort), + TestResource.class, + AUXILIARY_REMOTING_PARAMETERS_NO_PAYLOAD_LIMIT); client.getTestNumber(); - String defaultUserAgent = UserAgents.fromStrings(UserAgents.DEFAULT_VALUE, UserAgents.DEFAULT_VALUE); availableServer.verify(getRequestedFor(urlMatching(GET_ENDPOINT)) - .withHeader(AtlasDbInterceptors.USER_AGENT_HEADER, WireMock.equalTo(defaultUserAgent))); + .withHeader(AtlasDbInterceptors.USER_AGENT_HEADER, WireMock.equalTo(DEFAULT_USER_AGENT))); } @Test public void directProxyIsConfigurableOnClientRequests() { - Optional directProxySelector = Optional.of( - ServiceCreator.createProxySelector(ProxyConfiguration.DIRECT)); TestResource clientWithDirectCall = AtlasDbHttpClients.createProxyWithFailover( new MetricRegistry(), - NO_SSL, - directProxySelector, - ImmutableSet.of(getUriForPort(availablePort)), - UserAgents.DEFAULT_USER_AGENT, - TestResource.class); + ImmutableServerListConfig.builder() + .addServers(getUriForPort(availablePort)) + .proxyConfiguration(ProxyConfiguration.DIRECT) + .build(), + TestResource.class, + AUXILIARY_REMOTING_PARAMETERS_NO_PAYLOAD_LIMIT); clientWithDirectCall.getTestNumber(); - String defaultUserAgent = UserAgents.fromStrings(UserAgents.DEFAULT_VALUE, UserAgents.DEFAULT_VALUE); availableServer.verify(getRequestedFor(urlMatching(GET_ENDPOINT)) - .withHeader(AtlasDbInterceptors.USER_AGENT_HEADER, WireMock.equalTo(defaultUserAgent))); + .withHeader(AtlasDbInterceptors.USER_AGENT_HEADER, WireMock.equalTo(DEFAULT_USER_AGENT))); } @Test public void httpProxyIsConfigurableOnClientRequests() { - Optional httpProxySelector = Optional.of( - ServiceCreator.createProxySelector(ProxyConfiguration.of(getHostAndPort(proxyPort)))); TestResource clientWithHttpProxy = AtlasDbHttpClients.createProxyWithFailover( new MetricRegistry(), - NO_SSL, - httpProxySelector, - ImmutableSet.of(getUriForPort(availablePort)), - UserAgents.DEFAULT_USER_AGENT, - TestResource.class); + ImmutableServerListConfig.builder() + .addServers(getUriForPort(availablePort)) + .proxyConfiguration(ProxyConfiguration.of(getHostAndPort(proxyPort))) + .build(), + TestResource.class, + AUXILIARY_REMOTING_PARAMETERS_NO_PAYLOAD_LIMIT); clientWithHttpProxy.getTestNumber(); - String defaultUserAgent = UserAgents.fromStrings(UserAgents.DEFAULT_VALUE, UserAgents.DEFAULT_VALUE); proxyServer.verify(getRequestedFor(urlMatching(GET_ENDPOINT)) - .withHeader(AtlasDbInterceptors.USER_AGENT_HEADER, WireMock.equalTo(defaultUserAgent))); + .withHeader(AtlasDbInterceptors.USER_AGENT_HEADER, WireMock.equalTo(DEFAULT_USER_AGENT))); availableServer.verify(0, getRequestedFor(urlMatching(GET_ENDPOINT))); } @@ -207,10 +229,8 @@ public void canLiveReloadServersList() { () -> ImmutableServerListConfig.builder() .servers(servers) .build(), - SslSocketFactories::createTrustContext, - unused -> ProxySelector.getDefault(), TestResource.class, - "user (123)"); + AUXILIARY_REMOTING_PARAMETERS_NO_PAYLOAD_LIMIT); // actually a Feign RetryableException but that's not on our classpath assertThatThrownBy(client::getTestNumber).isInstanceOf(RuntimeException.class); @@ -229,10 +249,8 @@ public void httpProxyThrowsServiceNotAvailableExceptionIfConfiguredWithZeroNodes TestResource testResource = AtlasDbHttpClients.createLiveReloadingProxyWithQuickFailoverForTesting( new MetricRegistry(), () -> ImmutableServerListConfig.builder().build(), - SslSocketFactories::createTrustContext, - proxyConfiguration -> ProxySelector.getDefault(), TestResource.class, - UserAgents.DEFAULT_VALUE); + AUXILIARY_REMOTING_PARAMETERS_NO_PAYLOAD_LIMIT); assertThatThrownBy(testResource::getTestNumber).isInstanceOf(ServiceNotAvailableException.class); } @@ -244,10 +262,8 @@ public void httpProxyCanBeCommissionedAndDecommissionedIfNodeAvailabilityChanges TestResource testResource = AtlasDbHttpClients.createLiveReloadingProxyWithQuickFailoverForTesting( new MetricRegistry(), config::get, - SslSocketFactories::createTrustContext, - proxyConfiguration -> ProxySelector.getDefault(), TestResource.class, - UserAgents.DEFAULT_VALUE); + AUXILIARY_REMOTING_PARAMETERS_NO_PAYLOAD_LIMIT); // At this point, there are zero nodes in the config, so we should get ServiceNotAvailable. assertThatThrownBy(testResource::getTestNumber).isInstanceOf(ServiceNotAvailableException.class); @@ -270,4 +286,5 @@ private static String getUriForPort(int port) { private static String getHostAndPort(int port) { return String.format("%s:%s", WireMockConfiguration.DEFAULT_BIND_ADDRESS, port); } + } diff --git a/atlasdb-ete-tests/src/test/java/com/palantir/atlasdb/ete/EteSetup.java b/atlasdb-ete-tests/src/test/java/com/palantir/atlasdb/ete/EteSetup.java index dc3a1573f88..dfadf89184a 100644 --- a/atlasdb-ete-tests/src/test/java/com/palantir/atlasdb/ete/EteSetup.java +++ b/atlasdb-ete-tests/src/test/java/com/palantir/atlasdb/ete/EteSetup.java @@ -32,8 +32,9 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; +import com.palantir.atlasdb.config.ImmutableServerListConfig; import com.palantir.atlasdb.http.AtlasDbHttpClients; -import com.palantir.atlasdb.http.UserAgents; +import com.palantir.atlasdb.http.TestProxyUtils; import com.palantir.atlasdb.todo.TodoResource; import com.palantir.conjure.java.config.ssl.TrustContext; import com.palantir.docker.compose.DockerComposeRule; @@ -185,15 +186,18 @@ private static T createClientToMultipleNodes(Class clazz, List no return AtlasDbHttpClients.createProxyWithFailover( new MetricRegistry(), - NO_SSL, - Optional.empty(), - uris, - UserAgents.DEFAULT_USER_AGENT, - clazz); + ImmutableServerListConfig.builder().addAllServers(uris).build(), + clazz, + TestProxyUtils.AUXILIARY_REMOTING_PARAMETERS); } private static T createClientFor(Class clazz, String host, short port) { String uri = String.format("http://%s:%s", host, port); - return AtlasDbHttpClients.createProxy(new MetricRegistry(), NO_SSL, uri, clazz); + return AtlasDbHttpClients.createProxy( + new MetricRegistry(), + NO_SSL, + uri, + clazz, + TestProxyUtils.AUXILIARY_REMOTING_PARAMETERS); } } diff --git a/atlasdb-ete-tests/src/test/java/com/palantir/atlasdb/ete/TimeLockMigrationEteTest.java b/atlasdb-ete-tests/src/test/java/com/palantir/atlasdb/ete/TimeLockMigrationEteTest.java index 4c796b94309..d972eeb5d41 100644 --- a/atlasdb-ete-tests/src/test/java/com/palantir/atlasdb/ete/TimeLockMigrationEteTest.java +++ b/atlasdb-ete-tests/src/test/java/com/palantir/atlasdb/ete/TimeLockMigrationEteTest.java @@ -33,6 +33,7 @@ import com.codahale.metrics.MetricRegistry; import com.palantir.atlasdb.http.AtlasDbHttpClients; +import com.palantir.atlasdb.http.TestProxyUtils; import com.palantir.atlasdb.http.errors.AtlasDbRemoteException; import com.palantir.atlasdb.todo.ImmutableTodo; import com.palantir.atlasdb.todo.Todo; @@ -181,11 +182,21 @@ private static Callable serversAreReady() { private static T createEteClientFor(Class clazz) { String uri = String.format("http://%s:%s", ETE_CONTAINER, ETE_PORT); - return AtlasDbHttpClients.createProxy(new MetricRegistry(), Optional.empty(), uri, clazz); + return AtlasDbHttpClients.createProxy( + new MetricRegistry(), + Optional.empty(), + uri, + clazz, + TestProxyUtils.AUXILIARY_REMOTING_PARAMETERS); } private static TimestampService createTimeLockTimestampClient() { String uri = String.format("http://%s:%s/%s", TIMELOCK_CONTAINER, TIMELOCK_PORT, TEST_CLIENT); - return AtlasDbHttpClients.createProxy(new MetricRegistry(), Optional.empty(), uri, TimestampService.class); + return AtlasDbHttpClients.createProxy( + new MetricRegistry(), + Optional.empty(), + uri, + TimestampService.class, + TestProxyUtils.AUXILIARY_REMOTING_PARAMETERS); } } diff --git a/atlasdb-feign/src/main/java/com/palantir/atlasdb/config/AuxiliaryRemotingParameters.java b/atlasdb-feign/src/main/java/com/palantir/atlasdb/config/AuxiliaryRemotingParameters.java new file mode 100644 index 00000000000..a1ae480b538 --- /dev/null +++ b/atlasdb-feign/src/main/java/com/palantir/atlasdb/config/AuxiliaryRemotingParameters.java @@ -0,0 +1,48 @@ +/* + * (c) Copyright 2019 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.atlasdb.config; + +import java.util.function.Supplier; + +import org.immutables.value.Value; + +import com.palantir.conjure.java.api.config.service.UserAgent; + +/** + * Additional parameters for clients to specify when connecting to remote services. + */ +@Value.Immutable +public interface AuxiliaryRemotingParameters { + UserAgent userAgent(); + + boolean shouldLimitPayload(); + + /** + * Whether clients should retry in the event of connection failures. + * This value may be ignored and assumed to be true for proxies that implement failover. + */ + boolean shouldRetry(); + + @Value.Default + default Supplier remotingClientConfig() { + return () -> RemotingClientConfig.DEFAULT; + } + + static ImmutableAuxiliaryRemotingParameters.Builder builder() { + return ImmutableAuxiliaryRemotingParameters.builder(); + } +} diff --git a/atlasdb-feign/src/main/java/com/palantir/atlasdb/config/RemotingClientConfig.java b/atlasdb-feign/src/main/java/com/palantir/atlasdb/config/RemotingClientConfig.java new file mode 100644 index 00000000000..dcb4054c7da --- /dev/null +++ b/atlasdb-feign/src/main/java/com/palantir/atlasdb/config/RemotingClientConfig.java @@ -0,0 +1,43 @@ +/* + * (c) Copyright 2019 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.atlasdb.config; + +import org.immutables.value.Value; + +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; +import com.palantir.logsafe.Preconditions; +import com.palantir.logsafe.SafeArg; + +@JsonSerialize(as = ImmutableRemotingClientConfig.class) +@JsonDeserialize(as = ImmutableRemotingClientConfig.class) +@Value.Immutable +public interface RemotingClientConfig { + RemotingClientConfig DEFAULT = ImmutableRemotingClientConfig.builder().build(); + + default double maximumConjureRemotingProbability() { + return 0.0; + } + + @Value.Check + default void check() { + Preconditions.checkState(0.0 <= maximumConjureRemotingProbability() + && maximumConjureRemotingProbability() <= 1.0, + "Maximum probability of choosing v2 must be between 0.0 and 1.0 inclusive", + SafeArg.of("probability", maximumConjureRemotingProbability())); + } +} diff --git a/atlasdb-feign/src/main/java/com/palantir/atlasdb/config/ServerListConfig.java b/atlasdb-feign/src/main/java/com/palantir/atlasdb/config/ServerListConfig.java index 3caa7090f3d..c9b04253cc0 100644 --- a/atlasdb-feign/src/main/java/com/palantir/atlasdb/config/ServerListConfig.java +++ b/atlasdb-feign/src/main/java/com/palantir/atlasdb/config/ServerListConfig.java @@ -24,6 +24,8 @@ import com.fasterxml.jackson.databind.annotation.JsonSerialize; import com.palantir.conjure.java.api.config.service.ProxyConfiguration; import com.palantir.conjure.java.api.config.ssl.SslConfiguration; +import com.palantir.conjure.java.config.ssl.SslSocketFactories; +import com.palantir.conjure.java.config.ssl.TrustContext; @JsonDeserialize(as = ImmutableServerListConfig.class) @JsonSerialize(as = ImmutableServerListConfig.class) @@ -35,6 +37,11 @@ public interface ServerListConfig { Optional proxyConfiguration(); + @Value.Lazy + default Optional trustContext() { + return sslConfiguration().map(SslSocketFactories::createTrustContext); + } + default boolean hasAtLeastOneServer() { return servers().size() >= 1; } diff --git a/atlasdb-feign/src/main/java/com/palantir/atlasdb/http/AtlasDbFeignTargetFactory.java b/atlasdb-feign/src/main/java/com/palantir/atlasdb/http/AtlasDbFeignTargetFactory.java index 30394363b52..27449a7a9b1 100644 --- a/atlasdb-feign/src/main/java/com/palantir/atlasdb/http/AtlasDbFeignTargetFactory.java +++ b/atlasdb-feign/src/main/java/com/palantir/atlasdb/http/AtlasDbFeignTargetFactory.java @@ -15,23 +15,31 @@ */ package com.palantir.atlasdb.http; +import java.io.IOException; +import java.net.InetSocketAddress; +import java.net.Proxy; import java.net.ProxySelector; -import java.util.Collection; +import java.net.SocketAddress; +import java.net.URI; +import java.util.List; import java.util.Optional; -import java.util.function.Function; import java.util.function.Supplier; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.datatype.guava.GuavaModule; import com.fasterxml.jackson.datatype.jdk8.Jdk8Module; import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; +import com.google.common.collect.ImmutableList; +import com.google.common.net.HostAndPort; import com.google.common.reflect.Reflection; +import com.palantir.atlasdb.config.AuxiliaryRemotingParameters; import com.palantir.atlasdb.config.ServerListConfig; import com.palantir.common.remoting.ServiceNotAvailableException; import com.palantir.conjure.java.api.config.service.ProxyConfiguration; -import com.palantir.conjure.java.api.config.ssl.SslConfiguration; +import com.palantir.conjure.java.api.config.service.UserAgent; import com.palantir.conjure.java.config.ssl.TrustContext; import com.palantir.conjure.java.ext.refresh.RefreshableProxyInvocationHandler; +import com.palantir.logsafe.exceptions.SafeIllegalArgumentException; import feign.Client; import feign.Contract; @@ -71,6 +79,7 @@ public final class AtlasDbFeignTargetFactory implements TargetFactory { private static final Decoder decoder = new TextDelegateDecoder( new OptionalAwareDecoder(new AtlasDbJacksonDecoder(mapper))); private static final ErrorDecoder errorDecoder = new AtlasDbErrorDecoder(); + public static final String CLIENT_VERSION_STRING = "AtlasDB-Feign"; private final int connectTimeout; private final int readTimeout; @@ -83,52 +92,35 @@ private AtlasDbFeignTargetFactory(int connectTimeout, int readTimeout, long maxB } @Override - public T createProxyWithoutRetrying( + public InstanceAndVersion createProxy( Optional trustContext, String uri, Class type, - String userAgent, - boolean limitPayloadSize) { - return Feign.builder() - .contract(contract) - .encoder(encoder) - .decoder(decoder) - .errorDecoder(errorDecoder) - .retryer(Retryer.NEVER_RETRY) - .client(createClient(trustContext, userAgent, limitPayloadSize)) - .target(type, uri); - } - - @Override - public T createProxy( - Optional trustContext, - String uri, - Class type, - String userAgent, - boolean limitPayloadSize) { - return Feign.builder() + AuxiliaryRemotingParameters parameters) { + return wrapWithVersion(Feign.builder() .contract(contract) .encoder(encoder) .decoder(decoder) .errorDecoder(errorDecoder) - .retryer(new InterruptHonoringRetryer()) - .client(createClient(trustContext, userAgent, limitPayloadSize)) - .target(type, uri); + .retryer(parameters.shouldRetry() ? new InterruptHonoringRetryer() : Retryer.NEVER_RETRY) + .client(createClient(trustContext, parameters)) + .target(type, uri)); } @Override - public T createProxyWithFailover( - Optional trustContext, - Optional proxySelector, - Collection endpointUris, + public InstanceAndVersion createProxyWithFailover( + ServerListConfig serverListConfig, Class type, - String userAgent, - boolean limitPayloadSize) { - FailoverFeignTarget failoverFeignTarget = new FailoverFeignTarget<>(endpointUris, maxBackoffMillis, type); + AuxiliaryRemotingParameters parameters) { + FailoverFeignTarget failoverFeignTarget = new FailoverFeignTarget<>( + serverListConfig.servers(), maxBackoffMillis, type); Client client = failoverFeignTarget.wrapClient( - FeignOkHttpClients.newRefreshingOkHttpClient(trustContext, proxySelector, userAgent, limitPayloadSize)); + FeignOkHttpClients.newRefreshingOkHttpClient( + serverListConfig.trustContext(), + serverListConfig.proxyConfiguration().map(AtlasDbFeignTargetFactory::createProxySelector), + parameters)); - return Feign.builder() + return wrapWithVersion(Feign.builder() .contract(contract) .encoder(encoder) .decoder(decoder) @@ -136,61 +128,51 @@ public T createProxyWithFailover( .client(client) .retryer(failoverFeignTarget) .options(new Request.Options(connectTimeout, readTimeout)) - .target(failoverFeignTarget); + .target(failoverFeignTarget)); } @Override - public T createLiveReloadingProxyWithFailover( + public InstanceAndVersion createLiveReloadingProxyWithFailover( Supplier serverListConfigSupplier, - Function trustContextCreator, - Function proxySelectorCreator, Class type, - String userAgent, - boolean limitPayload) { + AuxiliaryRemotingParameters parameters) { PollingRefreshable configPollingRefreshable = PollingRefreshable.create(serverListConfigSupplier); - return Reflection.newProxy( + return wrapWithVersion(Reflection.newProxy( type, RefreshableProxyInvocationHandler.create( configPollingRefreshable.getRefreshable(), serverListConfig -> { if (serverListConfig.hasAtLeastOneServer()) { - return createProxyWithFailover( - serverListConfig.sslConfiguration().map(trustContextCreator), - serverListConfig.proxyConfiguration().map(proxySelectorCreator), - serverListConfig.servers(), - type, - userAgent, - limitPayload); + return createProxyWithFailover(serverListConfig, type, parameters).instance(); } return createProxyForZeroNodes(type); - })); - } - - @Override - public String getClientVersion() { - return "AtlasDB-Feign"; + }))); } public static T createRsProxy( Optional trustContext, String uri, Class type, - String userAgent) { + UserAgent userAgent) { + AuxiliaryRemotingParameters remotingParameters = AuxiliaryRemotingParameters.builder() + .userAgent(userAgent) + .shouldLimitPayload(false) // Only used for leader blocks + .shouldRetry(true) + .build(); return Feign.builder() .contract(contract) .encoder(encoder) .decoder(decoder) .errorDecoder(new RsErrorDecoder()) - .client(createClient(trustContext, userAgent, false)) + .client(createClient(trustContext, remotingParameters)) .target(type, uri); } private static Client createClient( Optional trustContext, - String userAgent, - boolean limitPayload) { - return FeignOkHttpClients.newRefreshingOkHttpClient(trustContext, Optional.empty(), userAgent, limitPayload); + AuxiliaryRemotingParameters parameters) { + return FeignOkHttpClients.newRefreshingOkHttpClient(trustContext, Optional.empty(), parameters); } private static T createProxyForZeroNodes(Class type) { @@ -199,4 +181,40 @@ private static T createProxyForZeroNodes(Class type) { + " because configuration contains zero servers."); }); } + + /** + * The code below is copied from http-remoting and should be removed when we switch the clients to use remoting. + */ + private static ProxySelector createProxySelector(ProxyConfiguration proxyConfig) { + switch (proxyConfig.type()) { + case DIRECT: + return fixedProxySelectorFor(Proxy.NO_PROXY); + case HTTP: + HostAndPort hostAndPort = HostAndPort.fromString(proxyConfig.hostAndPort() + .orElseThrow(() -> new SafeIllegalArgumentException( + "Expected to find proxy hostAndPort configuration for HTTP proxy"))); + InetSocketAddress addr = new InetSocketAddress(hostAndPort.getHost(), hostAndPort.getPort()); + return fixedProxySelectorFor(new Proxy(Proxy.Type.HTTP, addr)); + default: + // fall through + } + + throw new IllegalStateException("Failed to create ProxySelector for proxy configuration: " + proxyConfig); + } + + private static ProxySelector fixedProxySelectorFor(Proxy proxy) { + return new ProxySelector() { + @Override + public List select(URI uri) { + return ImmutableList.of(proxy); + } + + @Override + public void connectFailed(URI uri, SocketAddress sa, IOException ioe) {} + }; + } + + private static InstanceAndVersion wrapWithVersion(T instance) { + return ImmutableInstanceAndVersion.of(instance, CLIENT_VERSION_STRING); + } } diff --git a/atlasdb-feign/src/main/java/com/palantir/atlasdb/http/AtlasDbRemotingConstants.java b/atlasdb-feign/src/main/java/com/palantir/atlasdb/http/AtlasDbRemotingConstants.java index 85450ade96e..55c6b331c72 100644 --- a/atlasdb-feign/src/main/java/com/palantir/atlasdb/http/AtlasDbRemotingConstants.java +++ b/atlasdb-feign/src/main/java/com/palantir/atlasdb/http/AtlasDbRemotingConstants.java @@ -22,9 +22,15 @@ public final class AtlasDbRemotingConstants { public static final String ATLASDB_HTTP_CLIENT = "atlasdb-http-client"; public static final AtlasDbHttpProtocolVersion CURRENT_CLIENT_PROTOCOL_VERSION = AtlasDbHttpProtocolVersion.CONJURE_JAVA_RUNTIME; + public static final UserAgent.Agent LEGACY_ATLASDB_HTTP_CLIENT_AGENT + = UserAgent.Agent.of(ATLASDB_HTTP_CLIENT, + AtlasDbHttpProtocolVersion.LEGACY_OR_UNKNOWN.getProtocolVersionString()); public static final UserAgent.Agent ATLASDB_HTTP_CLIENT_AGENT = UserAgent.Agent.of(ATLASDB_HTTP_CLIENT, CURRENT_CLIENT_PROTOCOL_VERSION.getProtocolVersionString()); + public static final UserAgent DEFAULT_USER_AGENT = UserAgent.of( + UserAgent.Agent.of("unknown", UserAgent.Agent.DEFAULT_VERSION)); + private AtlasDbRemotingConstants() { // constants } diff --git a/atlasdb-feign/src/main/java/com/palantir/atlasdb/http/FeignOkHttpClients.java b/atlasdb-feign/src/main/java/com/palantir/atlasdb/http/FeignOkHttpClients.java index b97e776e4ec..d6611ce85fc 100644 --- a/atlasdb-feign/src/main/java/com/palantir/atlasdb/http/FeignOkHttpClients.java +++ b/atlasdb-feign/src/main/java/com/palantir/atlasdb/http/FeignOkHttpClients.java @@ -25,6 +25,8 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; +import com.palantir.atlasdb.config.AuxiliaryRemotingParameters; +import com.palantir.conjure.java.api.config.service.UserAgents; import com.palantir.conjure.java.config.ssl.TrustContext; import feign.Client; @@ -97,10 +99,14 @@ private FeignOkHttpClients() { public static Client newRefreshingOkHttpClient( Optional trustContext, Optional proxySelector, - String userAgent, - boolean limitPayloadSize) { + AuxiliaryRemotingParameters parameters) { Supplier clientSupplier = () -> CounterBackedRefreshingClient.createRefreshingClient( - () -> newOkHttpClient(trustContext, proxySelector, userAgent, limitPayloadSize)); + () -> newOkHttpClient( + trustContext, + proxySelector, + UserAgents.format(parameters.userAgent().addAgent( + AtlasDbRemotingConstants.LEGACY_ATLASDB_HTTP_CLIENT_AGENT)), + parameters.shouldLimitPayload())); return ExceptionCountingRefreshingClient.createRefreshingClient(clientSupplier); } diff --git a/atlasdb-feign/src/main/java/com/palantir/atlasdb/http/TargetFactory.java b/atlasdb-feign/src/main/java/com/palantir/atlasdb/http/TargetFactory.java index be45e63b541..e7a7164f5c8 100644 --- a/atlasdb-feign/src/main/java/com/palantir/atlasdb/http/TargetFactory.java +++ b/atlasdb-feign/src/main/java/com/palantir/atlasdb/http/TargetFactory.java @@ -16,47 +16,38 @@ package com.palantir.atlasdb.http; -import java.net.ProxySelector; -import java.util.Collection; import java.util.Optional; -import java.util.function.Function; import java.util.function.Supplier; +import org.immutables.value.Value; + +import com.palantir.atlasdb.config.AuxiliaryRemotingParameters; import com.palantir.atlasdb.config.ServerListConfig; -import com.palantir.conjure.java.api.config.service.ProxyConfiguration; -import com.palantir.conjure.java.api.config.ssl.SslConfiguration; import com.palantir.conjure.java.config.ssl.TrustContext; public interface TargetFactory { - T createProxyWithoutRetrying( + InstanceAndVersion createProxy( Optional trustContext, String uri, Class type, - String userAgent, - boolean limitPayloadSize); + AuxiliaryRemotingParameters parameters); - T createProxy( - Optional trustContext, - String uri, + InstanceAndVersion createProxyWithFailover( + ServerListConfig serverListConfig, Class type, - String userAgent, - boolean limitPayloadSize); + AuxiliaryRemotingParameters parameters); - T createProxyWithFailover( - Optional trustContext, - Optional proxySelector, - Collection endpointUris, - Class type, - String userAgent, - boolean limitPayloadSize); - - T createLiveReloadingProxyWithFailover( + InstanceAndVersion createLiveReloadingProxyWithFailover( Supplier serverListConfigSupplier, - Function trustContextCreator, - Function proxySelectorCreator, Class type, - String userAgent, - boolean limitPayload); + AuxiliaryRemotingParameters parameters); + + @Value.Immutable + interface InstanceAndVersion { + @Value.Parameter + T instance(); - String getClientVersion(); + @Value.Parameter + String version(); + } } diff --git a/atlasdb-jepsen-tests/src/main/java/com/palantir/atlasdb/http/TimelockUtils.java b/atlasdb-jepsen-tests/src/main/java/com/palantir/atlasdb/http/TimelockUtils.java index 69f8a1d4f2a..34547fc3718 100644 --- a/atlasdb-jepsen-tests/src/main/java/com/palantir/atlasdb/http/TimelockUtils.java +++ b/atlasdb-jepsen-tests/src/main/java/com/palantir/atlasdb/http/TimelockUtils.java @@ -16,10 +16,12 @@ package com.palantir.atlasdb.http; import java.util.List; -import java.util.Optional; import com.codahale.metrics.MetricRegistry; import com.google.common.collect.Lists; +import com.palantir.atlasdb.config.AuxiliaryRemotingParameters; +import com.palantir.atlasdb.config.ImmutableServerListConfig; +import com.palantir.conjure.java.api.config.service.UserAgent; public final class TimelockUtils { private static final int PORT = 8080; @@ -40,9 +42,12 @@ private static List hostnamesToEndpointUris(List hosts) { private static T createFromUris(MetricRegistry metricRegistry, List endpointUris, Class type) { return AtlasDbHttpClients.createProxyWithQuickFailoverForTesting( metricRegistry, - Optional.empty(), - Optional.empty(), - endpointUris, - type); + ImmutableServerListConfig.builder().addAllServers(endpointUris).build(), + type, + AuxiliaryRemotingParameters.builder() + .shouldRetry(true) + .shouldLimitPayload(false) + .userAgent(UserAgent.of(UserAgent.Agent.of("atlasdb-jepsen", UserAgent.Agent.DEFAULT_VERSION))) + .build()); } } diff --git a/atlasdb-tests-shared/build.gradle b/atlasdb-tests-shared/build.gradle index 52abd327e60..ed1ea92e806 100644 --- a/atlasdb-tests-shared/build.gradle +++ b/atlasdb-tests-shared/build.gradle @@ -6,7 +6,7 @@ schemas = [ ] dependencies { - compile project(":atlasdb-impl-shared") + compile project(":atlasdb-config") testCompile project(":atlasdb-config") compile project(path: ":atlasdb-feign", configuration: "shadow") diff --git a/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/http/TestProxyUtils.java b/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/http/TestProxyUtils.java new file mode 100644 index 00000000000..1c2069e26c4 --- /dev/null +++ b/atlasdb-tests-shared/src/main/java/com/palantir/atlasdb/http/TestProxyUtils.java @@ -0,0 +1,33 @@ +/* + * (c) Copyright 2019 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.atlasdb.http; + +import com.palantir.atlasdb.config.AuxiliaryRemotingParameters; +import com.palantir.conjure.java.api.config.service.UserAgent; + +public final class TestProxyUtils { + public static final AuxiliaryRemotingParameters AUXILIARY_REMOTING_PARAMETERS + = AuxiliaryRemotingParameters.builder() + .shouldLimitPayload(false) + .userAgent(UserAgent.of(UserAgent.Agent.of("bla", "0.1.2"))) + .shouldRetry(true) + .build(); + + private TestProxyUtils() { + // constants + } +} diff --git a/changelog/@unreleased/pr-4264.v2.yml b/changelog/@unreleased/pr-4264.v2.yml new file mode 100644 index 00000000000..b7071117191 --- /dev/null +++ b/changelog/@unreleased/pr-4264.v2.yml @@ -0,0 +1,26 @@ +changes: + - type: deprecation + deprecation: + description: "`TransactionManagers#userAgent()` is now deprecated. Users should specify a `structuredUserAgent()` as per Conjure service configuration standards." + links: + - https://github.com/palantir/atlasdb/pull/4264 + - type: break + break: + description: |- + `AtlasDbHttpClients` now expects an `AuxiliaryRemotingParameters` struct which encapsulates information about whether to use payload limiting, the user agent, and any additional remoting client configuration (such as that that may be provided by the AtlasDB library). These were previously expected as primitives. + Whether the client should retry or not is also now embedded in this parameter object. + + To replicate previous behaviour, users should create an `AuxiliaryRemotingParameters` struct, possibly as follows: + + ``` + AuxiliaryRemotingParameters.builder() + .userAgent(UserAgents.tryParse(userAgentString)) + .shouldLimitPayload(false) + .remotingClientConfig(remotingClientConfigSupplier) + .shouldRetry(true) + .build(); + ``` + + `userAgent` and `remotingClientConfig` are optional; `shouldLimitPayload` is compulsory. + links: + - https://github.com/palantir/atlasdb/pull/4264 diff --git a/timelock-agent/src/main/java/com/palantir/atlasdb/timelock/paxos/PaxosRemoteClients.java b/timelock-agent/src/main/java/com/palantir/atlasdb/timelock/paxos/PaxosRemoteClients.java index c15f49c490d..7f7c92c7cff 100644 --- a/timelock-agent/src/main/java/com/palantir/atlasdb/timelock/paxos/PaxosRemoteClients.java +++ b/timelock-agent/src/main/java/com/palantir/atlasdb/timelock/paxos/PaxosRemoteClients.java @@ -22,8 +22,10 @@ import org.immutables.value.Value; import com.google.common.collect.ImmutableMap; +import com.palantir.atlasdb.config.AuxiliaryRemotingParameters; import com.palantir.atlasdb.http.AtlasDbHttpClients; import com.palantir.atlasdb.util.AtlasDbMetrics; +import com.palantir.conjure.java.api.config.service.UserAgents; import com.palantir.timelock.paxos.TimelockPaxosAcceptorRpcClient; import com.palantir.timelock.paxos.TimelockPaxosLearnerRpcClient; import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; @@ -68,8 +70,11 @@ private List createInstrumentedRemoteProxies(Class clazz, String name) context().trustContext(), uri, clazz, - name, - false)) + AuxiliaryRemotingParameters.builder() + .userAgent(UserAgents.tryParse(name)) + .shouldLimitPayload(false) + .shouldRetry(true) + .build()).instance()) .map(proxy -> AtlasDbMetrics.instrumentWithTaggedMetrics( metrics(), clazz, diff --git a/timelock-agent/src/main/java/com/palantir/timelock/paxos/PaxosLeadershipCreator.java b/timelock-agent/src/main/java/com/palantir/timelock/paxos/PaxosLeadershipCreator.java index 99def825160..142ba4710af 100644 --- a/timelock-agent/src/main/java/com/palantir/timelock/paxos/PaxosLeadershipCreator.java +++ b/timelock-agent/src/main/java/com/palantir/timelock/paxos/PaxosLeadershipCreator.java @@ -32,6 +32,7 @@ import com.palantir.atlasdb.timelock.paxos.PaxosTimeLockConstants; import com.palantir.atlasdb.timelock.paxos.PaxosTimeLockUriUtils; import com.palantir.atlasdb.util.MetricsManager; +import com.palantir.conjure.java.api.config.service.UserAgent; import com.palantir.leader.LeaderElectionService; import com.palantir.leader.LeadershipObserver; import com.palantir.leader.PingableLeader; @@ -78,7 +79,7 @@ void registerLeaderElectionService() { .remoteAcceptorUris(paxosSubresourceUris) .remoteLearnerUris(paxosSubresourceUris) .build(), - "leader-election-service"); + UserAgent.of(UserAgent.Agent.of("leader-election-service", UserAgent.Agent.DEFAULT_VERSION))); localPingableLeader = localPaxosServices.pingableLeader(); leaderElectionService = localPaxosServices.leaderElectionService(); leadershipObserver = localPaxosServices.leadershipObserver(); diff --git a/timelock-server-benchmark-client/src/main/java/com/palantir/atlasdb/timelock/benchmarks/runner/BenchmarkRunnerBase.java b/timelock-server-benchmark-client/src/main/java/com/palantir/atlasdb/timelock/benchmarks/runner/BenchmarkRunnerBase.java index 36d6c4252da..4ed3cd35d22 100644 --- a/timelock-server-benchmark-client/src/main/java/com/palantir/atlasdb/timelock/benchmarks/runner/BenchmarkRunnerBase.java +++ b/timelock-server-benchmark-client/src/main/java/com/palantir/atlasdb/timelock/benchmarks/runner/BenchmarkRunnerBase.java @@ -19,7 +19,6 @@ import java.io.IOException; import java.nio.charset.Charset; import java.util.Map; -import java.util.Optional; import java.util.function.BiFunction; import java.util.function.Supplier; @@ -28,14 +27,18 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.SerializationFeature; -import com.google.common.collect.ImmutableSet; import com.google.common.io.Files; +import com.palantir.atlasdb.config.AuxiliaryRemotingParameters; +import com.palantir.atlasdb.config.ImmutableServerListConfig; import com.palantir.atlasdb.http.AtlasDbFeignTargetFactory; import com.palantir.atlasdb.timelock.benchmarks.BenchmarksService; +import com.palantir.conjure.java.api.config.service.UserAgent; import com.palantir.logsafe.exceptions.SafeIllegalStateException; public class BenchmarkRunnerBase { + private static final UserAgent BENCHMARK_CLIENT_USER_AGENT = UserAgent.of( + UserAgent.Agent.of("benchmarks", UserAgent.Agent.DEFAULT_VERSION)); private static final String BENCHMARK_SERVER = readBenchmarkServerUri(); private static final int BENCHMARK_SERVER_PORT = 9425; @@ -64,12 +67,13 @@ protected void printResults(Map results) { protected static BenchmarksService createClient() { return AtlasDbFeignTargetFactory.DEFAULT.createProxyWithFailover( - Optional.empty(), - Optional.empty(), - ImmutableSet.of(BENCHMARK_SERVER), + ImmutableServerListConfig.builder().addServers(BENCHMARK_SERVER).build(), BenchmarksService.class, - "benchmarks", - false); + AuxiliaryRemotingParameters.builder() + .userAgent(BENCHMARK_CLIENT_USER_AGENT) + .shouldLimitPayload(false) + .shouldRetry(true) + .build()).instance(); } private static String readBenchmarkServerUri() { diff --git a/timelock-server/src/integTest/java/com/palantir/atlasdb/timelock/IsolatedPaxosTimeLockServerIntegrationTest.java b/timelock-server/src/integTest/java/com/palantir/atlasdb/timelock/IsolatedPaxosTimeLockServerIntegrationTest.java index dd86ebc2856..c68ad7e5b6e 100644 --- a/timelock-server/src/integTest/java/com/palantir/atlasdb/timelock/IsolatedPaxosTimeLockServerIntegrationTest.java +++ b/timelock-server/src/integTest/java/com/palantir/atlasdb/timelock/IsolatedPaxosTimeLockServerIntegrationTest.java @@ -25,6 +25,7 @@ import com.codahale.metrics.MetricRegistry; import com.palantir.atlasdb.http.AtlasDbHttpClients; +import com.palantir.atlasdb.http.TestProxyUtils; import com.palantir.atlasdb.timelock.paxos.PaxosTimeLockConstants; import com.palantir.atlasdb.timelock.util.ExceptionMatchers; import com.palantir.atlasdb.timelock.util.TestProxies; @@ -92,7 +93,8 @@ private static T createProxyForInternalNamespacedTestService(Class clazz) PaxosTimeLockConstants.INTERNAL_NAMESPACE, PaxosTimeLockConstants.CLIENT_PAXOS_NAMESPACE, CLIENT), - clazz); + clazz, + TestProxyUtils.AUXILIARY_REMOTING_PARAMETERS); } } diff --git a/timelock-server/src/integTest/java/com/palantir/atlasdb/timelock/PaxosTimeLockServerIntegrationTest.java b/timelock-server/src/integTest/java/com/palantir/atlasdb/timelock/PaxosTimeLockServerIntegrationTest.java index af9dadd2d66..b60134f0fd2 100644 --- a/timelock-server/src/integTest/java/com/palantir/atlasdb/timelock/PaxosTimeLockServerIntegrationTest.java +++ b/timelock-server/src/integTest/java/com/palantir/atlasdb/timelock/PaxosTimeLockServerIntegrationTest.java @@ -41,6 +41,7 @@ import org.junit.Test; import org.junit.rules.RuleChain; import org.junit.rules.TemporaryFolder; +import org.slf4j.LoggerFactory; import com.codahale.metrics.MetricRegistry; import com.fasterxml.jackson.databind.ObjectMapper; @@ -48,11 +49,14 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; +import com.palantir.atlasdb.config.AuxiliaryRemotingParameters; import com.palantir.atlasdb.http.AtlasDbHttpClients; import com.palantir.atlasdb.http.FeignOkHttpClients; +import com.palantir.atlasdb.http.TestProxyUtils; import com.palantir.atlasdb.http.errors.AtlasDbRemoteException; import com.palantir.atlasdb.timelock.config.CombinedTimeLockServerConfiguration; import com.palantir.atlasdb.timelock.util.TestProxies; +import com.palantir.conjure.java.api.config.service.UserAgents; import com.palantir.leader.PingableLeader; import com.palantir.lock.LockDescriptor; import com.palantir.lock.LockMode; @@ -96,7 +100,7 @@ public class PaxosTimeLockServerIntegrationTest { private static final int FORTY_TWO = 42; private static final String LOCK_CLIENT_NAME = "remoteLock-client-name"; - public static final LockDescriptor LOCK_1 = StringLockDescriptor.of("lock1"); + private static final LockDescriptor LOCK_1 = StringLockDescriptor.of("lock1"); private static final SortedMap LOCK_MAP = ImmutableSortedMap.of(LOCK_1, LockMode.WRITE); private static final File TIMELOCK_CONFIG_TEMPLATE = @@ -128,7 +132,8 @@ public static void waitForClusterToStabilize() { new MetricRegistry(), Optional.of(TestProxies.TRUST_CONTEXT), "https://localhost:" + TIMELOCK_SERVER_HOLDER.getTimelockPort(), - PingableLeader.class); + PingableLeader.class, + TestProxyUtils.AUXILIARY_REMOTING_PARAMETERS); Awaitility.await() .atMost(30, TimeUnit.SECONDS) .pollInterval(1, TimeUnit.SECONDS) @@ -140,6 +145,7 @@ public static void waitForClusterToStabilize() { CLIENTS.forEach(client -> getLockService(client).currentTimeMillis()); return leader.ping(); } catch (Throwable t) { + LoggerFactory.getLogger(PaxosTimeLockServerIntegrationTest.class).error("erreur!", t); return false; } }); @@ -387,10 +393,10 @@ public void fastForwardToThePastDoesNothing() { } @Test - public void throwsOnQueryingTimestampWithWithInvalidClientName() { + public void throwsOnQueryingTimestampWithInvalidClientName() { TimestampService invalidTimestampService = getTimestampService(INVALID_CLIENT); assertThatThrownBy(invalidTimestampService::getFreshTimestamp) - .hasMessageContaining("Unexpected char 0x08"); + .hasMessageContaining("NOT_FOUND"); } @Test @@ -460,8 +466,7 @@ private static T getProxyForRootService(String client, Class clazz) { Optional.of(TestProxies.TRUST_CONTEXT), getGenericRootUri(), clazz, - client, - true); + remotingParametersForClient(client)); } private static T getProxyForService(String client, Class clazz) { @@ -470,8 +475,15 @@ private static T getProxyForService(String client, Class clazz) { Optional.of(TestProxies.TRUST_CONTEXT), getRootUriForClient(client), clazz, - client, - true); + remotingParametersForClient(client)); + } + + private static AuxiliaryRemotingParameters remotingParametersForClient(String client) { + return AuxiliaryRemotingParameters.builder() + .shouldLimitPayload(true) + .userAgent(UserAgents.tryParse(client)) + .shouldRetry(true) + .build(); } private static String getGenericRootUri() { diff --git a/timelock-server/src/testCommon/java/com/palantir/atlasdb/timelock/util/TestProxies.java b/timelock-server/src/testCommon/java/com/palantir/atlasdb/timelock/util/TestProxies.java index b7334bc641d..1b4aea7b82f 100644 --- a/timelock-server/src/testCommon/java/com/palantir/atlasdb/timelock/util/TestProxies.java +++ b/timelock-server/src/testCommon/java/com/palantir/atlasdb/timelock/util/TestProxies.java @@ -24,8 +24,9 @@ import com.codahale.metrics.MetricRegistry; import com.google.common.collect.ImmutableList; import com.google.common.collect.Maps; +import com.palantir.atlasdb.config.ImmutableServerListConfig; import com.palantir.atlasdb.http.AtlasDbHttpClients; -import com.palantir.atlasdb.http.UserAgents; +import com.palantir.atlasdb.http.TestProxyUtils; import com.palantir.atlasdb.timelock.TestableTimelockServer; import com.palantir.atlasdb.timelock.TimeLockServerHolder; import com.palantir.conjure.java.api.config.ssl.SslConfiguration; @@ -34,8 +35,9 @@ public class TestProxies { - public static final TrustContext TRUST_CONTEXT = - SslSocketFactories.createTrustContext(SslConfiguration.of(Paths.get("var/security/trustStore.jks"))); + public static final SslConfiguration SSL_CONFIGURATION + = SslConfiguration.of(Paths.get("var/security/trustStore.jks")); + public static final TrustContext TRUST_CONTEXT = SslSocketFactories.createTrustContext(SSL_CONFIGURATION); private final String baseUri; private final List servers; @@ -63,8 +65,7 @@ public T singleNode(Class serviceInterface, String uri) { Optional.of(TRUST_CONTEXT), uri, serviceInterface, - "junit-test", - false)); + TestProxyUtils.AUXILIARY_REMOTING_PARAMETERS)); } public T failoverForClient(String client, Class serviceInterface) { @@ -75,11 +76,9 @@ public T failover(Class serviceInterface, List uris) { List key = ImmutableList.of(serviceInterface, uris, "failover"); return (T) proxies.computeIfAbsent(key, ignored -> AtlasDbHttpClients.createProxyWithFailover( new MetricRegistry(), - Optional.of(TRUST_CONTEXT), - Optional.empty(), - uris, - UserAgents.DEFAULT_USER_AGENT, - serviceInterface)); + ImmutableServerListConfig.builder().addAllServers(uris).sslConfiguration(SSL_CONFIGURATION).build(), + serviceInterface, + TestProxyUtils.AUXILIARY_REMOTING_PARAMETERS)); } public List getServerUris() {