You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by cw...@apache.org on 2020/04/01 18:57:50 UTC

[druid] branch master updated: Allow Cloud Deep Storage configs without segment bucket or path specified (#9588)

This is an automated email from the ASF dual-hosted git repository.

cwylie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git


The following commit(s) were added to refs/heads/master by this push:
     new e855c7f  Allow Cloud Deep Storage configs without segment bucket or path specified (#9588)
e855c7f is described below

commit e855c7fe1bd481cdc79d38343563db661a1ccc43
Author: zachjsh <za...@gmail.com>
AuthorDate: Wed Apr 1 11:57:32 2020 -0700

    Allow Cloud Deep Storage configs without segment bucket or path specified (#9588)
    
    * Allow Cloud SegmentKillers to be instantiated without segment bucket or path
    
    This change fixes a bug that was introduced that causes ingestion
    to fail if data is ingested from one of the supported cloud storages
    (Azure, Google, S3), and the user is using another type of storage
    for deep storage. In this case the all segment killer implementations
    are instantiated. A change recently made forced a dependency between
    the supported cloud storage type SegmentKiller classes and the
    deep storage configuration for that storage type being set, which
    forced the deep storage bucket and prefix to be non-null. This caused
    a NullPointerException to be thrown when instantiating the
    SegmentKiller classes during ingestion.
    
    To fix this issue, the respective deep storage segment configs for the
    cloud storage types supported in druid are now allowed to have nullable
    bucket and prefix configurations
    
    * * Allow google deep storage bucket to be null
---
 extensions-core/azure-extensions/pom.xml           |  2 +-
 .../storage/azure/AzureDataSegmentConfig.java      |  4 ----
 .../storage/azure/AzureDataSegmentKiller.java      |  6 ++++-
 .../storage/azure/AzureDataSegmentKillerTest.java  | 28 ++++++++++++++++++++++
 .../druid/storage/google/GoogleAccountConfig.java  |  3 ---
 .../storage/google/GoogleDataSegmentKiller.java    |  5 ++++
 .../google/GoogleDataSegmentKillerTest.java        | 24 +++++++++++++++++++
 .../druid/storage/s3/S3DataSegmentKiller.java      |  5 ++++
 .../druid/storage/s3/S3DataSegmentKillerTest.java  | 25 +++++++++++++++++++
 9 files changed, 93 insertions(+), 9 deletions(-)

diff --git a/extensions-core/azure-extensions/pom.xml b/extensions-core/azure-extensions/pom.xml
index 7fbf8fb..acb5bb9 100644
--- a/extensions-core/azure-extensions/pom.xml
+++ b/extensions-core/azure-extensions/pom.xml
@@ -190,7 +190,7 @@
                                 <limit>
                                     <counter>BRANCH</counter>
                                     <value>COVEREDRATIO</value>
-                                    <minimum>0.89</minimum>
+                                    <minimum>0.88</minimum>
                                 </limit>
                                 <limit>
                                     <counter>COMPLEXITY</counter>
diff --git a/extensions-core/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureDataSegmentConfig.java b/extensions-core/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureDataSegmentConfig.java
index e84658c..1272e24 100644
--- a/extensions-core/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureDataSegmentConfig.java
+++ b/extensions-core/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureDataSegmentConfig.java
@@ -21,19 +21,15 @@ package org.apache.druid.storage.azure;
 
 import com.fasterxml.jackson.annotation.JsonProperty;
 
-import javax.validation.constraints.NotNull;
-
 /**
  * Stores the configuration for segments written to Azure deep storage
  */
 public class AzureDataSegmentConfig
 {
   @JsonProperty
-  @NotNull
   private String container;
 
   @JsonProperty
-  @NotNull
   private String prefix = "";
 
   public void setContainer(String container)
diff --git a/extensions-core/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureDataSegmentKiller.java b/extensions-core/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureDataSegmentKiller.java
index e4cfc35..bdfdb99 100644
--- a/extensions-core/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureDataSegmentKiller.java
+++ b/extensions-core/azure-extensions/src/main/java/org/apache/druid/storage/azure/AzureDataSegmentKiller.java
@@ -22,6 +22,7 @@ package org.apache.druid.storage.azure;
 import com.google.common.base.Predicates;
 import com.google.inject.Inject;
 import com.microsoft.azure.storage.StorageException;
+import org.apache.druid.java.util.common.ISE;
 import org.apache.druid.java.util.common.MapUtils;
 import org.apache.druid.java.util.common.logger.Logger;
 import org.apache.druid.segment.loading.DataSegmentKiller;
@@ -88,6 +89,10 @@ public class AzureDataSegmentKiller implements DataSegmentKiller
   @Override
   public void killAll() throws IOException
   {
+    if (segmentConfig.getContainer() == null || segmentConfig.getPrefix() == null) {
+      throw new ISE(
+          "Cannot delete all segment files since Azure Deep Storage since druid.azure.container and druid.azure.prefix are not both set.");
+    }
     log.info(
         "Deleting all segment files from Azure storage location [bucket: '%s' prefix: '%s']",
         segmentConfig.getContainer(),
@@ -109,5 +114,4 @@ public class AzureDataSegmentKiller implements DataSegmentKiller
       throw new IOException(e);
     }
   }
-
 }
diff --git a/extensions-core/azure-extensions/src/test/java/org/apache/druid/storage/azure/AzureDataSegmentKillerTest.java b/extensions-core/azure-extensions/src/test/java/org/apache/druid/storage/azure/AzureDataSegmentKillerTest.java
index fc78b2d..e031015 100644
--- a/extensions-core/azure-extensions/src/test/java/org/apache/druid/storage/azure/AzureDataSegmentKillerTest.java
+++ b/extensions-core/azure-extensions/src/test/java/org/apache/druid/storage/azure/AzureDataSegmentKillerTest.java
@@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.microsoft.azure.storage.StorageException;
 import com.microsoft.azure.storage.StorageExtendedErrorInformation;
+import org.apache.druid.java.util.common.ISE;
 import org.apache.druid.java.util.common.Intervals;
 import org.apache.druid.java.util.common.StringUtils;
 import org.apache.druid.segment.loading.SegmentLoadingException;
@@ -149,6 +150,33 @@ public class AzureDataSegmentKillerTest extends EasyMockSupport
   }
 
   @Test
+  public void test_killAll_segmentConfigWithNullContainerAndPrefix_throwsISEException() throws Exception
+  {
+    EasyMock.expect(segmentConfig.getContainer()).andReturn(null).atLeastOnce();
+    EasyMock.expect(segmentConfig.getPrefix()).andReturn(null).anyTimes();
+
+    boolean thrownISEException = false;
+
+    try {
+      AzureDataSegmentKiller killer = new AzureDataSegmentKiller(
+          segmentConfig,
+          inputDataConfig,
+          accountConfig,
+          azureStorage,
+          azureCloudBlobIterableFactory
+      );
+      EasyMock.replay(segmentConfig, inputDataConfig, accountConfig, azureStorage, azureCloudBlobIterableFactory);
+      killer.killAll();
+    }
+    catch (ISE e) {
+      thrownISEException = true;
+    }
+
+    Assert.assertTrue(thrownISEException);
+    EasyMock.verify(segmentConfig, inputDataConfig, accountConfig, azureStorage, azureCloudBlobIterableFactory);
+  }
+
+  @Test
   public void test_killAll_noException_deletesAllSegments() throws Exception
   {
     EasyMock.expect(segmentConfig.getContainer()).andReturn(CONTAINER).atLeastOnce();
diff --git a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleAccountConfig.java b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleAccountConfig.java
index e551f65..b444cfa 100644
--- a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleAccountConfig.java
+++ b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleAccountConfig.java
@@ -21,12 +21,9 @@ package org.apache.druid.storage.google;
 
 import com.fasterxml.jackson.annotation.JsonProperty;
 
-import javax.validation.constraints.NotNull;
-
 public class GoogleAccountConfig
 {
   @JsonProperty
-  @NotNull
   private String bucket;
 
   @JsonProperty
diff --git a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleDataSegmentKiller.java b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleDataSegmentKiller.java
index 1c50f52..c0dc4c8 100644
--- a/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleDataSegmentKiller.java
+++ b/extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleDataSegmentKiller.java
@@ -22,6 +22,7 @@ package org.apache.druid.storage.google;
 import com.google.api.client.http.HttpResponseException;
 import com.google.common.base.Predicates;
 import com.google.inject.Inject;
+import org.apache.druid.java.util.common.ISE;
 import org.apache.druid.java.util.common.MapUtils;
 import org.apache.druid.java.util.common.RE;
 import org.apache.druid.java.util.common.RetryUtils;
@@ -104,6 +105,10 @@ public class GoogleDataSegmentKiller implements DataSegmentKiller
   @Override
   public void killAll() throws IOException
   {
+    if (accountConfig.getBucket() == null || accountConfig.getPrefix() == null) {
+      throw new ISE(
+          "Cannot delete all segment files from Google Deep Storage since druid.google.bucket and druid.google.prefix are not both set.");
+    }
     LOG.info(
         "Deleting all segment files from gs location [bucket: '%s' prefix: '%s']",
         accountConfig.getBucket(),
diff --git a/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleDataSegmentKillerTest.java b/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleDataSegmentKillerTest.java
index 82cda77..97a0e61 100644
--- a/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleDataSegmentKillerTest.java
+++ b/extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleDataSegmentKillerTest.java
@@ -28,6 +28,7 @@ import com.google.api.services.storage.Storage;
 import com.google.api.services.storage.model.StorageObject;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import org.apache.druid.java.util.common.ISE;
 import org.apache.druid.java.util.common.Intervals;
 import org.apache.druid.java.util.common.StringUtils;
 import org.apache.druid.segment.loading.DataSegmentKiller;
@@ -143,6 +144,29 @@ public class GoogleDataSegmentKillerTest extends EasyMockSupport
   }
 
   @Test
+  public void test_killAll_accountConfigWithNullBucketAndPrefix_throwsISEException() throws IOException
+  {
+
+    EasyMock.expect(accountConfig.getBucket()).andReturn(null).atLeastOnce();
+    EasyMock.expect(accountConfig.getPrefix()).andReturn(null).anyTimes();
+
+    boolean thrownISEException = false;
+
+    try {
+      GoogleDataSegmentKiller killer = new GoogleDataSegmentKiller(storage, accountConfig, inputDataConfig);
+      EasyMock.replay(storage, inputDataConfig, accountConfig);
+
+      killer.killAll();
+    }
+    catch (ISE e) {
+      thrownISEException = true;
+    }
+
+    Assert.assertTrue(thrownISEException);
+    EasyMock.verify(accountConfig, inputDataConfig, storage);
+  }
+
+  @Test
   public void test_killAll_noException_deletesAllTaskLogs() throws IOException
   {
     StorageObject object1 = GoogleTestUtils.newStorageObject(BUCKET, KEY_1, TIME_0);
diff --git a/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3DataSegmentKiller.java b/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3DataSegmentKiller.java
index cffc3d3..8a24dc7 100644
--- a/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3DataSegmentKiller.java
+++ b/extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3DataSegmentKiller.java
@@ -22,6 +22,7 @@ package org.apache.druid.storage.s3;
 import com.amazonaws.AmazonServiceException;
 import com.google.common.base.Predicates;
 import com.google.inject.Inject;
+import org.apache.druid.java.util.common.ISE;
 import org.apache.druid.java.util.common.MapUtils;
 import org.apache.druid.java.util.common.logger.Logger;
 import org.apache.druid.segment.loading.DataSegmentKiller;
@@ -82,6 +83,10 @@ public class S3DataSegmentKiller implements DataSegmentKiller
   @Override
   public void killAll() throws IOException
   {
+    if (segmentPusherConfig.getBucket() == null || segmentPusherConfig.getBaseKey() == null) {
+      throw new ISE(
+          "Cannot delete all segment from S3 Deep Storage since druid.storage.bucket and druid.storage.baseKey are not both set.");
+    }
     log.info("Deleting all segment files from s3 location [bucket: '%s' prefix: '%s']",
              segmentPusherConfig.getBucket(), segmentPusherConfig.getBaseKey()
     );
diff --git a/extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3DataSegmentKillerTest.java b/extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3DataSegmentKillerTest.java
index 6260d3a..2e029fe 100644
--- a/extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3DataSegmentKillerTest.java
+++ b/extensions-core/s3-extensions/src/test/java/org/apache/druid/storage/s3/S3DataSegmentKillerTest.java
@@ -24,6 +24,7 @@ import com.amazonaws.services.s3.model.DeleteObjectsRequest;
 import com.amazonaws.services.s3.model.S3ObjectSummary;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import org.apache.druid.java.util.common.ISE;
 import org.apache.druid.java.util.common.StringUtils;
 import org.easymock.EasyMock;
 import org.easymock.EasyMockRunner;
@@ -60,6 +61,30 @@ public class S3DataSegmentKillerTest extends EasyMockSupport
   private S3DataSegmentKiller segmentKiller;
 
   @Test
+  public void test_killAll_accountConfigWithNullBucketAndBaseKey_throwsISEException() throws IOException
+  {
+    EasyMock.expect(segmentPusherConfig.getBucket()).andReturn(null);
+    EasyMock.expectLastCall().atLeastOnce();
+    EasyMock.expect(segmentPusherConfig.getBaseKey()).andReturn(null);
+    EasyMock.expectLastCall().anyTimes();
+
+    boolean thrownISEException = false;
+
+    try {
+
+      EasyMock.replay(s3Client, segmentPusherConfig, inputDataConfig);
+
+      segmentKiller = new S3DataSegmentKiller(s3Client, segmentPusherConfig, inputDataConfig);
+      segmentKiller.killAll();
+    }
+    catch (ISE e) {
+      thrownISEException = true;
+    }
+    Assert.assertTrue(thrownISEException);
+    EasyMock.verify(s3Client, segmentPusherConfig, inputDataConfig);
+  }
+
+  @Test
   public void test_killAll_noException_deletesAllSegments() throws IOException
   {
     S3ObjectSummary objectSummary1 = S3TestUtils.newS3ObjectSummary(TEST_BUCKET, KEY_1, TIME_0);


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org