-
Notifications
You must be signed in to change notification settings - Fork 558
Indis based d2 warmup for restli client #1124
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
Changes from all commits
81f97f0
7186ea7
ee1bb31
d8a4494
6cc1640
fddcd3b
12f6e8f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| package com.linkedin.d2.balancer.util; | ||
|
|
||
| import javax.annotation.Nonnull; | ||
|
|
||
|
|
||
| /** | ||
| * Records the callee service information and periodically sends out the recorded information to a persistence medium. | ||
| */ | ||
| public interface D2CalleeInfoRecorder { | ||
| /** | ||
| * Records a callee service name. | ||
| * @param serviceName the callee service name to record | ||
| */ | ||
| void record(String serviceName); | ||
|
|
||
| @Nonnull | ||
| String getAppName(); | ||
|
|
||
| @Nonnull | ||
| String getAppInstanceID(); | ||
|
|
||
| @Nonnull | ||
| String getScope(); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,7 @@ | |
|
|
||
| import com.google.common.annotations.VisibleForTesting; | ||
| import com.linkedin.common.callback.Callback; | ||
| import com.linkedin.common.callback.SuccessCallback; | ||
| import com.linkedin.common.util.None; | ||
| import com.linkedin.d2.balancer.LoadBalancerWithFacilities; | ||
| import com.linkedin.d2.balancer.LoadBalancerWithFacilitiesDelegator; | ||
|
|
@@ -49,9 +50,13 @@ | |
| import java.util.function.Supplier; | ||
| import java.util.stream.Collectors; | ||
| import java.util.stream.IntStream; | ||
| import javax.annotation.Nullable; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| import static com.linkedin.d2.balancer.simple.SimpleLoadBalancer.*; | ||
|
|
||
|
|
||
| /** | ||
| * The WarmUpLoadBalancer warms up the internal {@link SimpleLoadBalancer} services/cluster list | ||
| * before the client is announced as "started". | ||
|
|
@@ -79,6 +84,7 @@ public class WarmUpLoadBalancer extends LoadBalancerWithFacilitiesDelegator { | |
| private final DualReadStateManager _dualReadStateManager; | ||
| private final boolean _isIndis; // whether warming up for Indis (false means warming up for ZK) | ||
| private final String _printName; // name of this warmup load balancer based on it's indis or not. | ||
| @Nullable private final D2CalleeInfoRecorder _d2CalleeInfoRecorder; | ||
| private volatile boolean _shuttingDown = false; | ||
| private long _allStartTime; | ||
| private List<String> _servicesToWarmUp = null; | ||
|
|
@@ -103,14 +109,32 @@ public WarmUpLoadBalancer(LoadBalancerWithFacilities balancer, WarmUpService ser | |
| DownstreamServicesFetcher downstreamServicesFetcher, int warmUpTimeoutSeconds, int concurrentRequests, | ||
| DualReadStateManager dualReadStateManager, boolean isIndis) { | ||
| this(balancer, serviceWarmupper, executorService, d2FsDirPath, d2ServicePath, downstreamServicesFetcher, | ||
| warmUpTimeoutSeconds * 1000, concurrentRequests, dualReadStateManager, isIndis, null); | ||
| warmUpTimeoutSeconds, concurrentRequests, dualReadStateManager, isIndis, (D2CalleeInfoRecorder) null); | ||
| } | ||
|
|
||
| @VisibleForTesting | ||
| public WarmUpLoadBalancer(LoadBalancerWithFacilities balancer, WarmUpService serviceWarmupper, | ||
| ScheduledExecutorService executorService, String d2FsDirPath, String d2ServicePath, | ||
| DownstreamServicesFetcher downstreamServicesFetcher, int warmUpTimeoutSeconds, int concurrentRequests, | ||
| DualReadStateManager dualReadStateManager, boolean isIndis, D2CalleeInfoRecorder d2CalleeInfoRecorder) { | ||
| this(balancer, serviceWarmupper, executorService, d2FsDirPath, d2ServicePath, downstreamServicesFetcher, | ||
| warmUpTimeoutSeconds * 1000, concurrentRequests, dualReadStateManager, isIndis, null, | ||
| d2CalleeInfoRecorder); | ||
| } | ||
|
|
||
| @VisibleForTesting | ||
| WarmUpLoadBalancer(LoadBalancerWithFacilities balancer, WarmUpService serviceWarmupper, | ||
| ScheduledExecutorService executorService, String d2FsDirPath, String d2ServicePath, | ||
| DownstreamServicesFetcher downstreamServicesFetcher, int warmUpTimeoutMillis, int concurrentRequests, | ||
| DualReadStateManager dualReadStateManager, boolean isIndis, Supplier<Long> timeSupplierForTest) | ||
| DualReadStateManager dualReadStateManager, boolean isIndis, Supplier<Long> timeSupplierForTest) { | ||
| this(balancer, serviceWarmupper, executorService, d2FsDirPath, d2ServicePath, downstreamServicesFetcher, | ||
| warmUpTimeoutMillis, concurrentRequests, dualReadStateManager, isIndis, timeSupplierForTest, null); | ||
| } | ||
|
|
||
| private WarmUpLoadBalancer(LoadBalancerWithFacilities balancer, WarmUpService serviceWarmupper, | ||
| ScheduledExecutorService executorService, String d2FsDirPath, String d2ServicePath, | ||
| DownstreamServicesFetcher downstreamServicesFetcher, int warmUpTimeoutMillis, int concurrentRequests, | ||
| DualReadStateManager dualReadStateManager, boolean isIndis, Supplier<Long> timeSupplierForTest, | ||
| @Nullable D2CalleeInfoRecorder d2CalleeInfoRecorder) | ||
| { | ||
| super(balancer); | ||
| _serviceWarmupper = serviceWarmupper; | ||
|
|
@@ -129,6 +153,7 @@ public WarmUpLoadBalancer(LoadBalancerWithFacilities balancer, WarmUpService ser | |
| { | ||
| _timeSupplier = timeSupplierForTest; | ||
| } | ||
| _d2CalleeInfoRecorder = d2CalleeInfoRecorder; | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -176,7 +201,7 @@ private void prepareWarmUp(Callback<None> callback) | |
| final AtomicBoolean hasTimedOut = new AtomicBoolean(false); | ||
|
|
||
| try { | ||
| _downstreamServicesFetcher.getServiceNames(serviceNames -> { | ||
| SuccessCallback<List<String>> serviceNamesCallback = serviceNames -> { | ||
| // The downstreamServicesFetcher is the core group of the services that will be used during the lifecycle | ||
| _usedServices.addAll(serviceNames); | ||
|
|
||
|
|
@@ -240,7 +265,13 @@ public void onSuccess(ServiceProperties result) { | |
| { | ||
| callback.onSuccess(None.none()); | ||
| } | ||
| }); | ||
| }; | ||
| if (_d2CalleeInfoRecorder != null) { | ||
| _downstreamServicesFetcher.getServiceNames(_d2CalleeInfoRecorder.getAppName(), | ||
| _d2CalleeInfoRecorder.getAppInstanceID(), _d2CalleeInfoRecorder.getScope(), serviceNamesCallback); | ||
| } else { | ||
| _downstreamServicesFetcher.getServiceNames(serviceNamesCallback); | ||
| } | ||
| } | ||
| catch (Exception e) | ||
| { | ||
|
|
@@ -446,6 +477,10 @@ public TransportClient getClient(Request request, RequestContext requestContext) | |
| // the call fails, we still *intend* to use serviceName, so it should be in _usedServices. | ||
| String serviceName = LoadBalancerUtil.getServiceNameFromUri(request.getURI()); | ||
| _usedServices.add(serviceName); | ||
| if (_d2CalleeInfoRecorder != null && D2_SCHEME_NAME.equalsIgnoreCase(request.getURI().getScheme())) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm curious, what is the reason for checking the scheme?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if this decorator LB is only ever used for d2:// requests. Currently, the recording of warmup service names doesn't happen here. Those are fetched from the d2 data stored on disk, which happens asynchronously via the property buses. I just wanna be sure that we don't record any unneeded information here. |
||
| { | ||
| _d2CalleeInfoRecorder.record(serviceName); | ||
| } | ||
| return _loadBalancer.getClient(request, requestContext); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This interface should've been added here and re-used in grpc infra. However, now I'll add an adapter internally to re-use that one for its implementation.