Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deletes code no longer needed regarding state checking of ES hosts #2800

Merged
merged 5 commits into from
Sep 13, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,11 @@

import com.linecorp.armeria.client.Endpoint;
import com.linecorp.armeria.client.endpoint.EndpointGroup;
import com.linecorp.armeria.client.endpoint.dns.DnsAddressEndpointGroup;
import com.linecorp.armeria.client.endpoint.dns.DnsAddressEndpointGroupBuilder;
import com.linecorp.armeria.common.SessionProtocol;
import java.net.URI;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.TimeUnit;
import java.util.function.Supplier;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
Expand Down Expand Up @@ -64,25 +62,13 @@ final class InitialEndpointSupplier implements Supplier<EndpointGroup> {
// A host that isn't an IP may resolve to multiple IP addresses, so we use a endpoint
// group to round-robin over them. Users can mix addresses that resolve to multiple IPs
// with single IPs freely, they'll all get used.
endpointGroups.add(resolveDnsAddresses(host, port));
endpointGroups.add(new DnsAddressEndpointGroupBuilder(host).port(port).build());
}
}

return EndpointGroup.of(endpointGroups);
}

// Rather than result in an empty group. Await DNS resolution as this call is deferred anyway
// TODO: delete the await when https://github.com/line/armeria/issues/2071 is complete
DnsAddressEndpointGroup resolveDnsAddresses(String host, int port) {
DnsAddressEndpointGroup result = new DnsAddressEndpointGroupBuilder(host).port(port).build();
try {
result.awaitInitialEndpoints(1, TimeUnit.SECONDS);
} catch (Exception e) {
// let it fail later
}
return result;
}

int getPort(URI url) {
int port = url.getPort();
if (port == -1) port = sessionProtocol.defaultPort();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

import com.linecorp.armeria.client.Endpoint;
import com.linecorp.armeria.client.HttpClient;
import com.linecorp.armeria.client.endpoint.DynamicEndpointGroup;
import com.linecorp.armeria.client.endpoint.EndpointGroup;
import com.linecorp.armeria.client.endpoint.EndpointGroupRegistry;
import com.linecorp.armeria.client.endpoint.healthcheck.HealthCheckedEndpointGroup;
Expand Down Expand Up @@ -71,30 +70,30 @@ final class LazyHttpClientImpl implements LazyHttpClient {
}

Endpoint getEndpoint() {
EndpointGroup endpointGroup = initialEndpoints.get();
if (endpointGroup instanceof Endpoint) return (Endpoint) endpointGroup;
EndpointGroup rawEndpointGroup = initialEndpoints.get();
// Only health-check when there are alternative endpoints. There aren't when instanceof Endpoint
if (rawEndpointGroup instanceof Endpoint) return (Endpoint) rawEndpointGroup;

// https://github.com/line/armeria/issues/2071 Composite endpoint groups don't extend dynamic
// once this is done, we can delete special-cased code in InitialEndpointSupplier about DNS, as
// when endpointGroup.isStatic() is false, we'd here.
if (endpointGroup instanceof DynamicEndpointGroup) {
try {
// Since we aren't holding up server startup, or sitting on the event loop, it is ok to
// block. The alternative is round-robin, which could be unlucky and hit a bad node first.
//
// We are blocking up to the connection timeout which should be enough time for any DNS
// resolution that hasn't happened yet to finish.
endpointGroup.awaitInitialEndpoints(timeoutMillis, TimeUnit.MILLISECONDS);
} catch (Exception e) {
// We'll try again next time around.
throw new IllegalStateException("couldn't connect any of " + endpointGroup.endpoints(), e);
}
}
EndpointGroup endpointGroup = rawEndpointGroup;
if (healthCheck.isEnabled()) endpointGroup = decorateHealthCheck(rawEndpointGroup);

if (healthCheck.isEnabled()) endpointGroup = decorateHealthCheck(endpointGroup);
try {
// Since we aren't holding up server startup, or sitting on the event loop, it is ok to
// block. The alternative is round-robin, which could be unlucky and hit a bad node first.
//
// We are blocking up to the connection timeout which should be enough time for any DNS
// resolution that hasn't happened yet to finish.
//
// TODO: When using health checks, if all hosts fail the health check, this will throw TOE
// See https://github.com/line/armeria/issues/2075
endpointGroup.awaitInitialEndpoints(timeoutMillis, TimeUnit.MILLISECONDS);
} catch (Exception e) {
// We'll try again next time around.
codefromthecrypt marked this conversation as resolved.
Show resolved Hide resolved
throw new IllegalStateException("couldn't connect any of " + rawEndpointGroup.endpoints(), e);
}

// TODO: why must we do this instead of using direct type references.
// The static factory is concerning. https://github.com/line/armeria/issues/1084
// Currently, there's no alternative to static registries when using Endpoint.ofGroup
// https://github.com/line/armeria/issues/1084
EndpointGroupRegistry.register("elasticsearch", endpointGroup, ROUND_ROBIN);
return Endpoint.ofGroup("elasticsearch");
}
Expand All @@ -120,18 +119,6 @@ HealthCheckedEndpointGroup decorateHealthCheck(EndpointGroup endpointGroup) {
.retryInterval(healthCheck.getInterval())
.build();
healthChecked.newMeterBinder("elasticsearch").bindTo(meterRegistry);

// Since we aren't holding up server startup, or sitting on the event loop, it is ok to block.
// The alternative is round-robin, which could be unlucky and hit a bad node first.
//
// We are blocking up to the connection timeout which should be enough time for a health check
// to respond.
try {
healthChecked.awaitInitialEndpoints(timeoutMillis, TimeUnit.MILLISECONDS);
} catch (Exception e) {
healthChecked.close(); // we'll recreate it the next time around.
throw new IllegalStateException("couldn't connect any of " + endpointGroup.endpoints(), e);
}
return healthChecked;
}

Expand Down