[GitHub] [druid] clintropolis opened a new pull request #10124: bump version to 0.20.0-SNAPSHOT
clintropolis opened a new pull request #10124: URL: https://github.com/apache/druid/pull/10124 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[druid] branch 0.19.0 created (now ed981ef)
This is an automated email from the ASF dual-hosted git repository. cwylie pushed a change to branch 0.19.0 in repository https://gitbox.apache.org/repos/asf/druid.git. at ed981ef Add DimFilter.toOptimizedFilter(), ensure that join filter pre-analysis operates on optimized filters (#10056) No new revisions were added by this update. - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[druid] branch master updated: Add DimFilter.toOptimizedFilter(), ensure that join filter pre-analysis operates on optimized filters (#10056)
This is an automated email from the ASF dual-hosted git repository.
ccaominh 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 ed981ef Add DimFilter.toOptimizedFilter(), ensure that join filter
pre-analysis operates on optimized filters (#10056)
ed981ef is described below
commit ed981ef88e1005d1b1e5235da4995b18288d6bf9
Author: Jonathan Wei
AuthorDate: Wed Jul 1 22:26:17 2020 -0700
Add DimFilter.toOptimizedFilter(), ensure that join filter pre-analysis
operates on optimized filters (#10056)
* Ensure that join filter pre-analysis operates on optimized filters, add
DimFilter.toOptimizedFilter
* Remove aggressive equality check that was used for testing
* Use Suppliers.memoize
* Checkstyle
---
.../apache/druid/query/filter/BloomDimFilter.java | 2 +-
.../filter/AbstractOptimizableDimFilter.java} | 35 -
.../apache/druid/query/filter/AndDimFilter.java| 2 +-
.../apache/druid/query/filter/BoundDimFilter.java | 2 +-
.../query/filter/ColumnComparisonDimFilter.java| 2 +-
.../org/apache/druid/query/filter/DimFilter.java | 12
.../druid/query/filter/ExpressionDimFilter.java| 2 +-
.../druid/query/filter/ExtractionDimFilter.java| 2 +-
.../apache/druid/query/filter/FalseDimFilter.java | 2 +-
.../org/apache/druid/query/filter/InDimFilter.java | 2 +-
.../druid/query/filter/IntervalDimFilter.java | 2 +-
.../druid/query/filter/JavaScriptDimFilter.java| 2 +-
.../apache/druid/query/filter/LikeDimFilter.java | 2 +-
.../apache/druid/query/filter/NotDimFilter.java| 2 +-
.../org/apache/druid/query/filter/OrDimFilter.java | 2 +-
.../apache/druid/query/filter/RegexDimFilter.java | 2 +-
.../druid/query/filter/SearchQueryDimFilter.java | 2 +-
.../druid/query/filter/SelectorDimFilter.java | 2 +-
.../druid/query/filter/SpatialDimFilter.java | 2 +-
.../apache/druid/query/filter/TrueDimFilter.java | 2 +-
.../query/groupby/GroupByQueryQueryToolChest.java | 10 +--
.../org/apache/druid/query/scan/ScanQuery.java | 5 --
.../druid/query/scan/ScanQueryQueryToolChest.java | 5 --
.../org/apache/druid/query/search/SearchQuery.java | 5 --
.../query/search/SearchQueryQueryToolChest.java| 5 --
.../timeseries/TimeseriesQueryQueryToolChest.java | 5 --
.../org/apache/druid/query/topn/TopNQuery.java | 5 --
.../druid/query/topn/TopNQueryQueryToolChest.java | 12 ++--
.../org/apache/druid/segment/filter/Filters.java | 2 +-
.../druid/query/filter/FalseDimFilterTest.java | 5 +-
.../druid/query/filter/TrueDimFilterTest.java | 5 +-
.../apache/druid/sql/calcite/CalciteQueryTest.java | 82 ++
32 files changed, 143 insertions(+), 86 deletions(-)
diff --git
a/extensions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/filter/BloomDimFilter.java
b/extensions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/filter/BloomDimFilter.java
index 6918269..a22fec7 100644
---
a/extensions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/filter/BloomDimFilter.java
+++
b/extensions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/filter/BloomDimFilter.java
@@ -38,7 +38,7 @@ import java.util.Set;
/**
*/
-public class BloomDimFilter implements DimFilter
+public class BloomDimFilter extends AbstractOptimizableDimFilter implements
DimFilter
{
private final String dimension;
diff --git
a/processing/src/test/java/org/apache/druid/query/filter/TrueDimFilterTest.java
b/processing/src/main/java/org/apache/druid/query/filter/AbstractOptimizableDimFilter.java
similarity index 53%
copy from
processing/src/test/java/org/apache/druid/query/filter/TrueDimFilterTest.java
copy to
processing/src/main/java/org/apache/druid/query/filter/AbstractOptimizableDimFilter.java
index 3c88382..24d32d3 100644
---
a/processing/src/test/java/org/apache/druid/query/filter/TrueDimFilterTest.java
+++
b/processing/src/main/java/org/apache/druid/query/filter/AbstractOptimizableDimFilter.java
@@ -19,29 +19,24 @@
package org.apache.druid.query.filter;
-import com.fasterxml.jackson.databind.ObjectMapper;
-import nl.jqno.equalsverifier.EqualsVerifier;
-import org.apache.druid.jackson.DefaultObjectMapper;
-import org.junit.Assert;
-import org.junit.Test;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.google.common.base.Supplier;
+import com.google.common.base.Suppliers;
-import java.io.IOException;
-
-public class TrueDimFilterTest
+/**
+ * Base class for DimFilters that support optimization. This abstract class
provides a default implementation of
+ * toOptimizedFilter that relies on the existing optimize() and toFilter()
methods. It uses a memoized supplier.
+ */
+abstract class AbstractOptimizableDimFilter implements DimFilter
{
- @Test
- public void testSerde() throws IOExcepti
[GitHub] [druid] ccaominh merged pull request #10056: Add DimFilter.toOptimizedFilter(), ensure that join filter pre-analysis operates on optimized filters
ccaominh merged pull request #10056: URL: https://github.com/apache/druid/pull/10056 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[druid] branch master updated (60c6bd5 -> 367eaed)
This is an automated email from the ASF dual-hosted git repository. cwylie pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/druid.git. from 60c6bd5 support Aliyun OSS service as deep storage (#9898) add 367eaed Clarify change in behavior for druid.server.maxSize (#10105) No new revisions were added by this update. Summary of changes: docs/configuration/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] clintropolis merged pull request #10105: Clarify change in behavior for druid.server.maxSize
clintropolis merged pull request #10105: URL: https://github.com/apache/druid/pull/10105 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[druid] branch master updated (e2c5bcc -> 60c6bd5)
This is an automated email from the ASF dual-hosted git repository. cwylie pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/druid.git. from e2c5bcc Fix UnknownComplexTypeColumn#makeVectorObjectSelector. Add a warning … (#10123) add 60c6bd5 support Aliyun OSS service as deep storage (#9898) No new revisions were added by this update. Summary of changes: .travis.yml| 4 +- LICENSE| 1 + distribution/pom.xml | 2 + .../extensions-contrib/aliyun-oss-extensions.md| 54 ++ docs/development/extensions.md | 1 + extensions-contrib/aliyun-oss-extensions/pom.xml | 181 ++ .../druid/data/input/aliyun/OssClientConfig.java | 122 .../apache/druid/data/input/aliyun/OssEntity.java | 88 +++ .../druid/data/input/aliyun/OssInputSource.java| 178 ++ .../input/aliyun/OssInputSourceDruidModule.java| 49 ++ .../firehose/aliyun/OssFirehoseDruidModule.java| 46 ++ .../firehose/aliyun/StaticOssFirehoseFactory.java | 243 .../storage/aliyun/OssDataSegmentArchiver.java | 101 .../aliyun/OssDataSegmentArchiverConfig.java | 41 ++ .../druid/storage/aliyun/OssDataSegmentKiller.java | 98 +++ .../druid/storage/aliyun/OssDataSegmentMover.java | 253 .../druid/storage/aliyun/OssDataSegmentPuller.java | 308 ++ .../druid/storage/aliyun/OssDataSegmentPusher.java | 131 .../druid/storage/aliyun/OssInputDataConfig.java | 52 ++ .../apache/druid/storage/aliyun/OssLoadSpec.java | 72 +++ .../storage/aliyun/OssObjectSummaryIterator.java | 156 + .../druid/storage/aliyun/OssStorageConfig.java | 50 ++ .../storage/aliyun/OssStorageDruidModule.java | 106 .../apache/druid/storage/aliyun/OssTaskLogs.java | 201 +++ .../druid/storage/aliyun/OssTaskLogsConfig.java| 73 +++ .../aliyun/OssTimestampVersionedDataFinder.java| 90 +++ .../org/apache/druid/storage/aliyun/OssUtils.java | 271 + .../org.apache.druid.initialization.DruidModule| 18 + .../data/input/aliyun/OssInputSourceTest.java | 660 + .../storage/aliyun/OssDataSegmentArchiverTest.java | 195 ++ .../storage/aliyun/OssDataSegmentKillerTest.java | 205 +++ .../storage/aliyun/OssDataSegmentMoverTest.java| 266 + .../storage/aliyun/OssDataSegmentPullerTest.java | 205 +++ .../aliyun/OssDataSegmentPusherConfigTest.java | 51 ++ .../storage/aliyun/OssDataSegmentPusherTest.java | 125 .../aliyun/OssObjectSummaryIteratorTest.java | 276 + .../druid/storage/aliyun/OssTaskLogsTest.java | 336 +++ .../apache/druid/storage/aliyun/OssTestUtils.java | 177 ++ .../OssTimestampVersionedDataFinderTest.java | 178 ++ .../environment-configs/override-examples/oss | 30 + .../java/org/apache/druid/tests/TestNGGroup.java | 7 + .../AbstractOssInputSourceParallelIndexTest.java | 132 + .../tests/indexer/ITOssToOssParallelIndexTest.java | 49 ++ pom.xml| 1 + website/.spelling | 10 +- 45 files changed, 5890 insertions(+), 3 deletions(-) create mode 100644 docs/development/extensions-contrib/aliyun-oss-extensions.md create mode 100644 extensions-contrib/aliyun-oss-extensions/pom.xml create mode 100644 extensions-contrib/aliyun-oss-extensions/src/main/java/org/apache/druid/data/input/aliyun/OssClientConfig.java create mode 100644 extensions-contrib/aliyun-oss-extensions/src/main/java/org/apache/druid/data/input/aliyun/OssEntity.java create mode 100644 extensions-contrib/aliyun-oss-extensions/src/main/java/org/apache/druid/data/input/aliyun/OssInputSource.java create mode 100644 extensions-contrib/aliyun-oss-extensions/src/main/java/org/apache/druid/data/input/aliyun/OssInputSourceDruidModule.java create mode 100644 extensions-contrib/aliyun-oss-extensions/src/main/java/org/apache/druid/firehose/aliyun/OssFirehoseDruidModule.java create mode 100644 extensions-contrib/aliyun-oss-extensions/src/main/java/org/apache/druid/firehose/aliyun/StaticOssFirehoseFactory.java create mode 100644 extensions-contrib/aliyun-oss-extensions/src/main/java/org/apache/druid/storage/aliyun/OssDataSegmentArchiver.java create mode 100644 extensions-contrib/aliyun-oss-extensions/src/main/java/org/apache/druid/storage/aliyun/OssDataSegmentArchiverConfig.java create mode 100644 extensions-contrib/aliyun-oss-extensions/src/main/java/org/apache/druid/storage/aliyun/OssDataSegmentKiller.java create mode 100644 extensions-contrib/aliyun-oss-extensions/src/main/java/org/apache/druid/storage/aliyun/OssDataSegmentMover.java create mode 100644 extensions-contrib/aliyun-oss-extensions/src/main/java/org/apache/druid/storage/aliyun/OssDataSegmentPuller.java create mode 100644 extensions-contrib/
[GitHub] [druid] clintropolis merged pull request #9898: support Aliyun OSS service as deep storage
clintropolis merged pull request #9898: URL: https://github.com/apache/druid/pull/9898 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] samarthjain merged pull request #10123: Fix UnknownComplexTypeColumn#makeVectorObjectSelector. Add a warning …
samarthjain merged pull request #10123: URL: https://github.com/apache/druid/pull/10123 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[druid] branch master updated: Fix UnknownComplexTypeColumn#makeVectorObjectSelector. Add a warning … (#10123)
This is an automated email from the ASF dual-hosted git repository.
samarth 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 e2c5bcc Fix UnknownComplexTypeColumn#makeVectorObjectSelector. Add a
warning … (#10123)
e2c5bcc is described below
commit e2c5bcc22d2c175bba51fbb2a9c90303ecdf6f44
Author: Samarth Jain
AuthorDate: Wed Jul 1 20:06:23 2020 -0700
Fix UnknownComplexTypeColumn#makeVectorObjectSelector. Add a warning …
(#10123)
* Fix UnknownComplexTypeColumn#makeVectorObjectSelector. Add a warning
message to indicate failure in deserializing.
---
.../segment/column/UnknownTypeComplexColumn.java | 19 +--
.../druid/segment/serde/ComplexColumnPartSerde.java | 7 +++
2 files changed, 8 insertions(+), 18 deletions(-)
diff --git
a/processing/src/main/java/org/apache/druid/segment/column/UnknownTypeComplexColumn.java
b/processing/src/main/java/org/apache/druid/segment/column/UnknownTypeComplexColumn.java
index 7332a5c..ce4db70 100644
---
a/processing/src/main/java/org/apache/druid/segment/column/UnknownTypeComplexColumn.java
+++
b/processing/src/main/java/org/apache/druid/segment/column/UnknownTypeComplexColumn.java
@@ -25,7 +25,6 @@ import org.apache.druid.segment.data.ReadableOffset;
import org.apache.druid.segment.vector.NilVectorSelector;
import org.apache.druid.segment.vector.ReadableVectorOffset;
import org.apache.druid.segment.vector.VectorObjectSelector;
-import org.apache.druid.segment.vector.VectorSizeInspector;
import javax.annotation.Nullable;
@@ -38,22 +37,6 @@ public class UnknownTypeComplexColumn implements
ComplexColumn
return INSTANCE;
}
- private static final NilVectorSelector NIL_VECTOR_SELECTOR_INSTANCE =
- NilVectorSelector.create(new VectorSizeInspector()
- {
-@Override
-public int getMaxVectorSize()
-{
- return 0;
-}
-
-@Override
-public int getCurrentVectorSize()
-{
- return 0;
-}
- });
-
@Override
public Class getClazz()
{
@@ -94,6 +77,6 @@ public class UnknownTypeComplexColumn implements ComplexColumn
@Override
public VectorObjectSelector makeVectorObjectSelector(ReadableVectorOffset
offset)
{
-return NIL_VECTOR_SELECTOR_INSTANCE;
+return NilVectorSelector.create(offset);
}
}
diff --git
a/processing/src/main/java/org/apache/druid/segment/serde/ComplexColumnPartSerde.java
b/processing/src/main/java/org/apache/druid/segment/serde/ComplexColumnPartSerde.java
index f10f53b..dc0278b 100644
---
a/processing/src/main/java/org/apache/druid/segment/serde/ComplexColumnPartSerde.java
+++
b/processing/src/main/java/org/apache/druid/segment/serde/ComplexColumnPartSerde.java
@@ -21,6 +21,7 @@ package org.apache.druid.segment.serde;
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.druid.java.util.common.logger.Logger;
import org.apache.druid.segment.GenericColumnSerializer;
import javax.annotation.Nullable;
@@ -33,11 +34,17 @@ public class ComplexColumnPartSerde implements
ColumnPartSerde
@Nullable
private final ComplexMetricSerde serde;
private final Serializer serializer;
+ private static final Logger log = new Logger(ComplexColumnPartSerde.class);
private ComplexColumnPartSerde(String typeName, Serializer serializer)
{
this.typeName = typeName;
this.serde = ComplexMetrics.getSerdeForType(typeName);
+if (this.serde == null) {
+ // Not choosing to fail here since this gets handled as
+ // an UnknownTypeComplexColumn. See SimpleColumnHolder#getColumn.
+ log.warn("Unknown complex column of type %s detected", typeName);
+}
this.serializer = serializer;
}
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] samarthjain commented on a change in pull request #10123: Fix UnknownComplexTypeColumn#makeVectorObjectSelector. Add a warning …
samarthjain commented on a change in pull request #10123:
URL: https://github.com/apache/druid/pull/10123#discussion_r448722745
##
File path:
processing/src/main/java/org/apache/druid/segment/serde/ComplexColumnPartSerde.java
##
@@ -33,11 +34,17 @@
@Nullable
private final ComplexMetricSerde serde;
private final Serializer serializer;
+ private static final Logger log = new Logger(ComplexColumnPartSerde.class);
private ComplexColumnPartSerde(String typeName, Serializer serializer)
{
this.typeName = typeName;
this.serde = ComplexMetrics.getSerdeForType(typeName);
+if (this.serde == null) {
+ // Not chosing to fail here since this gets handled as
Review comment:
Thanks. I pushed a commit to fix the typo. Will go ahead and merge to
master.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] ccaominh commented on a change in pull request #10056: Add DimFilter.toOptimizedFilter(), ensure that join filter pre-analysis operates on optimized filters
ccaominh commented on a change in pull request #10056: URL: https://github.com/apache/druid/pull/10056#discussion_r448715436 ## File path: processing/src/main/java/org/apache/druid/query/filter/DimFilter.java ## @@ -23,7 +23,11 @@ import com.fasterxml.jackson.annotation.JsonTypeInfo; import com.google.common.collect.RangeSet; import org.apache.druid.java.util.common.Cacheable; +import org.apache.druid.java.util.common.granularity.Granularity; +import org.apache.druid.query.QueryMetrics; import org.apache.druid.query.extraction.ExtractionFn; +import org.apache.druid.segment.VirtualColumns; +import org.joda.time.Interval; Review comment: CI is flagging these 4 as unused imports This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] clintropolis commented on a change in pull request #10123: Fix UnknownComplexTypeColumn#makeVectorObjectSelector. Add a warning …
clintropolis commented on a change in pull request #10123:
URL: https://github.com/apache/druid/pull/10123#discussion_r448709942
##
File path:
processing/src/main/java/org/apache/druid/segment/serde/ComplexColumnPartSerde.java
##
@@ -33,11 +34,17 @@
@Nullable
private final ComplexMetricSerde serde;
private final Serializer serializer;
+ private static final Logger log = new Logger(ComplexColumnPartSerde.class);
private ComplexColumnPartSerde(String typeName, Serializer serializer)
{
this.typeName = typeName;
this.serde = ComplexMetrics.getSerdeForType(typeName);
+if (this.serde == null) {
+ // Not chosing to fail here since this gets handled as
Review comment:
nit: chosing -> choosing
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[druid] branch master updated (1676ba2 -> c5540f4)
This is an automated email from the ASF dual-hosted git repository. cwylie pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/druid.git. from 1676ba2 Fix Stack overflow with infinite loop in ReduceExpressionsRule of HepProgram (#10120) add c5540f4 fixes for ranger docs (#10109) No new revisions were added by this update. Summary of changes: .../extensions-core/druid-ranger-security.md | 77 +- website/sidebars.json | 1 + 2 files changed, 19 insertions(+), 59 deletions(-) - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] clintropolis merged pull request #10109: fixes for apache ranger extension docs
clintropolis merged pull request #10109: URL: https://github.com/apache/druid/pull/10109 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] FrankChen021 commented on issue #10122: StorageLocation.availableSizeBytes doesn't consider segment cache usage on disk
FrankChen021 commented on issue #10122: URL: https://github.com/apache/druid/issues/10122#issuecomment-652720683 There's another problem related to `CurrSize` which I have not investigated. That is `CurrSize` is bigger than the actual disk consumption. On several default tier historical nodes of our clusters, OS shows that there's only 390G+ total disk consumption however the web console shows that `CurrSize` is 410G+. I don't know whether this is related to the problem above. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] jon-wei merged pull request #10120: Fix Stack overflow with infinite loop in ReduceExpressionsRule of HepProgram
jon-wei merged pull request #10120: URL: https://github.com/apache/druid/pull/10120 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] jon-wei closed issue #9906: Stack overflow with SELECT ARRAY ['Hello', NULL]
jon-wei closed issue #9906: URL: https://github.com/apache/druid/issues/9906 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[druid] branch master updated: Fix Stack overflow with infinite loop in ReduceExpressionsRule of HepProgram (#10120)
This is an automated email from the ASF dual-hosted git repository.
jonwei 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 1676ba2 Fix Stack overflow with infinite loop in
ReduceExpressionsRule of HepProgram (#10120)
1676ba2 is described below
commit 1676ba22e300ea95cc92c0808f390aaa769546f9
Author: Maytas Monsereenusorn
AuthorDate: Wed Jul 1 17:48:09 2020 -0700
Fix Stack overflow with infinite loop in ReduceExpressionsRule of
HepProgram (#10120)
* Fix Stack overflow with SELECT ARRAY ['Hello', NULL]
* address comments
---
.../apache/druid/sql/calcite/planner/Rules.java| 30 +-
.../apache/druid/sql/calcite/CalciteQueryTest.java | 13 ++
2 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/Rules.java
b/sql/src/main/java/org/apache/druid/sql/calcite/planner/Rules.java
index 03b1a31..c9135d5 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/Rules.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/Rules.java
@@ -26,10 +26,13 @@ import org.apache.calcite.plan.RelOptMaterialization;
import org.apache.calcite.plan.RelOptPlanner;
import org.apache.calcite.plan.RelOptRule;
import org.apache.calcite.plan.RelTraitSet;
+import org.apache.calcite.plan.hep.HepProgram;
+import org.apache.calcite.plan.hep.HepProgramBuilder;
import org.apache.calcite.plan.volcano.AbstractConverter;
import org.apache.calcite.rel.RelNode;
import org.apache.calcite.rel.core.RelFactories;
import org.apache.calcite.rel.metadata.DefaultRelMetadataProvider;
+import org.apache.calcite.rel.metadata.RelMetadataProvider;
import org.apache.calcite.rel.rules.AggregateCaseToFilterRule;
import org.apache.calcite.rel.rules.AggregateExpandDistinctAggregatesRule;
import org.apache.calcite.rel.rules.AggregateJoinTransposeRule;
@@ -85,6 +88,16 @@ public class Rules
public static final int DRUID_CONVENTION_RULES = 0;
public static final int BINDABLE_CONVENTION_RULES = 1;
+ // Due to Calcite bug (CALCITE-3845), ReduceExpressionsRule can considered
expression which is the same as the
+ // previous input expression as reduced. Basically, the expression is
actually not reduced but is still considered as
+ // reduced. Hence, this resulted in an infinite loop of Calcite trying to
reducing the same expression over and over.
+ // Calcite 1.23.0 fixes this issue by not consider expression as reduced if
this case happens. However, while
+ // we are still using Calcite 1.21.0, a workaround is to limit the number of
pattern matches to avoid infinite loop.
+ private static final String HEP_DEFAULT_MATCH_LIMIT_CONFIG_STRING =
"druid.sql.planner.hepMatchLimit";
+ private static final int HEP_DEFAULT_MATCH_LIMIT = Integer.valueOf(
+ System.getProperty(HEP_DEFAULT_MATCH_LIMIT_CONFIG_STRING, "1200")
+ );
+
// Rules from RelOptUtil's registerBaseRules, minus:
//
// 1) AggregateExpandDistinctAggregatesRule (it'll be added back later if
approximate count distinct is disabled)
@@ -191,12 +204,14 @@ public class Rules
public static List programs(final PlannerContext plannerContext,
final QueryMaker queryMaker)
{
+
+
// Program that pre-processes the tree before letting the full-on
VolcanoPlanner loose.
final Program preProgram =
Programs.sequence(
Programs.subQuery(DefaultRelMetadataProvider.INSTANCE),
DecorrelateAndTrimFieldsProgram.INSTANCE,
-Programs.hep(REDUCTION_RULES, true,
DefaultRelMetadataProvider.INSTANCE)
+buildHepProgram(REDUCTION_RULES, true,
DefaultRelMetadataProvider.INSTANCE, HEP_DEFAULT_MATCH_LIMIT)
);
return ImmutableList.of(
@@ -205,6 +220,19 @@ public class Rules
);
}
+ private static Program buildHepProgram(Iterable rules,
+ boolean noDag,
+ RelMetadataProvider metadataProvider,
+ int matchLimit)
+ {
+final HepProgramBuilder builder = HepProgram.builder();
+builder.addMatchLimit(matchLimit);
+for (RelOptRule rule : rules) {
+ builder.addRuleInstance(rule);
+}
+return Programs.of(builder.build(), noDag, metadataProvider);
+ }
+
private static List druidConventionRuleSet(
final PlannerContext plannerContext,
final QueryMaker queryMaker
diff --git
a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
index 17c970f..483cdd2 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
@@ -140,6 +140,19 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
}
@Test
+ public vo
[GitHub] [druid] samarthjain commented on a change in pull request #10123: Fix UnknownComplexTypeColumn#makeVectorObjectSelector. Add a warning …
samarthjain commented on a change in pull request #10123: URL: https://github.com/apache/druid/pull/10123#discussion_r448684854 ## File path: processing/src/main/java/org/apache/druid/segment/column/SimpleColumnHolder.java ## @@ -21,6 +21,7 @@ import com.google.common.base.Preconditions; import com.google.common.base.Supplier; +import org.apache.druid.java.util.common.logger.Logger; Review comment: oh no! I just pushed a commit to remove the import. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] clintropolis commented on a change in pull request #10123: Fix UnknownComplexTypeColumn#makeVectorObjectSelector. Add a warning …
clintropolis commented on a change in pull request #10123: URL: https://github.com/apache/druid/pull/10123#discussion_r448682947 ## File path: processing/src/main/java/org/apache/druid/segment/column/SimpleColumnHolder.java ## @@ -21,6 +21,7 @@ import com.google.common.base.Preconditions; import com.google.common.base.Supplier; +import org.apache.druid.java.util.common.logger.Logger; Review comment: I think this is going to fail Travis for unused import This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] lgtm-com[bot] commented on pull request #10097: Do not echo back username on auth failure
lgtm-com[bot] commented on pull request #10097: URL: https://github.com/apache/druid/pull/10097#issuecomment-652689662 This pull request **fixes 1 alert** when merging 37e88dab5afdc8ff077f6503a88db79faa5f7a89 into 477335abb4fe3e872a1b8f71d9c2bd90e4315fbc - [view on LGTM.com](https://lgtm.com/projects/g/apache/druid/rev/pr-b711591b3e76bbe446fa20ef95d9291e56b236c8) **fixed alerts:** * 1 for Cross\-site scripting This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] gianm commented on a change in pull request #10106: Add validation for authorizer name
gianm commented on a change in pull request #10106:
URL: https://github.com/apache/druid/pull/10106#discussion_r448641609
##
File path:
server/src/main/java/org/apache/druid/server/initialization/AuthorizerMapperModule.java
##
@@ -58,7 +62,10 @@ public void configure(Binder binder)
binder.bind(AuthorizerMapper.class)
.toProvider(new AuthorizerMapperProvider())
.in(LazySingleton.class);
-
+binder.bind(new TypeLiteral>() {})
Review comment:
Why not use an interface named `AuthorizerNameValidator`? Then the
interface could have javadocs explaining the expected usage and contract.
##
File path:
server/src/main/java/org/apache/druid/server/security/AuthorizerNameValidator.java
##
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.server.security;
+
+import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
+import org.apache.druid.java.util.common.StringUtils;
+
+import java.util.function.Consumer;
+
+/**
+ * A class that validates the name of an authorizer.
+ */
+public class AuthorizerNameValidator implements Consumer
+{
+ private static final String AUTHORIZER_NAME = "authorizerName";
Review comment:
It would be good to also use this constant in
`AuthorizerResourceFilter.filter` and in the `@PathParam` annotations of
BasicAuthorizerResource. There's a lot of places where the string
"authorizerName" appears, and using a constant would emphasize that this
specific string is important.
##
File path:
indexing-service/src/main/java/org/apache/druid/indexing/overlord/http/security/AuthorizerResourceFilter.java
##
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.indexing.overlord.http.security;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Iterables;
+import com.sun.jersey.spi.container.ContainerRequest;
+import com.sun.jersey.spi.container.ContainerRequestFilter;
+import com.sun.jersey.spi.container.ContainerResponseFilter;
+import com.sun.jersey.spi.container.ResourceFilter;
+import org.apache.druid.server.security.AuthorizerValidation;
+
+import javax.inject.Inject;
+import java.util.function.Consumer;
+
+public class AuthorizerResourceFilter implements ResourceFilter,
ContainerRequestFilter
+{
+ private final Consumer authorizerNameValidator;
+
+ @Inject
+ AuthorizerResourceFilter(@AuthorizerValidation Consumer
authorizerNameValidator)
+ {
+this.authorizerNameValidator = authorizerNameValidator;
+ }
+
+ @Override
+ public ContainerRequestFilter getRequestFilter()
+ {
+return this;
+ }
+
+ @Override
+ public ContainerResponseFilter getResponseFilter()
+ {
+return null;
+ }
+
+ @Override
+ public ContainerRequest filter(ContainerRequest request)
+ {
+String authorizerName = Preconditions.checkNotNull(
+request.getPathSegments()
+ .get(
+ Iterables.indexOf(
+ request.getPathSegments(),
+ input -> "authorizerName".equals(input.getPath())
Review comment:
Are you sure this is right? I would think that `getPath()` returns the
contents of the path segment (i.e., the name of the authorizer), not the name
of the path segment.
##
File path:
indexing-service/src/main/java/org/apache/druid/indexing/overlord/
[GitHub] [druid] clintropolis commented on a change in pull request #10123: Fix UnknownComplexTypeColumn#makeVectorObjectSelector. Add a warning …
clintropolis commented on a change in pull request #10123:
URL: https://github.com/apache/druid/pull/10123#discussion_r448642655
##
File path:
processing/src/main/java/org/apache/druid/segment/column/SimpleColumnHolder.java
##
@@ -61,6 +63,8 @@
capabilities.getType() == ValueType.COMPLEX,
"Only complex column types can have nullable column suppliers"
);
+ log.warn("Null column supplier for a complex column was passed which
indicates "
Review comment:
Sorry I just looked even closer, I now think the _actual_ best place for
this log is probably in `ComplexColumnPartSerde`, in [the `getDeserializer`
method](https://github.com/apache/druid/blob/master/processing/src/main/java/org/apache/druid/segment/serde/ComplexColumnPartSerde.java#L73)
if serde is null. This way you can include the 'typeName' of the missing type,
which isn't available in `SimpleColumnHolder`, my bad.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] suneet-s commented on pull request #10097: Do not echo back username on auth failure
suneet-s commented on pull request #10097: URL: https://github.com/apache/druid/pull/10097#issuecomment-652667680 Since this wasn't an issue before this patch, I've removed the label `Security` @clintropolis please add it back / lmk if it's needed. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] jihoonson edited a comment on pull request #10121: Revert "Fix UnknownTypeComplexColumn#makeVectorObjectSelector"
jihoonson edited a comment on pull request #10121: URL: https://github.com/apache/druid/pull/10121#issuecomment-652667303 I'm fine with doing it in a follow-up too. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] jihoonson commented on pull request #10121: Revert "Fix UnknownTypeComplexColumn#makeVectorObjectSelector"
jihoonson commented on pull request #10121: URL: https://github.com/apache/druid/pull/10121#issuecomment-652667303 I'm fine with follow-up too. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[druid] branch master updated (3e92cdf -> 477335a)
This is an automated email from the ASF dual-hosted git repository. ccaominh pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/druid.git. from 3e92cdf Revert "Fix UnknownTypeComplexColumn#makeVectorObjectSelector" (#10121) add 477335a update links datasketches.github.io to datasketches.apache.org (#10107) No new revisions were added by this update. Summary of changes: docs/development/extensions-core/datasketches-extension.md | 2 +- docs/development/extensions-core/datasketches-hll.md | 2 +- docs/development/extensions-core/datasketches-quantiles.md | 4 ++-- docs/development/extensions-core/datasketches-theta.md | 4 ++-- docs/development/extensions-core/datasketches-tuple.md | 4 ++-- docs/development/extensions.md | 8 docs/querying/aggregations.md| 12 ++-- extensions-core/datasketches/README.md | 2 +- extensions-core/datasketches/pom.xml | 2 +- .../query/aggregation/datasketches/hll/HllSketchModule.java | 2 +- .../datasketches/tuple/ArrayOfDoublesSketchModule.java | 2 +- .../ArrayOfDoublesSketchToQuantilesSketchPostAggregator.java | 2 +- web-console/src/utils/ingestion-spec.tsx | 4 ++-- 13 files changed, 25 insertions(+), 25 deletions(-) - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] ccaominh merged pull request #10107: update links datasketches.github.io to datasketches.apache.org
ccaominh merged pull request #10107: URL: https://github.com/apache/druid/pull/10107 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] samarthjain opened a new pull request #10123: Fix UnknownComplexTypeColumn#makeVectorObjectSelector. Add a warning …
samarthjain opened a new pull request #10123: URL: https://github.com/apache/druid/pull/10123 Follow up fix for #9422 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] clintropolis commented on pull request #10121: Revert "Fix UnknownTypeComplexColumn#makeVectorObjectSelector"
clintropolis commented on pull request #10121: URL: https://github.com/apache/druid/pull/10121#issuecomment-652658749 @samarthjain could you open a PR to make the change of this revert, also maybe consider adding a log.warn somewhere https://github.com/apache/druid/pull/9422#discussion_r44862179? It would also be nice to try to setup a test that tries to use the selectors of this unknown complex column in a query, to make sure the fix works, but since I think this change should go into 0.19 and this is all I am waiting on, I wouldn't block a PR that is missing that and would be ok to add as another follow-up. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] clintropolis commented on pull request #10121: Revert "Fix UnknownTypeComplexColumn#makeVectorObjectSelector"
clintropolis commented on pull request #10121: URL: https://github.com/apache/druid/pull/10121#issuecomment-652657541 skipping CI since this is a revert This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] clintropolis merged pull request #10121: Revert "Fix UnknownTypeComplexColumn#makeVectorObjectSelector"
clintropolis merged pull request #10121: URL: https://github.com/apache/druid/pull/10121 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[druid] branch master updated: Revert "Fix UnknownTypeComplexColumn#makeVectorObjectSelector" (#10121)
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 3e92cdf Revert "Fix
UnknownTypeComplexColumn#makeVectorObjectSelector" (#10121)
3e92cdf is described below
commit 3e92cdf1cfcde8447e3a61dfe36eb813716ad202
Author: Samarth Jain
AuthorDate: Wed Jul 1 14:33:17 2020 -0700
Revert "Fix UnknownTypeComplexColumn#makeVectorObjectSelector" (#10121)
This reverts commit 7bb7489afc7a2cc496be93ae69681b6ab13a7c66.
---
.../segment/column/UnknownTypeComplexColumn.java | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git
a/processing/src/main/java/org/apache/druid/segment/column/UnknownTypeComplexColumn.java
b/processing/src/main/java/org/apache/druid/segment/column/UnknownTypeComplexColumn.java
index ce4db70..7332a5c 100644
---
a/processing/src/main/java/org/apache/druid/segment/column/UnknownTypeComplexColumn.java
+++
b/processing/src/main/java/org/apache/druid/segment/column/UnknownTypeComplexColumn.java
@@ -25,6 +25,7 @@ import org.apache.druid.segment.data.ReadableOffset;
import org.apache.druid.segment.vector.NilVectorSelector;
import org.apache.druid.segment.vector.ReadableVectorOffset;
import org.apache.druid.segment.vector.VectorObjectSelector;
+import org.apache.druid.segment.vector.VectorSizeInspector;
import javax.annotation.Nullable;
@@ -37,6 +38,22 @@ public class UnknownTypeComplexColumn implements
ComplexColumn
return INSTANCE;
}
+ private static final NilVectorSelector NIL_VECTOR_SELECTOR_INSTANCE =
+ NilVectorSelector.create(new VectorSizeInspector()
+ {
+@Override
+public int getMaxVectorSize()
+{
+ return 0;
+}
+
+@Override
+public int getCurrentVectorSize()
+{
+ return 0;
+}
+ });
+
@Override
public Class getClazz()
{
@@ -77,6 +94,6 @@ public class UnknownTypeComplexColumn implements ComplexColumn
@Override
public VectorObjectSelector makeVectorObjectSelector(ReadableVectorOffset
offset)
{
-return NilVectorSelector.create(offset);
+return NIL_VECTOR_SELECTOR_INSTANCE;
}
}
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] clintropolis commented on a change in pull request #9422: Handle unknown complex types
clintropolis commented on a change in pull request #9422:
URL: https://github.com/apache/druid/pull/9422#discussion_r448621794
##
File path:
processing/src/main/java/org/apache/druid/segment/column/UnknownTypeComplexColumn.java
##
@@ -0,0 +1,99 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.segment.column;
+
+import org.apache.druid.segment.ColumnValueSelector;
+import org.apache.druid.segment.NilColumnValueSelector;
+import org.apache.druid.segment.data.ReadableOffset;
+import org.apache.druid.segment.vector.NilVectorSelector;
+import org.apache.druid.segment.vector.ReadableVectorOffset;
+import org.apache.druid.segment.vector.VectorObjectSelector;
+import org.apache.druid.segment.vector.VectorSizeInspector;
+
+import javax.annotation.Nullable;
+
+public class UnknownTypeComplexColumn implements ComplexColumn
+{
+ private static final UnknownTypeComplexColumn INSTANCE = new
UnknownTypeComplexColumn();
+
+ public static UnknownTypeComplexColumn instance()
+ {
+return INSTANCE;
+ }
+
+ private static final NilVectorSelector NIL_VECTOR_SELECTOR_INSTANCE =
+ NilVectorSelector.create(new VectorSizeInspector()
+ {
+@Override
+public int getMaxVectorSize()
+{
+ return 0;
+}
+
+@Override
+public int getCurrentVectorSize()
+{
+ return 0;
+}
+ });
+
+ @Override
+ public Class getClazz()
+ {
+return ComplexColumn.class;
+ }
+
+ @Override
+ public String getTypeName()
+ {
+return "UNKNOWN_COMPLEX_COLUMN_TYPE";
+ }
+
+ @Nullable
+ @Override
+ public Object getRowValue(int rowNum)
+ {
+return null;
+ }
+
+ @Override
+ public int getLength()
+ {
+return 0;
+ }
+
+ @Override
+ public void close()
+ {
+
+ }
+
+ @Override
+ public ColumnValueSelector makeColumnValueSelector(ReadableOffset offset)
+ {
+return NilColumnValueSelector.instance();
Review comment:
Thinking more about it though, I do think we should log somewhere so the
operator can know that an aggregator isn't loaded so a complex column can't be
read. Maybe the `SimpleColumnHolder` would be more appropriate of a place?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[druid] branch master updated (657f8ee -> a337ef3)
This is an automated email from the ASF dual-hosted git repository. gian pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/druid.git. from 657f8ee Fix RetryQueryRunner to actually do the job (#10082) add a337ef3 Closing yielder from ParallelMergeCombiningSequence should trigger cancellation (#10117) No new revisions were added by this update. Summary of changes: .../guava/ParallelMergeCombiningSequence.java | 14 ++- .../guava/ParallelMergeCombiningSequenceTest.java | 107 + 2 files changed, 120 insertions(+), 1 deletion(-) - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] gianm merged pull request #10117: Closing yielder from ParallelMergeCombiningSequence should trigger cancellation
gianm merged pull request #10117: URL: https://github.com/apache/druid/pull/10117 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] jihoonson commented on pull request #10082: Fix RetryQueryRunner to actually do the job
jihoonson commented on pull request #10082: URL: https://github.com/apache/druid/pull/10082#issuecomment-652645612 @gianm @clintropolis thank you for the review. I'll address @gianm's last comments in follow-ups. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] clintropolis merged pull request #10082: Fix RetryQueryRunner to actually do the job
clintropolis merged pull request #10082: URL: https://github.com/apache/druid/pull/10082 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] jon-wei commented on a change in pull request #10056: Add DimFilter.toOptimizedFilter(), ensure that join filter pre-analysis operates on optimized filters
jon-wei commented on a change in pull request #10056:
URL: https://github.com/apache/druid/pull/10056#discussion_r448611416
##
File path:
processing/src/main/java/org/apache/druid/query/filter/AbstractOptimizableDimFilter.java
##
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.query.filter;
+
+import com.fasterxml.jackson.annotation.JsonIgnore;
+
+/**
+ * Base class for DimFilters that support optimization.
+ */
+abstract class AbstractOptimizableDimFilter implements DimFilter
+{
+ private Filter cachedOptimizedFilter = null;
+
+ @JsonIgnore
+ @Override
+ public Filter toOptimizedFilter()
Review comment:
Ah, I was originally thinking of just letting the processing threads for
non-joins possibly do some redundant computation (for joins, the pre-analysis
would call toOptimizedFilter before the processing threads run the query), but
I can adjust it.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] gianm commented on a change in pull request #10056: Add DimFilter.toOptimizedFilter(), ensure that join filter pre-analysis operates on optimized filters
gianm commented on a change in pull request #10056:
URL: https://github.com/apache/druid/pull/10056#discussion_r448606808
##
File path:
processing/src/main/java/org/apache/druid/query/filter/AbstractOptimizableDimFilter.java
##
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.query.filter;
+
+import com.fasterxml.jackson.annotation.JsonIgnore;
+
+/**
+ * Base class for DimFilters that support optimization.
+ */
+abstract class AbstractOptimizableDimFilter implements DimFilter
+{
+ private Filter cachedOptimizedFilter = null;
+
+ @JsonIgnore
+ @Override
+ public Filter toOptimizedFilter()
Review comment:
I think this is going to be called by different processing threads
simultaneously, so it should be thread-safe. Perhaps use Suppliers.memoize.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] a2l007 opened a new issue #10122: StorageLocation.availableSizeBytes doesn't consider segment cache usage on disk
a2l007 opened a new issue #10122: URL: https://github.com/apache/druid/issues/10122 One of our internal clusters had a weird issue where certain historicals were running out of disk space and during the investigation I came across the following behavior: It looks like [StorageLocation.availableSizeBytes()](https://github.com/apache/druid/blob/master/server/src/main/java/org/apache/druid/segment/loading/StorageLocation.java#L179) isn't an accurate representation of the available size left in the segment cache. [StorageLocation.currSizeBytes](https://github.com/apache/druid/blob/master/server/src/main/java/org/apache/druid/segment/loading/StorageLocation.java#L60) is zero every time a node starts up and therefore it doesn't account for segments already present in the segmentcache. Lets say, a historical has filled up 50% of its 10GB segment cache capacity and the historical had to be restarted for some reason. Once the process comes back up, the currSizeBytes is zero and `StorageLocation.availableSizeBytes` would incorrectly return 10GB instead of 5GB. I would expect this method to work similar to [ServerHolder.getAvailableSize()](https://github.com/apache/druid/blob/master/server/src/main/java/org/apache/druid/server/coordinator/ServerHolder.java#L97) which does take into account the actual usage of the server. Before treating it as a bug and fixing it, I wanted to check if this is indeed the expected behavior for `StorageLocation.availableSizeBytes()` and if it isn't expected to keep track of the actual segment cache size on disk? @jihoonson @clintropolis Any comments regarding this? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] suneet-s commented on a change in pull request #10109: fixes for apache ranger extension docs
suneet-s commented on a change in pull request #10109: URL: https://github.com/apache/druid/pull/10109#discussion_r448592173 ## File path: docs/development/extensions-core/druid-ranger-security.md ## @@ -22,43 +22,30 @@ title: "Apache Ranger Security" ~ under the License. --> -This Apache Druid extension adds: - -- an Authorizer which implements access control for the Druid metastore against Apache Ranger +This Apache Druid extension adds an Authorizer which implements access control for Druid, backed by [Apache Ranger](https://ranger.apache.org/). Please see [Authentication and Authorization](../../design/auth.md) for more information on the basic facilities this extension provides. Make sure to [include](../../development/extensions.md#loading-extensions) `druid-ranger-security` as an extension. -Please see [Authentication and Authorization](../../design/auth.md) for more information on the extension interfaces being implemented. - -**NOTE** - -The latest release of Apache Ranger is at the time of writing version 2.0. This version has a dependency -on `log4j 1.2.17` which has a vulnerability if you configure it to use a `SocketServer` (CVE-2019-17571). Next to that, -it also includes Kafka 2.0.0 which has 2 known vulnerabilities (CVE-2019-12399, CVE-2018-17196). Kafka can be used -by the audit component in Ranger, but is not required. - +> The latest release of Apache Ranger is at the time of writing version 2.0. This version has a dependency on `log4j 1.2.17` which has a vulnerability if you configure it to use a `SocketServer` (CVE-2019-17571). Next to that, it also includes Kafka 2.0.0 which has 2 known vulnerabilities (CVE-2019-12399, CVE-2018-17196). Kafka can be used by the audit component in Ranger, but is not required. ## Configuration -Support for Apache Ranger authorization consists of three elements: configuration of the extension -in Apache Druid, configuring the connection to Apache Ranger and providing the service definition for Druid to Apache Ranger. +Support for Apache Ranger authorization consists of three elements: +* configuring the extension in Apache Druid +* configuring the connection to Apache Ranger +* providing the service definition for Druid to Apache Ranger Review comment: super nit: deep link to the sections below This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] maytasm commented on a change in pull request #10120: Fix Stack overflow with infinite loop in ReduceExpressionsRule of HepProgram
maytasm commented on a change in pull request #10120: URL: https://github.com/apache/druid/pull/10120#discussion_r448587442 ## File path: sql/src/main/java/org/apache/druid/sql/calcite/planner/Rules.java ## @@ -85,6 +88,13 @@ public static final int DRUID_CONVENTION_RULES = 0; public static final int BINDABLE_CONVENTION_RULES = 1; + // Due to Calcite bug (CALCITE-3845), ReduceExpressionsRule can considered expression which is the same as the + // previous input expression as reduced. Basically, the expression is actually not reduced but is still considered as + // reduced. Hence, this resulted in an infinite loop of Calcite trying to reducing the same expression over and over. + // Calcite 1.23.0 fixes this issue by not consider expression as reduced if this case happens. However, while + // we are still using Calcite 1.21.0, a workaround is to limit the number of pattern matches to avoid infinite loop. + private static final int HEP_DEFAULT_MATCH_LIMIT = 1200; Review comment: Added a override system property `druid.sql.planner.hepMatchLimit` that can override the default value of 1200 This system property is not documented though as it should not be modify by user without deep investigation into Calcite/HepProgram use by Druid. Assuming you are doing investigation on determining new value for matchLimit in Druid code, then you will see this system property anyway. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] suneet-s commented on a change in pull request #10120: Fix Stack overflow with infinite loop in ReduceExpressionsRule of HepProgram
suneet-s commented on a change in pull request #10120: URL: https://github.com/apache/druid/pull/10120#discussion_r448570647 ## File path: sql/src/main/java/org/apache/druid/sql/calcite/planner/Rules.java ## @@ -85,6 +88,13 @@ public static final int DRUID_CONVENTION_RULES = 0; public static final int BINDABLE_CONVENTION_RULES = 1; + // Due to Calcite bug (CALCITE-3845), ReduceExpressionsRule can considered expression which is the same as the + // previous input expression as reduced. Basically, the expression is actually not reduced but is still considered as + // reduced. Hence, this resulted in an infinite loop of Calcite trying to reducing the same expression over and over. + // Calcite 1.23.0 fixes this issue by not consider expression as reduced if this case happens. However, while + // we are still using Calcite 1.21.0, a workaround is to limit the number of pattern matches to avoid infinite loop. + private static final int HEP_DEFAULT_MATCH_LIMIT = 1200; Review comment: Is it possible to make this a system property? Having to restart the broker sounds like a lot less pain than having to make a new patch if this estimated `HEP_DEFAULT_MATCH_LIMIT` is incorrect for some edge case that we don't have tests for. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] maytasm closed pull request #10114: Add integration tests for CSV InputFormat
maytasm closed pull request #10114: URL: https://github.com/apache/druid/pull/10114 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] lgtm-com[bot] commented on pull request #10097: Do not echo back username on auth failure
lgtm-com[bot] commented on pull request #10097: URL: https://github.com/apache/druid/pull/10097#issuecomment-652601110 This pull request **fixes 1 alert** when merging a08f140cd9c07f7116173bf987d4a535b3bc95ae into d3497a6581c69f810090f5180f1f18328b06c781 - [view on LGTM.com](https://lgtm.com/projects/g/apache/druid/rev/pr-344f5b6eb881f4ffba1ad3995c1c7a9f3a9972f5) **fixed alerts:** * 1 for Cross\-site scripting This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] samarthjain opened a new pull request #10121: Revert "Fix UnknownTypeComplexColumn#makeVectorObjectSelector"
samarthjain opened a new pull request #10121: URL: https://github.com/apache/druid/pull/10121 This reverts commit 7bb7489afc7a2cc496be93ae69681b6ab13a7c66. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] suneet-s commented on pull request #10107: update links datasketches.github.io to datasketches.apache.org
suneet-s commented on pull request #10107: URL: https://github.com/apache/druid/pull/10107#issuecomment-652598589 It looks like travis passed, but the trigger didn't change the CI to green. I re-triggered the `animal-sniffer-checker` job hopefully it will mark this as passed soon This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] maytasm opened a new pull request #10120: Fix Stack overflow with infinite loop in ReduceExpressionsRule of HepProgram
maytasm opened a new pull request #10120: URL: https://github.com/apache/druid/pull/10120 Fix Stack overflow with infinite loop in ReduceExpressionsRule of HepProgram ### Description This fix https://github.com/apache/druid/issues/9906 Due to Calcite bug (CALCITE-3845), ReduceExpressionsRule can considered expression which is the same as the previous input expression as reduced. Basically, the expression is actually not reduced but is still considered as reduced. Hence, this resulted in an infinite loop of Calcite trying to reducing the same expression over and over in ReduceExpressionsRule. Calcite 1.23.0 fixes this issue by not consider expression as reduced if this case happens. However, while we are still using Calcite 1.21.0, a workaround is to limit the number of pattern matches to avoid infinite loop in ReduceExpressionsRule of the HepProgram. One example of this case is the query `SELECT ARRAY ['Hello', NULL]` The matchLimit value of 1200 is chosen with the suggestion by a Calcite committer. This PR has: - [x] been self-reviewed. - [ ] added documentation for new or modified features or behaviors. - [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links. - [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/licenses.yaml) - [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader. - [x] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met. - [ ] added integration tests. - [ ] been tested in a test Druid cluster. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[druid] branch master updated (d3497a6 -> 7bb7489)
This is an automated email from the ASF dual-hosted git repository. samarth pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/druid.git. from d3497a6 Filter on metrics doc (#10087) add 7bb7489 Fix UnknownTypeComplexColumn#makeVectorObjectSelector No new revisions were added by this update. Summary of changes: .../segment/column/UnknownTypeComplexColumn.java | 19 +-- 1 file changed, 1 insertion(+), 18 deletions(-) - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] clintropolis commented on a change in pull request #9422: Handle unknown complex types
clintropolis commented on a change in pull request #9422:
URL: https://github.com/apache/druid/pull/9422#discussion_r448557485
##
File path:
processing/src/main/java/org/apache/druid/segment/column/UnknownTypeComplexColumn.java
##
@@ -0,0 +1,99 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.segment.column;
+
+import org.apache.druid.segment.ColumnValueSelector;
+import org.apache.druid.segment.NilColumnValueSelector;
+import org.apache.druid.segment.data.ReadableOffset;
+import org.apache.druid.segment.vector.NilVectorSelector;
+import org.apache.druid.segment.vector.ReadableVectorOffset;
+import org.apache.druid.segment.vector.VectorObjectSelector;
+import org.apache.druid.segment.vector.VectorSizeInspector;
+
+import javax.annotation.Nullable;
+
+public class UnknownTypeComplexColumn implements ComplexColumn
+{
+ private static final UnknownTypeComplexColumn INSTANCE = new
UnknownTypeComplexColumn();
+
+ public static UnknownTypeComplexColumn instance()
+ {
+return INSTANCE;
+ }
+
+ private static final NilVectorSelector NIL_VECTOR_SELECTOR_INSTANCE =
+ NilVectorSelector.create(new VectorSizeInspector()
+ {
+@Override
+public int getMaxVectorSize()
+{
+ return 0;
+}
+
+@Override
+public int getCurrentVectorSize()
+{
+ return 0;
+}
+ });
+
+ @Override
+ public Class getClazz()
+ {
+return ComplexColumn.class;
+ }
+
+ @Override
+ public String getTypeName()
+ {
+return "UNKNOWN_COMPLEX_COLUMN_TYPE";
+ }
+
+ @Nullable
+ @Override
+ public Object getRowValue(int rowNum)
+ {
+return null;
+ }
+
+ @Override
+ public int getLength()
+ {
+return 0;
+ }
+
+ @Override
+ public void close()
+ {
+
+ }
+
+ @Override
+ public ColumnValueSelector makeColumnValueSelector(ReadableOffset offset)
+ {
+return NilColumnValueSelector.instance();
Review comment:
Yeah, it wouldn't really tell us which column is invalid, just that it
is happening I think. Up to you if you want to add the log, I'm not totally
sure its useful was just thinking out loud.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] samarthjain commented on a change in pull request #9422: Handle unknown complex types
samarthjain commented on a change in pull request #9422:
URL: https://github.com/apache/druid/pull/9422#discussion_r448544157
##
File path:
processing/src/main/java/org/apache/druid/segment/column/UnknownTypeComplexColumn.java
##
@@ -0,0 +1,99 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.segment.column;
+
+import org.apache.druid.segment.ColumnValueSelector;
+import org.apache.druid.segment.NilColumnValueSelector;
+import org.apache.druid.segment.data.ReadableOffset;
+import org.apache.druid.segment.vector.NilVectorSelector;
+import org.apache.druid.segment.vector.ReadableVectorOffset;
+import org.apache.druid.segment.vector.VectorObjectSelector;
+import org.apache.druid.segment.vector.VectorSizeInspector;
+
+import javax.annotation.Nullable;
+
+public class UnknownTypeComplexColumn implements ComplexColumn
+{
+ private static final UnknownTypeComplexColumn INSTANCE = new
UnknownTypeComplexColumn();
+
+ public static UnknownTypeComplexColumn instance()
+ {
+return INSTANCE;
+ }
+
+ private static final NilVectorSelector NIL_VECTOR_SELECTOR_INSTANCE =
+ NilVectorSelector.create(new VectorSizeInspector()
+ {
+@Override
+public int getMaxVectorSize()
+{
+ return 0;
+}
+
+@Override
+public int getCurrentVectorSize()
+{
+ return 0;
+}
+ });
+
+ @Override
+ public Class getClazz()
+ {
+return ComplexColumn.class;
+ }
+
+ @Override
+ public String getTypeName()
+ {
+return "UNKNOWN_COMPLEX_COLUMN_TYPE";
+ }
+
+ @Nullable
+ @Override
+ public Object getRowValue(int rowNum)
+ {
+return null;
+ }
+
+ @Override
+ public int getLength()
+ {
+return 0;
+ }
+
+ @Override
+ public void close()
+ {
+
+ }
+
+ @Override
+ public ColumnValueSelector makeColumnValueSelector(ReadableOffset offset)
+ {
+return NilColumnValueSelector.instance();
Review comment:
While it wouldn't be too noisy, it made me think will it give us enough
information to understand which column is actually invalid now?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] samarthjain commented on a change in pull request #9422: Handle unknown complex types
samarthjain commented on a change in pull request #9422:
URL: https://github.com/apache/druid/pull/9422#discussion_r448544343
##
File path:
processing/src/main/java/org/apache/druid/segment/column/UnknownTypeComplexColumn.java
##
@@ -0,0 +1,99 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.segment.column;
+
+import org.apache.druid.segment.ColumnValueSelector;
+import org.apache.druid.segment.NilColumnValueSelector;
+import org.apache.druid.segment.data.ReadableOffset;
+import org.apache.druid.segment.vector.NilVectorSelector;
+import org.apache.druid.segment.vector.ReadableVectorOffset;
+import org.apache.druid.segment.vector.VectorObjectSelector;
+import org.apache.druid.segment.vector.VectorSizeInspector;
+
+import javax.annotation.Nullable;
+
+public class UnknownTypeComplexColumn implements ComplexColumn
+{
+ private static final UnknownTypeComplexColumn INSTANCE = new
UnknownTypeComplexColumn();
+
+ public static UnknownTypeComplexColumn instance()
+ {
+return INSTANCE;
+ }
+
+ private static final NilVectorSelector NIL_VECTOR_SELECTOR_INSTANCE =
+ NilVectorSelector.create(new VectorSizeInspector()
+ {
+@Override
+public int getMaxVectorSize()
+{
+ return 0;
+}
+
+@Override
+public int getCurrentVectorSize()
+{
+ return 0;
+}
+ });
+
+ @Override
+ public Class getClazz()
+ {
+return ComplexColumn.class;
+ }
+
+ @Override
+ public String getTypeName()
+ {
+return "UNKNOWN_COMPLEX_COLUMN_TYPE";
+ }
+
+ @Nullable
+ @Override
+ public Object getRowValue(int rowNum)
+ {
+return null;
+ }
+
+ @Override
+ public int getLength()
+ {
+return 0;
+ }
+
+ @Override
+ public void close()
+ {
+
+ }
+
+ @Override
+ public ColumnValueSelector makeColumnValueSelector(ReadableOffset offset)
+ {
+return NilColumnValueSelector.instance();
+ }
+
+ @Override
+ public VectorObjectSelector makeVectorObjectSelector(ReadableVectorOffset
offset)
+ {
+return NIL_VECTOR_SELECTOR_INSTANCE;
Review comment:
Thanks!
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] samarthjain commented on a change in pull request #9422: Handle unknown complex types
samarthjain commented on a change in pull request #9422:
URL: https://github.com/apache/druid/pull/9422#discussion_r448544157
##
File path:
processing/src/main/java/org/apache/druid/segment/column/UnknownTypeComplexColumn.java
##
@@ -0,0 +1,99 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.segment.column;
+
+import org.apache.druid.segment.ColumnValueSelector;
+import org.apache.druid.segment.NilColumnValueSelector;
+import org.apache.druid.segment.data.ReadableOffset;
+import org.apache.druid.segment.vector.NilVectorSelector;
+import org.apache.druid.segment.vector.ReadableVectorOffset;
+import org.apache.druid.segment.vector.VectorObjectSelector;
+import org.apache.druid.segment.vector.VectorSizeInspector;
+
+import javax.annotation.Nullable;
+
+public class UnknownTypeComplexColumn implements ComplexColumn
+{
+ private static final UnknownTypeComplexColumn INSTANCE = new
UnknownTypeComplexColumn();
+
+ public static UnknownTypeComplexColumn instance()
+ {
+return INSTANCE;
+ }
+
+ private static final NilVectorSelector NIL_VECTOR_SELECTOR_INSTANCE =
+ NilVectorSelector.create(new VectorSizeInspector()
+ {
+@Override
+public int getMaxVectorSize()
+{
+ return 0;
+}
+
+@Override
+public int getCurrentVectorSize()
+{
+ return 0;
+}
+ });
+
+ @Override
+ public Class getClazz()
+ {
+return ComplexColumn.class;
+ }
+
+ @Override
+ public String getTypeName()
+ {
+return "UNKNOWN_COMPLEX_COLUMN_TYPE";
+ }
+
+ @Nullable
+ @Override
+ public Object getRowValue(int rowNum)
+ {
+return null;
+ }
+
+ @Override
+ public int getLength()
+ {
+return 0;
+ }
+
+ @Override
+ public void close()
+ {
+
+ }
+
+ @Override
+ public ColumnValueSelector makeColumnValueSelector(ReadableOffset offset)
+ {
+return NilColumnValueSelector.instance();
Review comment:
I thought about it. While it wouldn't be too noisy, it made me think
will it give us enough information to understand which column is actually
invalid now?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] gianm commented on pull request #10082: Fix RetryQueryRunner to actually do the job
gianm commented on pull request #10082: URL: https://github.com/apache/druid/pull/10082#issuecomment-652576873 > @gianm thanks for the review. As Travis is extremely slow recently, I'd like to address your last comments in a follow-up with doc for the known issue with response context not shared by subqueries. That works for me. I think the follow-ups we'd want before the next release would be: 1. Add integration tests. 2. Add the new error code to the documentation: https://github.com/apache/druid/pull/10082#discussion_r448525940 3. Modify this method name: https://github.com/apache/druid/pull/10082#discussion_r448526783 For documenting the limitations of response context, I'm actually not sure what we should do, because I realized that currently they aren't documented at all. They are a secret feature, I suppose. So I don't think there's any need to document their limitations if we aren't including it as a documented feature in the first place. I think there are two good options here: - Do nothing, don't add any docs. - Document response contexts as an 'alpha' or 'beta' level feature, and include documentation about their limitations (they can be too long and will either truncate or fail the query; and they aren't currently collected from all subqueries). This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] jihoonson commented on pull request #10082: Fix RetryQueryRunner to actually do the job
jihoonson commented on pull request #10082: URL: https://github.com/apache/druid/pull/10082#issuecomment-652572586 @gianm thanks for the review. As Travis is extremely slow recently, I'd like to address your last comments in a follow-up with doc for the known issue with response context not shared by subqueries. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] gianm commented on pull request #10082: Fix RetryQueryRunner to actually do the job
gianm commented on pull request #10082: URL: https://github.com/apache/druid/pull/10082#issuecomment-652569145 Btw, if you can do the above two things (and add integration tests) in follow-ups before the next release, I think that would be OK. Let us know what you prefer @jihoonson. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] gianm commented on a change in pull request #10082: Fix RetryQueryRunner to actually do the job
gianm commented on a change in pull request #10082:
URL: https://github.com/apache/druid/pull/10082#discussion_r448525940
##
File path:
processing/src/main/java/org/apache/druid/query/QueryInterruptedException.java
##
@@ -105,6 +106,8 @@ private static String getErrorCodeFromThrowable(Throwable e)
return RESOURCE_LIMIT_EXCEEDED;
} else if (e instanceof UnsupportedOperationException) {
return UNSUPPORTED_OPERATION;
+} else if (e instanceof TruncatedResponseContextException) {
+ return TRUNCATED_RESPONSE_CONTEXT;
Review comment:
This should be added to the documentation (all of these error codes are
spelled out in a table in `querying/querying.md`).
##
File path: processing/src/main/java/org/apache/druid/query/QueryContexts.java
##
@@ -344,6 +346,19 @@ public String toString()
return defaultTimeout;
}
+ public static Query setFailOnTruncatedResponseContext(Query query)
Review comment:
Should be `withFailOnTruncatedResponseContext`, not
`setFailOnTruncatedResponseContext`, because nothing's being set (a modified
copy is being returned).
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] damnMeddlingKid commented on issue #10072: Average aggregate does not ignore nulls in long column when druid.generic.useDefaultValueForNull=false
damnMeddlingKid commented on issue #10072: URL: https://github.com/apache/druid/issues/10072#issuecomment-652560849 Thanks Gian, I'll refactor it and put up a PR. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] gianm edited a comment on issue #10072: Average aggregate does not ignore nulls in long column when druid.generic.useDefaultValueForNull=false
gianm edited a comment on issue #10072: URL: https://github.com/apache/druid/issues/10072#issuecomment-652550179 Ah, it's possible both cases (nullable and non-nullable) would be covered by existing tests, but if not, please add enough tests to cover both to CalciteQueryTest. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] gianm commented on issue #10072: Average aggregate does not ignore nulls in long column when druid.generic.useDefaultValueForNull=false
gianm commented on issue #10072: URL: https://github.com/apache/druid/issues/10072#issuecomment-652550179 It's possible both cases (nullable and non-nullable) would be covered by existing tests, but if not, please add enough tests to cover both to CalciteQueryTest. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] gianm commented on issue #10072: Average aggregate does not ignore nulls in long column when druid.generic.useDefaultValueForNull=false
gianm commented on issue #10072: URL: https://github.com/apache/druid/issues/10072#issuecomment-652548762 Hi @damnMeddlingKid, Looks like this is a bug and you got the right fix. The logic is similar to what's in CountSqlAggregator already, so I'd suggest extracting it to a helper function, for example, `CountAggregatorFactory.createFieldCountAggregatorFactory(PlannerContext, RowSignature, VirtualColumnRegistry, RexBuilder, RexNode)`. Then we could call that helper in AvgSqlAggregator as well. It would be similar to how it already calls out to `SumSqlAggregator.createSumAggregatorFactory`. If you raised a PR doing this it would be appreciated! Please feel free to add me as a reviewer on it or at-mention me in a comment. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] suneet-s closed pull request #10106: Add validation for authorizer name
suneet-s closed pull request #10106: URL: https://github.com/apache/druid/pull/10106 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] gianm opened a new pull request #10119: Combine InDimFilter, InFilter.
gianm opened a new pull request #10119: URL: https://github.com/apache/druid/pull/10119 There are two motivations: 1. Ensure that when HashJoinSegmentStorageAdapter compares its Filter to the original one, and it is an "in" type, the comparison is by reference and does not need to check deep equality. This is useful when the "in" filter is very large. 2. Simplify things. (There isn't a great reason for the DimFilter and Filter logic to be separate, and combining them reduces some duplication.) This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] lgtm-com[bot] commented on pull request #10097: Do not echo back username on auth failure
lgtm-com[bot] commented on pull request #10097: URL: https://github.com/apache/druid/pull/10097#issuecomment-652489374 This pull request **fixes 1 alert** when merging 01999dae21e5d60fa5cf9344e6d0a0a30c95ace6 into d3497a6581c69f810090f5180f1f18328b06c781 - [view on LGTM.com](https://lgtm.com/projects/g/apache/druid/rev/pr-266ed1ec3397ce70a967cf345397f40d02dd) **fixed alerts:** * 1 for Cross\-site scripting This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] xiangqiao123 opened a new issue #10118: DruidSchema is not thread safe
xiangqiao123 opened a new issue #10118: URL: https://github.com/apache/druid/issues/10118 ### Affected Version 0.18.1 ### Description Suppose that an interval has more than one segment, if some segments are deleted, then refreshing the remaining segments by refreshing segments will be skipped and re added to the segmentsNeedingRefresh. As a result, these remaining segments can no longer be refreshed successfully(PartitionHolders are not complete). This can easily happen when the broker is restarted. We can see the following logs: `Refreshed metadata for dataSource[test_datasource] in 156 ms (97 segments queried, 206 segments left).` Suppose the code is executed as follows 1.removeSegment：segmentsNeedingRefresh.remove(segment) （delete partial segment for a interval） 2.segmentsToRefresh.addAll(segmentsNeedingRefresh); （add remaining segment to refresh） segmentsNeedingRefresh.clear(); (clear segmentsNeedingRefresh) 3.final Set refreshed = refreshSegments(segmentsToRefresh); 4.segmentsNeedingRefresh.addAll(Sets.difference(segmentsToRefresh, refreshed)); (remaining segment add to segmentsNeedingRefresh again,and can no longer refresh successfully) @clintropolis Can you help me confirm that this is a bug? thank you！ This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] a2l007 commented on pull request #10080: Add integration tests for SqlInputSource
a2l007 commented on pull request #10080: URL: https://github.com/apache/druid/pull/10080#issuecomment-652427533 Thank you for adding this! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] jihoonson commented on a change in pull request #10082: Fix RetryQueryRunner to actually do the job
jihoonson commented on a change in pull request #10082:
URL: https://github.com/apache/druid/pull/10082#discussion_r448229976
##
File path: server/src/main/java/org/apache/druid/query/RetryQueryRunner.java
##
@@ -0,0 +1,250 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.query;
+
+import com.fasterxml.jackson.core.type.TypeReference;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
+import org.apache.druid.java.util.common.ISE;
+import org.apache.druid.java.util.common.guava.BaseSequence;
+import org.apache.druid.java.util.common.guava.BaseSequence.IteratorMaker;
+import org.apache.druid.java.util.common.guava.MergeSequence;
+import org.apache.druid.java.util.common.guava.Sequence;
+import org.apache.druid.java.util.common.guava.Yielder;
+import org.apache.druid.java.util.common.guava.YieldingAccumulator;
+import org.apache.druid.java.util.common.guava.YieldingSequenceBase;
+import org.apache.druid.java.util.common.logger.Logger;
+import org.apache.druid.query.context.ResponseContext;
+import org.apache.druid.query.context.ResponseContext.Key;
+import org.apache.druid.segment.SegmentMissingException;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.BiFunction;
+
+public class RetryQueryRunner implements QueryRunner
+{
+ private static final Logger LOG = new Logger(RetryQueryRunner.class);
+
+ private final QueryRunner baseRunner;
+ private final BiFunction, List, QueryRunner>
retryRunnerCreateFn;
+ private final RetryQueryRunnerConfig config;
+ private final ObjectMapper jsonMapper;
+
+ /**
+ * Runnable executed after the broker creates query distribution tree for
the first attempt. This is only
+ * for testing and must not be used in production code.
+ */
+ private final Runnable runnableAfterFirstAttempt;
+
+ private int totalNumRetries;
+
+ public RetryQueryRunner(
+ QueryRunner baseRunner,
+ BiFunction, List, QueryRunner>
retryRunnerCreateFn,
+ RetryQueryRunnerConfig config,
+ ObjectMapper jsonMapper
+ )
+ {
+this(baseRunner, retryRunnerCreateFn, config, jsonMapper, () -> {});
+ }
+
+ /**
+ * Constructor only for testing.
+ */
+ @VisibleForTesting
+ RetryQueryRunner(
+ QueryRunner baseRunner,
+ BiFunction, List, QueryRunner>
retryRunnerCreateFn,
+ RetryQueryRunnerConfig config,
+ ObjectMapper jsonMapper,
+ Runnable runnableAfterFirstAttempt
+ )
+ {
+this.baseRunner = baseRunner;
+this.retryRunnerCreateFn = retryRunnerCreateFn;
+this.config = config;
+this.jsonMapper = jsonMapper;
+this.runnableAfterFirstAttempt = runnableAfterFirstAttempt;
+ }
+
+ @VisibleForTesting
+ int getTotalNumRetries()
+ {
+return totalNumRetries;
+ }
+
+ @Override
+ public Sequence run(final QueryPlus queryPlus, final ResponseContext
context)
+ {
+return new YieldingSequenceBase()
+{
+ @Override
+ public Yielder toYielder(OutType initValue,
YieldingAccumulator accumulator)
+ {
+final Sequence> retryingSequence = new BaseSequence<>(
+new IteratorMaker, RetryingSequenceIterator>()
+{
+ @Override
+ public RetryingSequenceIterator make()
+ {
+return new RetryingSequenceIterator(queryPlus, context,
baseRunner, runnableAfterFirstAttempt);
+ }
+
+ @Override
+ public void cleanup(RetryingSequenceIterator iterFromMake)
+ {
+totalNumRetries = iterFromMake.retryCount;
+ }
+}
+);
+return new MergeSequence<>(queryPlus.getQuery().getResultOrdering(),
retryingSequence)
+.toYielder(initValue, accumulator);
+ }
+};
+ }
+
+ private List getMissingSegments(QueryPlus queryPlus,
final ResponseContext context)
Review comment:
Added a flag and made the historicals fail when response context i
[GitHub] [druid] jihoonson commented on a change in pull request #10082: Fix RetryQueryRunner to actually do the job
jihoonson commented on a change in pull request #10082:
URL: https://github.com/apache/druid/pull/10082#discussion_r448229976
##
File path: server/src/main/java/org/apache/druid/query/RetryQueryRunner.java
##
@@ -0,0 +1,250 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.query;
+
+import com.fasterxml.jackson.core.type.TypeReference;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
+import org.apache.druid.java.util.common.ISE;
+import org.apache.druid.java.util.common.guava.BaseSequence;
+import org.apache.druid.java.util.common.guava.BaseSequence.IteratorMaker;
+import org.apache.druid.java.util.common.guava.MergeSequence;
+import org.apache.druid.java.util.common.guava.Sequence;
+import org.apache.druid.java.util.common.guava.Yielder;
+import org.apache.druid.java.util.common.guava.YieldingAccumulator;
+import org.apache.druid.java.util.common.guava.YieldingSequenceBase;
+import org.apache.druid.java.util.common.logger.Logger;
+import org.apache.druid.query.context.ResponseContext;
+import org.apache.druid.query.context.ResponseContext.Key;
+import org.apache.druid.segment.SegmentMissingException;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.BiFunction;
+
+public class RetryQueryRunner implements QueryRunner
+{
+ private static final Logger LOG = new Logger(RetryQueryRunner.class);
+
+ private final QueryRunner baseRunner;
+ private final BiFunction, List, QueryRunner>
retryRunnerCreateFn;
+ private final RetryQueryRunnerConfig config;
+ private final ObjectMapper jsonMapper;
+
+ /**
+ * Runnable executed after the broker creates query distribution tree for
the first attempt. This is only
+ * for testing and must not be used in production code.
+ */
+ private final Runnable runnableAfterFirstAttempt;
+
+ private int totalNumRetries;
+
+ public RetryQueryRunner(
+ QueryRunner baseRunner,
+ BiFunction, List, QueryRunner>
retryRunnerCreateFn,
+ RetryQueryRunnerConfig config,
+ ObjectMapper jsonMapper
+ )
+ {
+this(baseRunner, retryRunnerCreateFn, config, jsonMapper, () -> {});
+ }
+
+ /**
+ * Constructor only for testing.
+ */
+ @VisibleForTesting
+ RetryQueryRunner(
+ QueryRunner baseRunner,
+ BiFunction, List, QueryRunner>
retryRunnerCreateFn,
+ RetryQueryRunnerConfig config,
+ ObjectMapper jsonMapper,
+ Runnable runnableAfterFirstAttempt
+ )
+ {
+this.baseRunner = baseRunner;
+this.retryRunnerCreateFn = retryRunnerCreateFn;
+this.config = config;
+this.jsonMapper = jsonMapper;
+this.runnableAfterFirstAttempt = runnableAfterFirstAttempt;
+ }
+
+ @VisibleForTesting
+ int getTotalNumRetries()
+ {
+return totalNumRetries;
+ }
+
+ @Override
+ public Sequence run(final QueryPlus queryPlus, final ResponseContext
context)
+ {
+return new YieldingSequenceBase()
+{
+ @Override
+ public Yielder toYielder(OutType initValue,
YieldingAccumulator accumulator)
+ {
+final Sequence> retryingSequence = new BaseSequence<>(
+new IteratorMaker, RetryingSequenceIterator>()
+{
+ @Override
+ public RetryingSequenceIterator make()
+ {
+return new RetryingSequenceIterator(queryPlus, context,
baseRunner, runnableAfterFirstAttempt);
+ }
+
+ @Override
+ public void cleanup(RetryingSequenceIterator iterFromMake)
+ {
+totalNumRetries = iterFromMake.retryCount;
+ }
+}
+);
+return new MergeSequence<>(queryPlus.getQuery().getResultOrdering(),
retryingSequence)
+.toYielder(initValue, accumulator);
+ }
+};
+ }
+
+ private List getMissingSegments(QueryPlus queryPlus,
final ResponseContext context)
Review comment:
Added a flag and made the historicals fail when response context i
[GitHub] [druid] jp707049 commented on issue #10078: How to use environment variables in runtime.properties?
jp707049 commented on issue #10078: URL: https://github.com/apache/druid/issues/10078#issuecomment-652285832 Great, I will check and update here soon using your first method. Thanks :) This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] FrankChen021 commented on issue #10078: How to use environment variables in runtime.properties?
FrankChen021 commented on issue #10078: URL: https://github.com/apache/druid/issues/10078#issuecomment-652251590 I checked the `run-druid` file, putting environment variables in `jvm.config` won't take effect. But you can try the first method I mention above, that is putting `-Ddruid.host=$DRUID_HOST` in [`run-druid`] (https://github.com/apache/druid/blob/d3497a6581c69f810090f5180f1f18328b06c781/examples/bin/run-druid#L45) as below exec "$JAVA_BIN"/java -Ddruid.host=$DRUID_HOST ... This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] clintropolis opened a new pull request #10117: Closing yielder from ParallelMergeCombiningSequence should trigger cancellation
clintropolis opened a new pull request #10117: URL: https://github.com/apache/druid/pull/10117 ### Description This PR fixes an issue with/modifies the behavior of closing a `Yielder` created from a `ParallelMergeCombiningSequence` to trigger a cancellation to immediately halt any additional processing on the broker's merging `ForkJoinPool`. I'm unsure if this causes any issues in practice, but could be an issue if closing a yielder early. Closing the yielder and stopping processing would potentially cause running pool tasks associated with the query to managed block until the offer/poll timeouts on the blocking queue, at which point they would fully die. Amusingly, prior to the change in this PR, you could continue to use the "closed" yielder to get the complete set of results assuming no exceptions occur inside of the parallel merge processing itself, however the sequence 'baggage' that reports the parallel merge pool metrics would have been triggered early, reporting incorrect metric values. The added test ensures that cancellation happens upon close of the `Yielder`, which is valid behavior within the contract of the `Yielder` interface which allows for close to put the entire chain into an undefined state. This PR has: - [x] been self-reviewed. - [x] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.) - [x] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader. - [x] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met. - [ ] added integration tests. - [ ] been tested in a test Druid cluster. # Key changed/added classes in this PR * `ParallelMergeCombiningSequence` This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] maytasm opened a new pull request #10116: Add integration tests for parquet InputFormat
maytasm opened a new pull request #10116: URL: https://github.com/apache/druid/pull/10116 Add integration tests for parquet InputFormat ### Description Add integration tests for parquet InputFormat This PR has: - [x] been self-reviewed. - [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.) - [ ] added documentation for new or modified features or behaviors. - [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links. - [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/licenses.yaml) - [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader. - [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met. - [x] added integration tests. - [ ] been tested in a test Druid cluster. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[GitHub] [druid] maytasm opened a new pull request #10115: Add integration tests for ORC InputFormat
maytasm opened a new pull request #10115: URL: https://github.com/apache/druid/pull/10115 Add integration tests for ORC InputFormat ### Description Add integration tests for ORC InputFormat This PR has: - [x] been self-reviewed. - [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.) - [ ] added documentation for new or modified features or behaviors. - [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links. - [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/licenses.yaml) - [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader. - [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met. - [x] added integration tests. - [ ] been tested in a test Druid cluster. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
