Make GCP initialization truly lazy (#14077)

The GCP initialization pulls credentials for
talking to GCP.  We want that to only happen
when fully required and thus want the GCP-related
objects lazily instantiated.
This commit is contained in:
imply-cheddar 2023-04-13 15:10:50 +09:00 committed by GitHub
parent 81074411a9
commit d2f82f8dd6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 92 additions and 26 deletions

View File

@ -19,7 +19,6 @@
package org.apache.druid.common.gcp;
import com.google.api.client.googleapis.testing.auth.oauth2.MockGoogleCredential;
import com.google.api.client.http.HttpRequestInitializer;
import com.google.api.client.http.HttpTransport;
import com.google.api.client.json.JsonFactory;
@ -30,7 +29,7 @@ import com.google.inject.Provides;
import org.apache.druid.guice.LazySingleton;
import org.apache.druid.initialization.DruidModule;
public class GcpMockModule implements DruidModule
public abstract class GcpMockModule implements DruidModule
{
@Override
public void configure(Binder binder)
@ -39,14 +38,19 @@ public class GcpMockModule implements DruidModule
@Provides
@LazySingleton
public HttpRequestInitializer mockRequestInitializer(
public HttpRequestInitializer provideRequestInitializer(
HttpTransport transport,
JsonFactory factory
)
{
return new MockGoogleCredential.Builder().setTransport(transport).setJsonFactory(factory).build();
return mockRequestInitializer(transport, factory);
}
public abstract HttpRequestInitializer mockRequestInitializer(
HttpTransport transport,
JsonFactory factory
);
@Provides
@LazySingleton
public HttpTransport buildMockTransport()

View File

@ -22,12 +22,13 @@ package org.apache.druid.common.gcp;
import com.google.api.client.googleapis.testing.auth.oauth2.MockGoogleCredential;
import com.google.api.client.http.HttpRequestInitializer;
import com.google.api.client.http.HttpTransport;
import com.google.api.client.json.JsonFactory;
import com.google.api.client.testing.http.MockHttpTransport;
import com.google.inject.Binder;
import com.google.inject.Guice;
import com.google.inject.Injector;
import com.google.inject.Scopes;
import com.google.inject.util.Modules;
import org.apache.druid.guice.DruidScopes;
import org.apache.druid.guice.LazySingleton;
import org.junit.Assert;
import org.junit.Test;
@ -42,7 +43,16 @@ public class GcpModuleTest
@Override
public void configure(Binder binder)
{
binder.bindScope(LazySingleton.class, Scopes.SINGLETON);
binder.bindScope(LazySingleton.class, DruidScopes.SINGLETON);
}
@Override
public HttpRequestInitializer mockRequestInitializer(
HttpTransport transport,
JsonFactory factory
)
{
return new MockGoogleCredential.Builder().setTransport(transport).setJsonFactory(factory).build();
}
}));
Assert.assertTrue(injector.getInstance(HttpRequestInitializer.class) instanceof MockGoogleCredential);

View File

@ -27,9 +27,9 @@ import com.google.api.client.http.HttpRequestInitializer;
import com.google.api.client.http.HttpTransport;
import com.google.api.client.json.JsonFactory;
import com.google.api.services.storage.Storage;
import com.google.common.base.Suppliers;
import com.google.common.collect.ImmutableList;
import com.google.inject.Binder;
import com.google.inject.Provider;
import com.google.inject.Provides;
import com.google.inject.multibindings.MapBinder;
import org.apache.druid.data.SearchableVersionedDataFinder;
@ -97,9 +97,23 @@ public class GoogleStorageDruidModule implements DruidModule
JsonConfigProvider.bind(binder, "druid.indexer.logs", GoogleTaskLogsConfig.class);
binder.bind(GoogleTaskLogs.class).in(LazySingleton.class);
MapBinder.newMapBinder(binder, String.class, SearchableVersionedDataFinder.class)
.addBinding(SCHEME_GS)
.to(GoogleTimestampVersionedDataFinder.class)
.in(LazySingleton.class);
.addBinding(SCHEME_GS)
.to(GoogleTimestampVersionedDataFinder.class)
.in(LazySingleton.class);
}
@Provides
@LazySingleton
public Storage getGcpStorage(
HttpTransport httpTransport,
JsonFactory jsonFactory,
HttpRequestInitializer requestInitializer
)
{
return new Storage
.Builder(httpTransport, jsonFactory, requestInitializer)
.setApplicationName(APPLICATION_NAME)
.build();
}
/**
@ -109,20 +123,10 @@ public class GoogleStorageDruidModule implements DruidModule
@Provides
@LazySingleton
public GoogleStorage getGoogleStorage(
HttpTransport httpTransport,
JsonFactory jsonFactory,
HttpRequestInitializer requestInitializer
Provider<Storage> baseStorageProvider
)
{
LOG.info("Building Cloud Storage Client...");
return new GoogleStorage(
Suppliers.memoize(
() -> new Storage
.Builder(httpTransport, jsonFactory, requestInitializer)
.setApplicationName(APPLICATION_NAME)
.build()
)
);
return new GoogleStorage(baseStorageProvider::get);
}
}

View File

@ -19,6 +19,11 @@
package org.apache.druid.storage.google;
import com.google.api.client.googleapis.testing.auth.oauth2.MockGoogleCredential;
import com.google.api.client.http.HttpRequestInitializer;
import com.google.api.client.http.HttpTransport;
import com.google.api.client.json.JsonFactory;
import com.google.api.services.storage.Storage;
import com.google.common.collect.ImmutableList;
import com.google.inject.Injector;
import org.apache.druid.common.gcp.GcpMockModule;
@ -32,22 +37,65 @@ public class GoogleStorageDruidModuleTest
@Test
public void testSegmentKillerBoundedSingleton()
{
Injector injector = createInjector();
// This test is primarily validating 2 things
// 1. That the Google credentials are loaded lazily, they are loaded as part of instantiation of the
// HttpRquestInitializer, the test throws an exception from that method, meaning that if they are not loaded
// lazily, the exception should end up thrown.
// 2. That the same object is returned.
Injector injector = GuiceInjectors.makeStartupInjectorWithModules(
ImmutableList.of(
new GcpMockModule()
{
@Override
public HttpRequestInitializer mockRequestInitializer(
HttpTransport transport,
JsonFactory factory
)
{
return new MockGoogleCredential.Builder().setTransport(transport).setJsonFactory(factory).build();
}
},
new GoogleStorageDruidModule()
)
);
OmniDataSegmentKiller killer = injector.getInstance(OmniDataSegmentKiller.class);
Assert.assertTrue(killer.getKillers().containsKey(GoogleStorageDruidModule.SCHEME));
Assert.assertSame(
killer.getKillers().get(GoogleStorageDruidModule.SCHEME).get(),
killer.getKillers().get(GoogleStorageDruidModule.SCHEME).get()
);
final Storage storage = injector.getInstance(Storage.class);
Assert.assertSame(storage, injector.getInstance(Storage.class));
}
private static Injector createInjector()
@Test
public void testLazyInstantiation()
{
return GuiceInjectors.makeStartupInjectorWithModules(
// This test is primarily validating 2 things
// 1. That the Google credentials are loaded lazily, they are loaded as part of instantiation of the
// HttpRquestInitializer, the test throws an exception from that method, meaning that if they are not loaded
// lazily, the exception should end up thrown.
// 2. That the same object is returned.
Injector injector = GuiceInjectors.makeStartupInjectorWithModules(
ImmutableList.of(
new GcpMockModule(),
new GcpMockModule()
{
@Override
public HttpRequestInitializer mockRequestInitializer(
HttpTransport transport,
JsonFactory factory
)
{
throw new UnsupportedOperationException("should not be called, because this should be lazy");
}
},
new GoogleStorageDruidModule()
)
);
final GoogleStorage instance = injector.getInstance(GoogleStorage.class);
Assert.assertSame(instance, injector.getInstance(GoogleStorage.class));
}
}