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 <[email protected]>
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: [email protected]
For additional commands, e-mail: [email protected]
