remove getLoadStatsStore method from LocalityStore

Currently `LoadStatsStore` is create by `LocalityStore` and `LoadReportClient` retrieves `LoadStatsStore` from `LocalityStore.getLoadStatsStore()`.

But `LocalityStore` is create by EDS policy, whereas `LoadReportClient` and `LoadStatsStore` should be created by CDS (if not EDS-only), before `LocalityStore` is created. If `LoadReportClient` is embedded in `XdsClientImpl`, it need a `LoadStatsStore` which shouldn't be created by `LocalityStore`.

Instead, `LoadStatsStore` should be create before `LocalityStore` is created, and be passed to `LocalityStore`'s constructor. A getter is not needed.
This commit is contained in:
ZHANG Dapeng 2019-11-21 12:37:48 -08:00 committed by GitHub
parent eaf99cf7fe
commit 81efecd86a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 14 additions and 17 deletions

View File

@ -76,15 +76,14 @@ interface LocalityStore {
void updateOobMetricsReportInterval(long reportIntervalNano);
LoadStatsStore getLoadStatsStore();
@VisibleForTesting
abstract class LocalityStoreFactory {
private static final LocalityStoreFactory DEFAULT_INSTANCE =
new LocalityStoreFactory() {
@Override
LocalityStore newLocalityStore(Helper helper, LoadBalancerRegistry lbRegistry) {
return new LocalityStoreImpl(helper, lbRegistry);
LocalityStore newLocalityStore(
Helper helper, LoadBalancerRegistry lbRegistry, LoadStatsStore loadStatsStore) {
return new LocalityStoreImpl(helper, lbRegistry, loadStatsStore);
}
};
@ -92,7 +91,8 @@ interface LocalityStore {
return DEFAULT_INSTANCE;
}
abstract LocalityStore newLocalityStore(Helper helper, LoadBalancerRegistry lbRegistry);
abstract LocalityStore newLocalityStore(
Helper helper, LoadBalancerRegistry lbRegistry, LoadStatsStore loadStatsStore);
}
final class LocalityStoreImpl implements LocalityStore {
@ -113,9 +113,10 @@ interface LocalityStore {
private List<DropOverload> dropOverloads = ImmutableList.of();
private long metricsReportIntervalNano = -1;
LocalityStoreImpl(Helper helper, LoadBalancerRegistry lbRegistry) {
LocalityStoreImpl(
Helper helper, LoadBalancerRegistry lbRegistry, LoadStatsStore loadStatsStore) {
this(helper, pickerFactoryImpl, lbRegistry, ThreadSafeRandom.ThreadSafeRandomImpl.instance,
new LoadStatsStoreImpl(), OrcaPerRequestUtil.getInstance(), OrcaOobUtil.getInstance());
loadStatsStore, OrcaPerRequestUtil.getInstance(), OrcaOobUtil.getInstance());
}
@VisibleForTesting
@ -284,11 +285,6 @@ interface LocalityStore {
TimeUnit.MINUTES, helper.getScheduledExecutorService());
}
@Override
public LoadStatsStore getLoadStatsStore() {
return loadStatsStore;
}
@Override
public void updateOobMetricsReportInterval(long reportIntervalNano) {
metricsReportIntervalNano = reportIntervalNano;

View File

@ -175,11 +175,13 @@ final class LookasideLb extends ForwardingLoadBalancer {
xdsClient = new XdsComms2(
channel, helper, new ExponentialBackoffPolicy.Provider(),
GrpcUtil.STOPWATCH_SUPPLIER, node);
localityStore = localityStoreFactory.newLocalityStore(helper, lbRegistry);
LoadStatsStore loadStatsStore = new LoadStatsStoreImpl();
localityStore = localityStoreFactory.newLocalityStore(
helper, lbRegistry, loadStatsStore);
// TODO(zdapeng): Use XdsClient to do Lrs directly.
lrsClient = loadReportClientFactory.createLoadReportClient(
channel, helper, new ExponentialBackoffPolicy.Provider(),
localityStore.getLoadStatsStore());
loadStatsStore);
final LoadReportCallback lrsCallback =
new LoadReportCallback() {
@Override

View File

@ -194,11 +194,10 @@ public class LookasideLbTest {
localityStoreFactory = new LocalityStoreFactory() {
@Override
public LocalityStore newLocalityStore(Helper helper, LoadBalancerRegistry lbRegistry) {
public LocalityStore newLocalityStore(
Helper helper, LoadBalancerRegistry lbRegistry, LoadStatsStore loadStatsStore) {
helpers.add(helper);
LocalityStore localityStore = mock(LocalityStore.class);
LoadStatsStore loadStatsStore = mock(LoadStatsStore.class);
doReturn(loadStatsStore).when(localityStore).getLoadStatsStore();
localityStores.add(localityStore);
return localityStore;
}