diff --git a/gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GoogleCloudStorageEventSubscriber.java b/gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GoogleCloudStorageEventSubscriber.java index 3294553870..4522d1a59b 100644 --- a/gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GoogleCloudStorageEventSubscriber.java +++ b/gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GoogleCloudStorageEventSubscriber.java @@ -18,19 +18,40 @@ import com.google.api.client.googleapis.json.GoogleJsonResponseException; import com.google.cloud.hadoop.util.GcsRequestExecutionEvent; +import com.google.common.annotations.VisibleForTesting; import com.google.common.eventbus.Subscribe; +import com.google.common.flogger.GoogleLogger; import io.grpc.Status; import java.io.IOException; import javax.annotation.Nonnull; /* Stores the subscriber methods corresponding to GoogleCloudStorageEventBus */ public class GoogleCloudStorageEventSubscriber { + private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); private static GhfsStorageStatistics storageStatistics; + private static GoogleCloudStorageEventSubscriber INSTANCE = null; - public GoogleCloudStorageEventSubscriber(GhfsStorageStatistics storageStatistics) { + private GoogleCloudStorageEventSubscriber(@Nonnull GhfsStorageStatistics storageStatistics) { this.storageStatistics = storageStatistics; } + /* + * Singleton class such that registration of subscriber methods is only once. + * */ + public static synchronized GoogleCloudStorageEventSubscriber getInstance( + @Nonnull GhfsStorageStatistics storageStatistics) { + if (INSTANCE == null) { + logger.atFiner().log("Subscriber class invoked for first time"); + INSTANCE = new GoogleCloudStorageEventSubscriber(storageStatistics); + } + return INSTANCE; + } + + @VisibleForTesting + protected static void reset() { + INSTANCE = null; + } + /** * Updating the required gcs specific statistics based on GoogleJsonResponseException. * diff --git a/gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFileSystemBase.java b/gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFileSystemBase.java index 9813db26a0..d0ee3bbf6c 100644 --- a/gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFileSystemBase.java +++ b/gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFileSystemBase.java @@ -361,7 +361,8 @@ public GoogleHadoopFileSystemBase() { storageStatistics = GhfsStorageStatistics.DUMMY_INSTANCE; } - GoogleCloudStorageEventBus.register(new GoogleCloudStorageEventSubscriber(storageStatistics)); + GoogleCloudStorageEventBus.register( + GoogleCloudStorageEventSubscriber.getInstance(storageStatistics)); } /** diff --git a/gcs/src/test/java/com/google/cloud/hadoop/fs/gcs/GoogleCloudStorageStatisticsTest.java b/gcs/src/test/java/com/google/cloud/hadoop/fs/gcs/GoogleCloudStorageStatisticsTest.java index 3098a90ead..9cf66e2643 100644 --- a/gcs/src/test/java/com/google/cloud/hadoop/fs/gcs/GoogleCloudStorageStatisticsTest.java +++ b/gcs/src/test/java/com/google/cloud/hadoop/fs/gcs/GoogleCloudStorageStatisticsTest.java @@ -50,7 +50,7 @@ public class GoogleCloudStorageStatisticsTest { private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); private GhfsStorageStatistics storageStatistics = new GhfsStorageStatistics(); protected GoogleCloudStorageEventSubscriber subscriber = - new GoogleCloudStorageEventSubscriber(storageStatistics); + GoogleCloudStorageEventSubscriber.getInstance(storageStatistics); @Before public void setUp() throws Exception { @@ -60,6 +60,7 @@ public void setUp() throws Exception { @After public void cleanup() throws Exception { GoogleCloudStorageEventBus.unregister(subscriber); + GoogleCloudStorageEventSubscriber.reset(); } private void verifyStatistics(GhfsStorageStatistics expectedStats) { @@ -79,6 +80,21 @@ private void verifyStatistics(GhfsStorageStatistics expectedStats) { assertThat(metricsVerified).isTrue(); } + @Test + public void test_multiple_register_of_statistics() throws Exception { + GoogleCloudStorageEventBus.register(subscriber); + GoogleCloudStorageEventBus.register(subscriber); + GoogleCloudStorageEventBus.register( + GoogleCloudStorageEventSubscriber.getInstance(storageStatistics)); + GoogleCloudStorageEventBus.register( + GoogleCloudStorageEventSubscriber.getInstance(storageStatistics)); + + GoogleCloudStorageEventBus.onGcsRequest(new GcsRequestExecutionEvent()); + GhfsStorageStatistics verifyCounterStats = new GhfsStorageStatistics(); + verifyCounterStats.incrementCounter(GCS_API_REQUEST_COUNT, 1); + verifyStatistics(verifyCounterStats); + } + @Test public void gcs_requestCounter() throws Exception { GoogleCloudStorageEventBus.onGcsRequest(new GcsRequestExecutionEvent()); diff --git a/gcs/src/test/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFileSystemIntegrationTest.java b/gcs/src/test/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFileSystemIntegrationTest.java index 68e4ca1c1a..7814f0ecee 100644 --- a/gcs/src/test/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFileSystemIntegrationTest.java +++ b/gcs/src/test/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFileSystemIntegrationTest.java @@ -58,6 +58,7 @@ import com.google.cloud.hadoop.gcsio.StatisticTypeEnum; import com.google.cloud.hadoop.gcsio.StorageResourceId; import com.google.cloud.hadoop.gcsio.testing.InMemoryGoogleCloudStorage; +import com.google.cloud.hadoop.util.GoogleCloudStorageEventBus; import com.google.common.collect.ImmutableList; import com.google.common.hash.Hashing; import com.google.common.primitives.Ints; @@ -1270,6 +1271,30 @@ public void testInitializeCompatibleWithHadoopCredentialProvider() throws Except // Initialization successful with no exception thrown. } + @Test + public void register_subscriber_multiple_time() throws Exception { + GoogleHadoopFileSystem myGhfs = + createInMemoryGoogleHadoopFileSystem(); // registers the subscriber class first time in + // myGhfs1 + StorageStatistics stats = TestUtils.getStorageStatistics(); + + GoogleCloudStorageEventBus.register( + GoogleCloudStorageEventSubscriber.getInstance( + (GhfsStorageStatistics) stats)); // registers the same subscriber class second time + + assertThat(getMetricValue(stats, INVOCATION_CREATE)).isEqualTo(0); + assertThat(getMetricValue(stats, FILES_CREATED)).isEqualTo(0); + + try (FSDataOutputStream fout = myGhfs.create(new Path("/file1"))) { + fout.writeBytes("Test Content"); + } + assertThat(getMetricValue(stats, INVOCATION_CREATE)).isEqualTo(1); + assertThat(getMetricValue(stats, FILES_CREATED)).isEqualTo(1); + assertThat(myGhfs.delete(new Path("/file1"))).isTrue(); + + TestUtils.verifyDurationMetric((GhfsStorageStatistics) stats, INVOCATION_CREATE, 1); + } + @Test public void create_IOstatistics() throws IOException { GoogleHadoopFileSystem myGhfs = createInMemoryGoogleHadoopFileSystem();