[GitHub] [incubator-druid] leventov commented on issue #8558: 7227 : Prohibit Non Final Static Field

[GitBox]

leventov commented on issue #8558: 7227 : Prohibit Non Final Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#issuecomment-542562434
 
 
   There are 7 related errors:
   
![image](https://user-images.githubusercontent.com/609240/66897260-4849b600-efff-11e9-8bd9-d77eb3699bad.png)
   
   
https://teamcity.jetbrains.com/viewLog.html?buildId=2590045&tab=Inspection&buildTypeId=OpenSourceProjects_Druid_InspectionsPullRequests
   


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] leventov commented on issue #8558: 7227 : Prohibit Non Final Static Field

[GitBox]

leventov commented on issue #8558: 7227 : Prohibit Non Final Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#issuecomment-542562851
 
 
   @SandishKumarHN can you see these errors yourself 
[here](https://teamcity.jetbrains.com/viewLog.html?buildId=2590045&tab=Inspection&buildTypeId=OpenSourceProjects_Druid_InspectionsPullRequests),
 or there is some sort of access problem that prevents your (a non-admin user) 
from seeing these problems?


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] denever commented on issue #8656: Message rejection absolute date

[GitBox]

denever commented on issue #8656: Message rejection absolute date
URL: https://github.com/apache/incubator-druid/pull/8656#issuecomment-542636264
 
 
   > thanks, can you also update the necessary documentation and mention the 
precedence of `lateMessageRejectionStartDateTime` vs 
`lateMessageRejectionPeriod` or maybe there should be some check ensuring user 
doesn't specify both as that is most likely an user error.
   
   I've update documentation and mentioned the precedence


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] clintropolis commented on a change in pull request #8578: parallel broker merges on fork join pool

[GitBox]

clintropolis commented on a change in pull request #8578: parallel broker 
merges on fork join pool
URL: https://github.com/apache/incubator-druid/pull/8578#discussion_r335410227
 
 

 ##
 File path: 
core/src/main/java/org/apache/druid/java/util/common/guava/ParallelMergeCombiningSequence.java
 ##
 @@ -0,0 +1,1071 @@
+/*
+ * 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.java.util.common.guava;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Ordering;
+import org.apache.druid.java.util.common.RE;
+import org.apache.druid.java.util.common.logger.Logger;
+import org.apache.druid.utils.JvmUtils;
+
+import javax.annotation.Nullable;
+import java.io.IOException;
+import java.util.ArrayDeque;
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+import java.util.PriorityQueue;
+import java.util.Queue;
+import java.util.concurrent.ArrayBlockingQueue;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.ForkJoinPool;
+import java.util.concurrent.RecursiveAction;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.BinaryOperator;
+
+/**
+ * Artisanal, locally-sourced, hand-crafted, gluten and GMO free, bespoke, 
small-batch parallel merge combinining sequence
+ */
+public class ParallelMergeCombiningSequence extends YieldingSequenceBase
+{
+  private static final Logger LOG = new 
Logger(ParallelMergeCombiningSequence.class);
+
+  private final ForkJoinPool workerPool;
+  private final List> baseSequences;
+  private final Ordering orderingFn;
+  private final BinaryOperator combineFn;
+  private final int queueSize;
+  private final boolean hasTimeout;
+  private final long timeoutAtNanos;
+  private final int queryPriority; // not currently used :(
+  private final int yieldAfter;
+  private final int batchSize;
+  private final int parallelism;
+  private final CancellationGizmo cancellationGizmo;
+
+  public ParallelMergeCombiningSequence(
+  ForkJoinPool workerPool,
+  List> baseSequences,
+  Ordering orderingFn,
+  BinaryOperator combineFn,
+  boolean hasTimeout,
+  long timeoutMillis,
+  int queryPriority,
+  int parallelism,
+  int yieldAfter,
+  int batchSize
+  )
+  {
+this.workerPool = workerPool;
+this.baseSequences = baseSequences;
+this.orderingFn = orderingFn;
+this.combineFn = combineFn;
+this.hasTimeout = hasTimeout;
+this.timeoutAtNanos = System.nanoTime() + 
TimeUnit.NANOSECONDS.convert(timeoutMillis, TimeUnit.MILLISECONDS);
+this.queryPriority = queryPriority;
+this.parallelism = parallelism;
+this.yieldAfter = yieldAfter;
+this.batchSize = batchSize;
+this.queueSize = 4 * (yieldAfter / batchSize);
+this.cancellationGizmo = new CancellationGizmo();
+  }
+
+  @Override
+  public  Yielder toYielder(OutType initValue, 
YieldingAccumulator accumulator)
+  {
+if (baseSequences.isEmpty()) {
+  return Sequences.empty().toYielder(initValue, accumulator);
+}
+
+final BlockingQueue> outputQueue = new 
ArrayBlockingQueue<>(queueSize);
+MergeCombinePartitioningAction finalMergeAction = new 
MergeCombinePartitioningAction<>(
+baseSequences,
+orderingFn,
+combineFn,
+outputQueue,
+queueSize,
+parallelism,
+yieldAfter,
+batchSize,
+hasTimeout,
+timeoutAtNanos,
+cancellationGizmo
+);
+workerPool.execute(finalMergeAction);
+Sequence finalOutSequence = makeOutputSequenceForQueue(outputQueue, 
hasTimeout, timeoutAtNanos, cancellationGizmo);
+return finalOutSequence.toYielder(initValue, accumulator);
+  }
+
+  /**
+   * Create an output {@link Sequence} that wraps the output {@link 
BlockingQueue} of a
+   * {@link MergeCombinePartitioningAction}
+   */
+  static  Sequence makeOutputSequenceForQueue(
+  BlockingQueue> queue,
+  boolean hasTimeout,
+  long timeoutAtNanos,
+  CancellationGizmo cancellationGizmo
+  )
+  {
+return new B

[GitHub] [incubator-druid] clintropolis commented on a change in pull request #8578: parallel broker merges on fork join pool

[GitBox]

clintropolis commented on a change in pull request #8578: parallel broker 
merges on fork join pool
URL: https://github.com/apache/incubator-druid/pull/8578#discussion_r335410227
 
 

 ##
 File path: 
core/src/main/java/org/apache/druid/java/util/common/guava/ParallelMergeCombiningSequence.java
 ##
 @@ -0,0 +1,1071 @@
+/*
+ * 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.java.util.common.guava;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Ordering;
+import org.apache.druid.java.util.common.RE;
+import org.apache.druid.java.util.common.logger.Logger;
+import org.apache.druid.utils.JvmUtils;
+
+import javax.annotation.Nullable;
+import java.io.IOException;
+import java.util.ArrayDeque;
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+import java.util.PriorityQueue;
+import java.util.Queue;
+import java.util.concurrent.ArrayBlockingQueue;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.ForkJoinPool;
+import java.util.concurrent.RecursiveAction;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.BinaryOperator;
+
+/**
+ * Artisanal, locally-sourced, hand-crafted, gluten and GMO free, bespoke, 
small-batch parallel merge combinining sequence
+ */
+public class ParallelMergeCombiningSequence extends YieldingSequenceBase
+{
+  private static final Logger LOG = new 
Logger(ParallelMergeCombiningSequence.class);
+
+  private final ForkJoinPool workerPool;
+  private final List> baseSequences;
+  private final Ordering orderingFn;
+  private final BinaryOperator combineFn;
+  private final int queueSize;
+  private final boolean hasTimeout;
+  private final long timeoutAtNanos;
+  private final int queryPriority; // not currently used :(
+  private final int yieldAfter;
+  private final int batchSize;
+  private final int parallelism;
+  private final CancellationGizmo cancellationGizmo;
+
+  public ParallelMergeCombiningSequence(
+  ForkJoinPool workerPool,
+  List> baseSequences,
+  Ordering orderingFn,
+  BinaryOperator combineFn,
+  boolean hasTimeout,
+  long timeoutMillis,
+  int queryPriority,
+  int parallelism,
+  int yieldAfter,
+  int batchSize
+  )
+  {
+this.workerPool = workerPool;
+this.baseSequences = baseSequences;
+this.orderingFn = orderingFn;
+this.combineFn = combineFn;
+this.hasTimeout = hasTimeout;
+this.timeoutAtNanos = System.nanoTime() + 
TimeUnit.NANOSECONDS.convert(timeoutMillis, TimeUnit.MILLISECONDS);
+this.queryPriority = queryPriority;
+this.parallelism = parallelism;
+this.yieldAfter = yieldAfter;
+this.batchSize = batchSize;
+this.queueSize = 4 * (yieldAfter / batchSize);
+this.cancellationGizmo = new CancellationGizmo();
+  }
+
+  @Override
+  public  Yielder toYielder(OutType initValue, 
YieldingAccumulator accumulator)
+  {
+if (baseSequences.isEmpty()) {
+  return Sequences.empty().toYielder(initValue, accumulator);
+}
+
+final BlockingQueue> outputQueue = new 
ArrayBlockingQueue<>(queueSize);
+MergeCombinePartitioningAction finalMergeAction = new 
MergeCombinePartitioningAction<>(
+baseSequences,
+orderingFn,
+combineFn,
+outputQueue,
+queueSize,
+parallelism,
+yieldAfter,
+batchSize,
+hasTimeout,
+timeoutAtNanos,
+cancellationGizmo
+);
+workerPool.execute(finalMergeAction);
+Sequence finalOutSequence = makeOutputSequenceForQueue(outputQueue, 
hasTimeout, timeoutAtNanos, cancellationGizmo);
+return finalOutSequence.toYielder(initValue, accumulator);
+  }
+
+  /**
+   * Create an output {@link Sequence} that wraps the output {@link 
BlockingQueue} of a
+   * {@link MergeCombinePartitioningAction}
+   */
+  static  Sequence makeOutputSequenceForQueue(
+  BlockingQueue> queue,
+  boolean hasTimeout,
+  long timeoutAtNanos,
+  CancellationGizmo cancellationGizmo
+  )
+  {
+return new B

[GitHub] [incubator-druid] wengwh opened a new issue #8684: i develop a lookup extension，read hbase into lookup

[GitBox]

wengwh opened a new issue #8684: i develop a lookup extension，read hbase into 
lookup 
URL: https://github.com/apache/incubator-druid/issues/8684
 
 
   i develop a lookup extension，read hbase into lookup, i deploy success,data 
load success, 
   
   but i load datasource are fail 
   
   druid version：0.15.1
   
   i put hbase-client.jar into druid/extensions/myextension/
   but load datasource is fail，the msg：not found class。
   
   ERROR：
   2019-10-16T12:52:13,833 INFO [task-runner-0-priority-0] 
org.apache.hadoop.mapreduce.Job - Task Id : 
attempt_1569862598380_177757_m_00_0, Status : FAILED
   Error: java.lang.ClassNotFoundException: 
org.apache.hadoop.hbase.client.Result
at java.net.URLClassLoader.findClass(URLClassLoader.java:381)
at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:331)
at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
at 
org.apache.hadoop.util.ApplicationClassLoader.loadClass(ApplicationClassLoader.java:202)
at 
org.apache.hadoop.util.ApplicationClassLoader.loadClass(ApplicationClassLoader.java:170)
at java.lang.Class.getDeclaredMethods0(Native Method)
at java.lang.Class.privateGetDeclaredMethods(Class.java:2701)
at java.lang.Class.getDeclaredMethods(Class.java:1975)
at 
com.google.inject.spi.InjectionPoint.getInjectionPoints(InjectionPoint.java:688)
at 
com.google.inject.spi.InjectionPoint.forInstanceMethodsAndFields(InjectionPoint.java:380)
at 
com.google.inject.internal.ConstructorBindingImpl.getInternalDependencies(ConstructorBindingImpl.java:165)
at 
com.google.inject.internal.InjectorImpl.getInternalDependencies(InjectorImpl.java:616)
at 
com.google.inject.internal.InjectorImpl.cleanup(InjectorImpl.java:572)
at 
com.google.inject.internal.InjectorImpl.initializeJitBinding(InjectorImpl.java:558)
at 
com.google.inject.internal.InjectorImpl.createJustInTimeBinding(InjectorImpl.java:887)
at 
com.google.inject.internal.InjectorImpl.createJustInTimeBindingRecursive(InjectorImpl.java:808)
at 
com.google.inject.internal.InjectorImpl.getJustInTimeBinding(InjectorImpl.java:285)
at 
com.google.inject.internal.InjectorImpl.getBindingOrThrow(InjectorImpl.java:217)
at 
com.google.inject.internal.InjectorImpl.getInternalFactory(InjectorImpl.java:893)
at com.google.inject.internal.FactoryProxy.notify(FactoryProxy.java:46)
at 
com.google.inject.internal.ProcessedBindingData.runCreationListeners(ProcessedBindingData.java:50)
at 
com.google.inject.internal.InternalInjectorCreator.initializeStatically(InternalInjectorCreator.java:134)
at 
com.google.inject.internal.InternalInjectorCreator.build(InternalInjectorCreator.java:107)
at com.google.inject.Guice.createInjector(Guice.java:99)
at com.google.inject.Guice.createInjector(Guice.java:73)
at com.google.inject.Guice.createInjector(Guice.java:62)
at 
org.apache.druid.initialization.Initialization.makeInjectorWithModules(Initialization.java:419)
at 
org.apache.druid.indexer.HadoopDruidIndexerConfig.(HadoopDruidIndexerConfig.java:102)
at 
org.apache.druid.indexer.HadoopDruidIndexerMapper.setup(HadoopDruidIndexerMapper.java:53)
at 
org.apache.druid.indexer.DetermineHashedPartitionsJob$DetermineCardinalityMapper.setup(DetermineHashedPartitionsJob.java:269)
at 
org.apache.druid.indexer.DetermineHashedPartitionsJob$DetermineCardinalityMapper.run(DetermineHashedPartitionsJob.java:326)
at org.apache.hadoop.mapred.MapTask.runNewMapper(MapTask.java:787)
at org.apache.hadoop.mapred.MapTask.run(MapTask.java:341)
at org.apache.hadoop.mapred.YarnChild$2.run(YarnChild.java:164)
at java.security.AccessController.doPrivileged(Native Method)
at javax.security.auth.Subject.doAs(Subject.java:422)
at 
org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1707)
at org.apache.hadoop.mapred.YarnChild.main(YarnChild.java:158)
   


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] denever closed pull request #8656: Message rejection absolute date

[GitBox]

denever closed pull request #8656: Message rejection absolute date
URL: https://github.com/apache/incubator-druid/pull/8656
 
 
   


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] denever opened a new pull request #8656: Message rejection absolute date

[GitBox]

denever opened a new pull request #8656: Message rejection absolute date
URL: https://github.com/apache/incubator-druid/pull/8656
 
 
   ### Description
   
   Add an option to druid kafka indexing to select an absolute date time for 
message rejection.
   Use case: Migrating from a kafka pull (tranquillity) indexing to a kafka 
push indexing.
   So is needed to select at an absolute time for starting new indexing.
   
   
   
   This PR has:
   - [x] been self-reviewed.
   - [x] 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/incubator-druid/blob/master/licenses.yaml)
   - [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.
   - [ ] added integration tests.
   - [x] 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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] denever opened a new pull request #8656: Message rejection absolute date

[GitBox]

denever opened a new pull request #8656: Message rejection absolute date
URL: https://github.com/apache/incubator-druid/pull/8656
 
 
   ### Description
   
   Add an option to druid kafka indexing to select an absolute date time for 
message rejection.
   Use case: Migrating from a kafka pull (tranquillity) indexing to a kafka 
push indexing.
   So is needed to select at an absolute time for starting new indexing.
   
   
   
   This PR has:
   - [x] been self-reviewed.
   - [x] 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/incubator-druid/blob/master/licenses.yaml)
   - [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.
   - [ ] added integration tests.
   - [x] 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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] denever closed pull request #8656: Message rejection absolute date

[GitBox]

denever closed pull request #8656: Message rejection absolute date
URL: https://github.com/apache/incubator-druid/pull/8656
 
 
   


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] denever commented on issue #8656: Message rejection absolute date

[GitBox]

denever commented on issue #8656: Message rejection absolute date
URL: https://github.com/apache/incubator-druid/pull/8656#issuecomment-542760791
 
 
   Travis is blocked, has passed, but here is still pending


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] jihoonson commented on issue #8573: Stateful auto compaction

[GitBox]

jihoonson commented on issue #8573: Stateful auto compaction
URL: https://github.com/apache/incubator-druid/pull/8573#issuecomment-542776910
 
 
   Thanks for the review @ccaominh and @himanshug!


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field

[GitBox]

leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final 
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335590033
 
 

 ##
 File path: 
extensions-core/kinesis-indexing-service/src/test/java/org/apache/druid/indexing/kinesis/KinesisRecordSupplierTest.java
 ##
 @@ -51,6 +51,7 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 
+@SuppressWarnings("SSBasedInspection") // static field(s) cannot be final 
because not initialized in constructor
 
 Review comment:
   I don't understand what does this comment mean. Besides, I think all fields 
starting from `recordsPerFetch` actually don't need to be static in this class.


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field

[GitBox]

leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final 
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335582995
 
 

 ##
 File path: 
core/src/main/java/org/apache/druid/java/util/common/io/NativeIO.java
 ##
 @@ -35,9 +35,10 @@
 /**
  * Native I/O operations in order to minimize cache impact.
  */
+@SuppressWarnings("SSBasedInspection") // static field(s) cannot be final 
because set in an initializer block
 
 Review comment:
   Please move this suppression to `FIELD`.


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field

[GitBox]

leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final 
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335604366
 
 

 ##
 File path: 
processing/src/test/java/org/apache/druid/segment/SchemalessIndexTest.java
 ##
 @@ -64,9 +64,10 @@
 
 /**
  */
+@SuppressWarnings("SSBasedInspection") // static field(s) cannot be final 
because set in an static method
 
 Review comment:
   `mergedIndex` doesn't need to be static. It seems to be that `index` doesn't 
need to be static, too. `getIncrementalIndex()` and `makeIncrementalIndex()` 
should become non-static, 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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field

[GitBox]

leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final 
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335591115
 
 

 ##
 File path: 
extensions-core/lookups-cached-global/src/test/java/org/apache/druid/query/lookup/namespace/StaticMapExtractionNamespaceTest.java
 ##
 @@ -32,6 +32,8 @@
 {
   private static final Map MAP = ImmutableMap.builder().put("foo", "bar").build();
   private static final ObjectMapper MAPPER = new DefaultObjectMapper();
+
+  @SuppressWarnings("SSBasedInspection") // static field(s) cannot be final 
because set in an initializer block
   private static String MAP_STRING;
 
 Review comment:
   Seems like `setUpStatic()` actually could be inlined:
   ```java
   private static final String MAP_STRING = MAPPER.writeValueAsString(MAP);
   ```


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field

[GitBox]

leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final 
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335582594
 
 

 ##
 File path: core/src/main/java/org/apache/druid/common/config/NullHandling.java
 ##
 @@ -50,6 +50,7 @@
* It does not take effect in all unit tests since we don't use Guice 
Injection.
*/
   @Inject
+  @SuppressWarnings("SSBasedInspection") // static field(s) cannot be final 
because set in an initializer block
 
 Review comment:
   Again, has nothing to do with an initializer block. In this case, it's 
because this field is injected it cannot be final.


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field

[GitBox]

leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final 
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335601766
 
 

 ##
 File path: 
processing/src/test/java/org/apache/druid/query/scan/ScanResultValueTimestampComparatorTest.java
 ##
 @@ -36,6 +36,7 @@
 
 public class ScanResultValueTimestampComparatorTest
 {
+  @SuppressWarnings("SSBasedInspection") // static field(s) cannot be final 
because set in an initializer block
   private static QuerySegmentSpec intervalSpec;
 
 Review comment:
   setup() can be inlined


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field

[GitBox]

leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final 
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335599463
 
 

 ##
 File path: 
processing/src/main/java/org/apache/druid/segment/data/CompressionFactory.java
 ##
 @@ -95,10 +95,17 @@ private CompressionFactory()
* clearEncodingFlag function.
*/
 
+  /*
 
 Review comment:
   This comment duplicates a comment 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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field

[GitBox]

leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final 
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335598648
 
 

 ##
 File path: 
processing/src/main/java/org/apache/druid/query/topn/PooledTopNAlgorithm.java
 ##
 @@ -50,17 +50,14 @@
 
 /**
  */
+@SuppressWarnings("SSBasedInspection") // static field(s) cannot be final 
because set in an static method
 public class PooledTopNAlgorithm
 extends BaseTopNAlgorithm
 {
-  private static boolean SPECIALIZE_GENERIC_ONE_AGG_POOLED_TOPN =
-  !Boolean.getBoolean("dontSpecializeGeneric1AggPooledTopN");
-  private static boolean SPECIALIZE_GENERIC_TWO_AGG_POOLED_TOPN =
-  !Boolean.getBoolean("dontSpecializeGeneric2AggPooledTopN");
-  private static boolean 
SPECIALIZE_HISTORICAL_ONE_SIMPLE_DOUBLE_AGG_POOLED_TOPN =
-  
!Boolean.getBoolean("dontSpecializeHistorical1SimpleDoubleAggPooledTopN");
-  private static boolean 
SPECIALIZE_HISTORICAL_SINGLE_VALUE_DIM_SELECTOR_ONE_SIMPLE_DOUBLE_AGG_POOLED_TOPN
 =
-  
!Boolean.getBoolean("dontSpecializeHistoricalSingleValueDimSelector1SimpleDoubleAggPooledTopN");
+  private static boolean SPECIALIZE_GENERIC_ONE_AGG_POOLED_TOPN = 
!Boolean.getBoolean("dontSpecializeGeneric1AggPooledTopN");
 
 Review comment:
   Lines longer than 120 cols


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field

[GitBox]

leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final 
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335591855
 
 

 ##
 File path: 
extensions-core/protobuf-extensions/src/test/java/org/apache/druid/data/input/protobuf/ProtoTestEventWrapper.java
 ##
 @@ -3199,8 +3199,7 @@ public ProtoTestEvent parsePartialFrom(
 return descriptor;
   }
 
-  private static com.google.protobuf.Descriptors.FileDescriptor
-  descriptor;
+  private static com.google.protobuf.Descriptors.FileDescriptor descriptor;
 
 Review comment:
   Don't edit generated class. You can change the scope of the inspection to 
`NonGeneratedFiles`.


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field

[GitBox]

leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final 
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335581582
 
 

 ##
 File path: 
benchmarks/src/main/java/org/apache/druid/benchmark/query/ScanBenchmark.java
 ##
 @@ -102,6 +102,10 @@
 @Measurement(iterations = 25)
 public class ScanBenchmark
 {
+  @Param({"NONE", "DESCENDING", "ASCENDING"})
+  @SuppressWarnings("SSBasedInspection") // static field(s) cannot be final 
because set in an initializer block
+  private static ScanQuery.Order ordering;
 
 Review comment:
   This field is not set in an initializer block. Please check the truthfulness 
of the comments that you add.
   
   Since this is `@Param`, it shouldn't be static at all. Could you please 
prohibit static `@Params` by adding another structured search pattern:
   ```java
   @Param($Values$) static $T$ $param$;
   ```
   in this 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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field

[GitBox]

leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final 
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335596027
 
 

 ##
 File path: 
processing/src/main/java/org/apache/druid/query/aggregation/hyperloglog/HyperUniquesSerde.java
 ##
 @@ -38,8 +38,7 @@
 
 public class HyperUniquesSerde extends ComplexMetricSerde
 {
-  private static Comparator comparator =
-  
Comparator.nullsFirst(Comparator.comparing(HyperLogLogCollector::toByteBuffer));
+  private static final Comparator COMPARATOR = 
Comparator.nullsFirst(Comparator.comparing(HyperLogLogCollector::toByteBuffer));
 
 Review comment:
   Line longer than 120 cols


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field

[GitBox]

leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final 
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335595130
 
 

 ##
 File path: 
indexing-service/src/test/java/org/apache/druid/indexing/overlord/TaskLifecycleTest.java
 ##
 @@ -250,6 +250,7 @@ public int compare(DataSegment dataSegment, DataSegment 
dataSegment2)
   private SegmentHandoffNotifierFactory handoffNotifierFactory;
   private Map> handOffCallbacks;
 
+  @SuppressWarnings("SSBasedInspection") // static field(s) cannot be final 
because set in an initializer block
   private static CountDownLatch publishCountDown;
 
 Review comment:
   This field doesn't need to be static.


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field

[GitBox]

leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final 
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335600321
 
 

 ##
 File path: 
processing/src/test/java/org/apache/druid/query/filter/IntervalDimFilterTest.java
 ##
 @@ -37,6 +37,7 @@
 
 public class IntervalDimFilterTest
 {
+  @SuppressWarnings("SSBasedInspection")  // static field(s) cannot be final 
because set in an initializer block
 
 Review comment:
   method


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field

[GitBox]

leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final 
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335583876
 
 

 ##
 File path: core/src/main/java/org/apache/druid/utils/JvmUtils.java
 ##
 @@ -50,11 +50,12 @@ public static boolean isIsJava9Compatible()
   }
 
   @Inject
-  private static RuntimeInfo runtimeInfo = new RuntimeInfo();
+  @SuppressWarnings("SSBasedInspection") // static field(s) cannot be final 
because set in an initializer block
 
 Review comment:
   Injection.


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field

[GitBox]

leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final 
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335585792
 
 

 ##
 File path: 
core/src/test/java/org/apache/druid/java/util/http/client/response/SequenceInputStreamResponseHandlerTest.java
 ##
 @@ -43,6 +43,8 @@
   private static final int TOTAL_BYTES = 1 << 10;
   private static final ArrayList BYTE_LIST = new ArrayList<>();
   private static final Random RANDOM = new Random(378134789L);
+
+  @SuppressWarnings("SSBasedInspection")  // static field(s) cannot be final 
because set in an initializer block
 
 Review comment:
   method


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field

[GitBox]

leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final 
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335590256
 
 

 ##
 File path: 
extensions-core/kinesis-indexing-service/src/test/java/org/apache/druid/indexing/kinesis/supervisor/KinesisSupervisorTest.java
 ##
 @@ -126,6 +126,7 @@
   private static final StreamPartition SHARD2_PARTITION = 
StreamPartition.of(STREAM, SHARD_ID2);
   private static final StreamPartition SHARD3_PARTITION = 
StreamPartition.of(STREAM, SHARD_ID3);
 
+  @SuppressWarnings("SSBasedInspection") // static field(s) cannot be final 
because set in an initializer block
 
 Review comment:
   method


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field

[GitBox]

leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final 
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335583652
 
 

 ##
 File path: 
core/src/main/java/org/apache/druid/java/util/emitter/EmittingLogger.java
 ##
 @@ -40,6 +40,7 @@
   public static final String EXCEPTION_MESSAGE_KEY = "exceptionMessage";
   public static final String EXCEPTION_STACK_TRACE_KEY = "exceptionStackTrace";
 
+  @SuppressWarnings("SSBasedInspection") // static field(s) cannot be final 
because set in an initializer block
 
 Review comment:
   Same problem here, the comment doesn't reflect the reality. 
registerEmitter() is the reason.


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field

[GitBox]

leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final 
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r33559
 
 

 ##
 File path: 
processing/src/test/java/org/apache/druid/query/extraction/MapLookupExtractionFnSerDeTest.java
 ##
 @@ -39,6 +39,7 @@
  */
 public class MapLookupExtractionFnSerDeTest
 {
+  @SuppressWarnings("SSBasedInspection") // static field(s) cannot be final 
because set in an initializer block
 
 Review comment:
   method


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field

[GitBox]

leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final 
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335585321
 
 

 ##
 File path: 
core/src/test/java/org/apache/druid/data/input/impl/prefetch/PrefetchableTextFilesFirehoseFactoryTest.java
 ##
 @@ -60,6 +60,7 @@
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeoutException;
 
+@SuppressWarnings("SSBasedInspection") // static field(s) cannot be final 
because set in an static method
 
 Review comment:
   Typo: "**a** static method". Please fix all the similar places in this 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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field

[GitBox]

leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final 
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335600158
 
 

 ##
 File path: 
processing/src/test/java/org/apache/druid/query/filter/InDimFilterSerDesrTest.java
 ##
 @@ -35,6 +35,8 @@
 
 public class InDimFilterSerDesrTest
 {
+
+  @SuppressWarnings("SSBasedInspection") // static field(s) cannot be final 
because set in an initializer block
 
 Review comment:
   method


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field

[GitBox]

leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final 
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335587140
 
 

 ##
 File path: 
extensions-core/datasketches/src/test/java/org/apache/druid/query/aggregation/datasketches/theta/sql/ThetaSketchSqlAggregatorTest.java
 ##
 @@ -84,13 +84,14 @@
 import java.util.List;
 import java.util.Map;
 
+@SuppressWarnings("SSBasedInspection") // static field(s) cannot be final 
because set in an static method
 public class ThetaSketchSqlAggregatorTest extends CalciteTestBase
 {
   private static final String DATA_SOURCE = "foo";
 
   private static QueryRunnerFactoryConglomerate conglomerate;
 
 Review comment:
   This code repeats in a number of classes, could it be factored out to 
`CalciteTestBase`?


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final Static Field

[GitBox]

leventov commented on a change in pull request #8558: 7227 : Prohibit Non Final 
Static Field
URL: https://github.com/apache/incubator-druid/pull/8558#discussion_r335595612
 
 

 ##
 File path: 
indexing-service/src/test/java/org/apache/druid/indexing/overlord/http/security/OverlordSecurityResourceFilterTest.java
 ##
 @@ -48,6 +48,7 @@
 import java.util.regex.Pattern;
 
 @RunWith(Parameterized.class)
+@SuppressWarnings("SSBasedInspection") // static field(s) cannot be final 
because not initialized in constructor.
 
 Review comment:
   Fields don't need to be static.


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] jihoonson commented on a change in pull request #8656: Message rejection absolute date

[GitBox]

jihoonson commented on a change in pull request #8656: Message rejection 
absolute date
URL: https://github.com/apache/incubator-druid/pull/8656#discussion_r335616355
 
 

 ##
 File path: docs/development/extensions-core/kafka-ingestion.md
 ##
 @@ -206,6 +206,7 @@ For Roaring bitmaps:
 |`period`|ISO8601 Period|How often the supervisor will execute its management 
logic. Note that the supervisor will also run in response to certain events 
(such as tasks succeeding, failing, and reaching their taskDuration) so this 
value specifies the maximum time between iterations.|no (default == PT30S)|
 |`useEarliestOffset`|Boolean|If a supervisor is managing a dataSource for the 
first time, it will obtain a set of starting offsets from Kafka. This flag 
determines whether it retrieves the earliest or latest offsets in Kafka. Under 
normal circumstances, subsequent tasks will start from where the previous 
segments ended so this flag will only be used on first run.|no (default == 
false)|
 |`completionTimeout`|ISO8601 Period|The length of time to wait before 
declaring a publishing task as failed and terminating it. If this is set too 
low, your tasks may never publish. The publishing clock for a task begins 
roughly after `taskDuration` elapses.|no (default == PT30M)|
+|`lateMessageRejectionStartDateTime`|ISO8601 DateTime|Configure tasks to 
reject messages with timestamps earlier than this date time; for example if 
this is set to `2016-01-01T11:00Z` and the supervisor creates a task at 
*2016-01-01T12:00Z*, messages with timestamps earlier than *2016-01-01T11:00Z* 
will be dropped. This may help prevent concurrency issues if your data stream 
has late messages and you have multiple pipelines that need to operate on the 
same segments (e.g. a realtime and a nightly batch ingestion pipeline). This 
option exclude `lateMessageRejectionPeriod`.|no (default == none)|
 |`lateMessageRejectionPeriod`|ISO8601 Period|Configure tasks to reject 
messages with timestamps earlier than this period before the task was created; 
for example if this is set to `PT1H` and the supervisor creates a task at 
*2016-01-01T12:00Z*, messages with timestamps earlier than *2016-01-01T11:00Z* 
will be dropped. This may help prevent concurrency issues if your data stream 
has late messages and you have multiple pipelines that need to operate on the 
same segments (e.g. a realtime and a nightly batch ingestion pipeline).|no 
(default == none)|
 
 Review comment:
   Please mention here that `lateMessageRejectionPeriod` is ignored if 
`lateMessageRejectionStartDateTime` is specified. 


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] JulianJaffePinterest opened a new issue #8685: Allow bootstrap.servers to be provided via a PasswordProvider for Kafka Ingestion

[GitBox]

JulianJaffePinterest opened a new issue #8685: Allow bootstrap.servers to be 
provided via a PasswordProvider for Kafka Ingestion
URL: https://github.com/apache/incubator-druid/issues/8685
 
 
   ### Description
   
   Currently, the `consumerProperties` map in a KafkaSupervisorIOConfig can use 
PasswordProviders for the `keystore`, `truststore` and `key` properties, but 
`bootstrap.servers` must be a hard-coded String. Although `bootstrap.servers` 
is not a password, it would be helpful if the value for this property could be 
provided via a similar mechanism to a PasswordProvider. This would allow 
SupervisorSpecs to be robust to changes in the Kafka bootstrap servers in the 
event of a cluster migration or the like.
   
   ### Motivation
   
   Currently, if the bootstrap servers for a Kafka cluster change, running 
supervisor specs that refer to those bootstrap servers must be manually killed 
and relaunched with references to the new bootstrap servers. If a cluster 
operator has access to a separate system that can provide updated bootstrap 
server information, these manual operations can be avoided by borrowing the 
concept of a "Provider" from the PasswordProvider.
   


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] gianm commented on issue #8685: Allow bootstrap.servers to be provided via a PasswordProvider for Kafka Ingestion

[GitBox]

gianm commented on issue #8685: Allow bootstrap.servers to be provided via a 
PasswordProvider for Kafka Ingestion
URL: 
https://github.com/apache/incubator-druid/issues/8685#issuecomment-542863946
 
 
   Maybe it'd make sense to allow this for _any_ property?


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] vogievetsky opened a new issue #8686: Router management Proxy routing should have a way to route to any node in the cluster

[GitBox]

vogievetsky opened a new issue #8686: Router management Proxy routing should 
have a way to route to any node in the cluster
URL: https://github.com/apache/incubator-druid/issues/8686
 
 
   ### Description
   
   It would be amazing if the router's management proxy could proxy to any node 
in the cluster
   
   Right now the router can proxy to the overlord and coordinator leaders (see 
https://druid.apache.org/docs/latest/design/router.html#management-proxy-routing
 ) via:
   
   `/proxy/coordinator/*` and `/proxy/overlord/*` 
   
   I would like tp propose adding a route like:
   
   `/proxy/node///*` that would proxy `*` to the node at 
`:` where `` and `` would 
correspond to the host and port columns as reported by the `sys.servers` table.
   
   ### Motivation
   
   This would enable a host of cool status and diagnostic checks from the 
router web console. In particular it would turbo charge what can be done in the 
Druid Doctor ( https://github.com/apache/incubator-druid/pull/8672 ) allowing 
the creation of a check that would make sure all runtime props are set 
correctly and that all nodes are reachable.


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] clintropolis commented on issue #7066: Support assign tasks to run on different categories of MiddleManagers

[GitBox]

clintropolis commented on issue #7066: Support assign tasks to run on different 
categories of MiddleManagers
URL: https://github.com/apache/incubator-druid/pull/7066#issuecomment-542879965
 
 
   >@clintropolis It seems didn't work.
   
   Oops, I guess you added it to the wrong part of the file so the exclusions 
are applying to 
   ```
- ../docs/tutorials/index.md
   ```
   
   The first part of the file is exclusions that apply to everything and then 
for some reason after that we have file specific exclusions, but not really 
sure why we have it split up like that instead of just a big global list.


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] jihoonson commented on issue #8663: Kafka indexing service duplicate entry exception in druid_pendingSegments

[GitBox]

jihoonson commented on issue #8663: Kafka indexing service duplicate entry 
exception in druid_pendingSegments
URL: 
https://github.com/apache/incubator-druid/issues/8663#issuecomment-542881773
 
 
   Hmm, the entries in `druid_pendingSegments` table are never updated but 
should be reused if possible. The way the segment ID allocation works is as 
below:
   
   1) a task asks a new segment ID to the overlord with `datasource`, 
`interval`, and `sequenceName`. Segment ID is unique per `datasource`, 
`interval`, and `sequenceName` and segment ID allocation is idempotent. This 
means, if a task asks with the same `datasource`, `interval`, and 
`sequenceName`, the overlord will return the same segment ID instead of 
creating a new one. The base `sequenceName` is generated as 
[here](https://github.com/apache/incubator-druid/blob/master/indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisor.java#L1813-L1848),
 and each task will attach a sequential number whenever intermediate publish is 
triggered.
   2) The overlord first looks up `druid_pendingSegments` table to see if 
there's a reusable segment ID.
   3) If not, it looks up both `druid_segments` and `druid_pendingSegments` 
tables to find the next available segment ID. This is done by searching for the 
max partition id of all segments in the interval including both `published` and 
`unpublished` ones. The new segment will have the partition id of `current max 
partition id + 1`.
   4) Once it finds the next segment ID, it will insert it into the metadata 
store. Otherwise, the segment ID allocation fails.
   
   `2) - 4)` should be done atomically. Based on what you see in the metadata 
store, do you think there could be a bug in any step?


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] jon-wei commented on a change in pull request #8487: Add initial SQL support for non-expression sketch postaggs

[GitBox]

jon-wei commented on a change in pull request #8487: Add initial SQL support 
for non-expression sketch postaggs
URL: https://github.com/apache/incubator-druid/pull/8487#discussion_r335716107
 
 

 ##
 File path: docs/querying/sql.md
 ##
 @@ -363,6 +366,44 @@ All 'array' references in the multi-value string function 
documentation can refe
 | `MV_TO_STRING(arr,str)` | joins all elements of arr by the delimiter 
specified by str |
 | `STRING_TO_MV(str1,str2)` | splits str1 into an array on the delimiter 
specified by str2 |
 
+### Sketch operators
+
+These functions operate on expressions or columns that return sketch objects.
+
+ HLL sketch operators
+
+The following functions operate on [DataSketches HLL 
sketches](../development/extensions-core/datasketches-hll.html).
+The [DataSketches 
extension](../development/extensions-core/datasketches-extension.html) must be 
loaded to use the following functions.
+
+|Function|Notes|
+||-|
+|`HLL_SKETCH_ESTIMATE(expr)`|Returns the distinct count estimate from an HLL 
sketch. `expr` must return an HLL sketch.|
 
 Review comment:
   Hmm, i actually like the new functions added in this PR more than 
`APPROX_COUNT_DISTINCT_DS_HLL`, I think they express a clearer boundary between 
the aggregated sketch object and the operations that can be performed on said 
object. The new functions names also have a 1-to-1 correspondence with the 
underlying native aggs/postaggs which I think is nice.
   
   If not for backwards compatibility, I would actually consider dropping 
`APPROX_COUNT_DISTINCT_DS_HLL`, it was introduced as a way to allow people to 
use the datasketches implementations before we supported postaggs in SQL (with 
this patch).
   
   


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] jon-wei commented on a change in pull request #8487: Add initial SQL support for non-expression sketch postaggs

[GitBox]

jon-wei commented on a change in pull request #8487: Add initial SQL support 
for non-expression sketch postaggs
URL: https://github.com/apache/incubator-druid/pull/8487#discussion_r335716427
 
 

 ##
 File path: docs/querying/sql.md
 ##
 @@ -363,6 +366,44 @@ All 'array' references in the multi-value string function 
documentation can refe
 | `MV_TO_STRING(arr,str)` | joins all elements of arr by the delimiter 
specified by str |
 | `STRING_TO_MV(str1,str2)` | splits str1 into an array on the delimiter 
specified by str2 |
 
+### Sketch operators
+
+These functions operate on expressions or columns that return sketch objects.
+
+ HLL sketch operators
+
+The following functions operate on [DataSketches HLL 
sketches](../development/extensions-core/datasketches-hll.html).
+The [DataSketches 
extension](../development/extensions-core/datasketches-extension.html) must be 
loaded to use the following functions.
+
+|Function|Notes|
+||-|
+|`HLL_SKETCH_ESTIMATE(expr)`|Returns the distinct count estimate from an HLL 
sketch. `expr` must return an HLL sketch.|
+|`HLL_SKETCH_ESTIMATE_WITH_ERROR_BOUNDS(expr, [numStdDev])`|Returns the 
distinct count estimate and error bounds from an HLL sketch. `expr` must return 
an HLL sketch. An optional `numStdDev` argument can be provided.|
 
 Review comment:
   Within the context of this sketch and its documentation 
(https://druid.apache.org/docs/latest/development/extensions-core/datasketches-hll.html),
 I think the meaning of `estimate` is clear


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] jon-wei commented on a change in pull request #8487: Add initial SQL support for non-expression sketch postaggs

[GitBox]

jon-wei commented on a change in pull request #8487: Add initial SQL support 
for non-expression sketch postaggs
URL: https://github.com/apache/incubator-druid/pull/8487#discussion_r335716800
 
 

 ##
 File path: docs/querying/sql.md
 ##
 @@ -363,6 +366,44 @@ All 'array' references in the multi-value string function 
documentation can refe
 | `MV_TO_STRING(arr,str)` | joins all elements of arr by the delimiter 
specified by str |
 | `STRING_TO_MV(str1,str2)` | splits str1 into an array on the delimiter 
specified by str2 |
 
+### Sketch operators
+
+These functions operate on expressions or columns that return sketch objects.
+
+ HLL sketch operators
+
+The following functions operate on [DataSketches HLL 
sketches](../development/extensions-core/datasketches-hll.html).
+The [DataSketches 
extension](../development/extensions-core/datasketches-extension.html) must be 
loaded to use the following functions.
+
+|Function|Notes|
+||-|
+|`HLL_SKETCH_ESTIMATE(expr)`|Returns the distinct count estimate from an HLL 
sketch. `expr` must return an HLL sketch.|
+|`HLL_SKETCH_ESTIMATE_WITH_ERROR_BOUNDS(expr, [numStdDev])`|Returns the 
distinct count estimate and error bounds from an HLL sketch. `expr` must return 
an HLL sketch. An optional `numStdDev` argument can be provided.|
+|`HLL_SKETCH_UNION([lgK, tgtHllType], expr0, expr1, ...)`|Returns a union of 
HLL sketches, where each input expression must return an HLL sketch. The `lgK` 
and `tgtHllType` can be optionally specified as the first parameter; if 
provided, both optional parameters must be specified.|
+|`HLL_SKETCH_TO_STRING(expr)`|Returns a human-readable string representation 
of an HLL sketch for debugging. `expr` must return an HLL sketch.|
+
+ Theta sketch operators
+
+The following functions operate on [theta 
sketches](../development/extensions-core/datasketches-theta.html).
+The [DataSketches 
extension](../development/extensions-core/datasketches-extension.html) must be 
loaded to use the following functions.
+
+|Function|Notes|
+||-|
+|`THETA_SKETCH_ESTIMATE(expr)`|Returns the distinct count estimate from a 
theta sketch. `expr` must return a theta sketch.|
 
 Review comment:
   My reasoning here is the same as in this comment: 
https://github.com/apache/incubator-druid/pull/8487/files#r335716107


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] jon-wei commented on a change in pull request #8487: Add initial SQL support for non-expression sketch postaggs

[GitBox]

jon-wei commented on a change in pull request #8487: Add initial SQL support 
for non-expression sketch postaggs
URL: https://github.com/apache/incubator-druid/pull/8487#discussion_r335717239
 
 

 ##
 File path: docs/querying/sql.md
 ##
 @@ -363,6 +366,44 @@ All 'array' references in the multi-value string function 
documentation can refe
 | `MV_TO_STRING(arr,str)` | joins all elements of arr by the delimiter 
specified by str |
 | `STRING_TO_MV(str1,str2)` | splits str1 into an array on the delimiter 
specified by str2 |
 
+### Sketch operators
+
+These functions operate on expressions or columns that return sketch objects.
+
+ HLL sketch operators
+
+The following functions operate on [DataSketches HLL 
sketches](../development/extensions-core/datasketches-hll.html).
+The [DataSketches 
extension](../development/extensions-core/datasketches-extension.html) must be 
loaded to use the following functions.
+
+|Function|Notes|
+||-|
+|`HLL_SKETCH_ESTIMATE(expr)`|Returns the distinct count estimate from an HLL 
sketch. `expr` must return an HLL sketch.|
+|`HLL_SKETCH_ESTIMATE_WITH_ERROR_BOUNDS(expr, [numStdDev])`|Returns the 
distinct count estimate and error bounds from an HLL sketch. `expr` must return 
an HLL sketch. An optional `numStdDev` argument can be provided.|
+|`HLL_SKETCH_UNION([lgK, tgtHllType], expr0, expr1, ...)`|Returns a union of 
HLL sketches, where each input expression must return an HLL sketch. The `lgK` 
and `tgtHllType` can be optionally specified as the first parameter; if 
provided, both optional parameters must be specified.|
+|`HLL_SKETCH_TO_STRING(expr)`|Returns a human-readable string representation 
of an HLL sketch for debugging. `expr` must return an HLL sketch.|
+
+ Theta sketch operators
+
+The following functions operate on [theta 
sketches](../development/extensions-core/datasketches-theta.html).
+The [DataSketches 
extension](../development/extensions-core/datasketches-extension.html) must be 
loaded to use the following functions.
+
+|Function|Notes|
+||-|
+|`THETA_SKETCH_ESTIMATE(expr)`|Returns the distinct count estimate from a 
theta sketch. `expr` must return a theta sketch.|
+|`THETA_SKETCH_ESTIMATE_WITH_ERROR_BOUNDS(expr, errorBoundsStdDev)`|Returns 
the distinct count estimate and error bounds from a theta sketch. `expr` must 
return a theta sketch.|
 
 Review comment:
   I think given the context and other docs 
(https://druid.apache.org/docs/latest/development/extensions-core/datasketches-theta.html),
 `estimate` is clear here


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] jon-wei commented on a change in pull request #8487: Add initial SQL support for non-expression sketch postaggs

[GitBox]

jon-wei commented on a change in pull request #8487: Add initial SQL support 
for non-expression sketch postaggs
URL: https://github.com/apache/incubator-druid/pull/8487#discussion_r335718002
 
 

 ##
 File path: 
extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/hll/HllSketchToEstimatePostAggregator.java
 ##
 @@ -0,0 +1,135 @@
+/*
+ * 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.aggregation.datasketches.hll;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.yahoo.sketches.hll.HllSketch;
+import org.apache.druid.query.aggregation.AggregatorFactory;
+import org.apache.druid.query.aggregation.PostAggregator;
+import org.apache.druid.query.aggregation.post.ArithmeticPostAggregator;
+import org.apache.druid.query.aggregation.post.PostAggregatorIds;
+import org.apache.druid.query.cache.CacheKeyBuilder;
+
+import java.util.Comparator;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+
+/**
+ * Returns a distinct count estimate a from a given {@link HllSketch}.
+ * The result will be a double value.
+ */
+public class HllSketchToEstimatePostAggregator implements PostAggregator
+{
+  private final String name;
+  private final PostAggregator field;
+
+  @JsonCreator
+  public HllSketchToEstimatePostAggregator(
+  @JsonProperty("name") final String name,
+  @JsonProperty("field") final PostAggregator field
+  )
+  {
+this.name = name;
+this.field = field;
+  }
+
+  @Override
+  @JsonProperty
+  public String getName()
+  {
+return name;
+  }
+
+  @JsonProperty
+  public PostAggregator getField()
+  {
+return field;
+  }
+
+  @Override
+  public Set getDependentFields()
+  {
+return field.getDependentFields();
+  }
+
+  @Override
+  public Comparator getComparator()
+  {
+return ArithmeticPostAggregator.DEFAULT_COMPARATOR;
+  }
+
+  @Override
+  public Object compute(final Map combinedAggregators)
+  {
+final HllSketch sketch = (HllSketch) field.compute(combinedAggregators);
+return sketch.getEstimate();
+  }
+
+  @Override
+  public PostAggregator decorate(final Map 
aggregators)
+  {
+return this;
+  }
+
+  @Override
+  public String toString()
+  {
+return getClass().getSimpleName() + "{" +
+"name='" + name + '\'' +
+", field=" + field +
+"}";
+  }
+
+  @Override
+  public boolean equals(final Object o)
+  {
+if (this == o) {
+  return true;
+}
+if (!(o instanceof HllSketchToEstimatePostAggregator)) {
+  return false;
+}
+
+final HllSketchToEstimatePostAggregator that = 
(HllSketchToEstimatePostAggregator) o;
+
+if (!name.equals(that.name)) {
+  return false;
+}
+return field.equals(that.field);
+  }
+
+  @Override
+  public int hashCode()
+  {
+return Objects.hash(name, field);
+  }
+
+  @Override
+  public byte[] getCacheKey()
+  {
+return new 
CacheKeyBuilder(PostAggregatorIds.HLL_SKETCH_TO_ESTIMATE_CACHE_TYPE_ID)
+.appendString(name)
 
 Review comment:
   Removed, 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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] jon-wei commented on a change in pull request #8487: Add initial SQL support for non-expression sketch postaggs

[GitBox]

jon-wei commented on a change in pull request #8487: Add initial SQL support 
for non-expression sketch postaggs
URL: https://github.com/apache/incubator-druid/pull/8487#discussion_r335718056
 
 

 ##
 File path: 
extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/hll/sql/HllSketchEstimateOperatorConversion.java
 ##
 @@ -0,0 +1,97 @@
+/*
+ * 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.aggregation.datasketches.hll.sql;
+
+import org.apache.calcite.rex.RexCall;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.SqlFunction;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.type.ReturnTypes;
+import org.apache.calcite.sql.type.SqlTypeFamily;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.query.aggregation.PostAggregator;
+import 
org.apache.druid.query.aggregation.datasketches.hll.HllSketchToEstimatePostAggregator;
+import org.apache.druid.sql.calcite.expression.DirectOperatorConversion;
+import org.apache.druid.sql.calcite.expression.DruidExpression;
+import org.apache.druid.sql.calcite.expression.OperatorConversions;
+import org.apache.druid.sql.calcite.expression.PostAggregatorVisitor;
+import org.apache.druid.sql.calcite.planner.PlannerContext;
+import org.apache.druid.sql.calcite.table.RowSignature;
+
+import javax.annotation.Nullable;
+import java.util.List;
+
+public class HllSketchEstimateOperatorConversion extends 
DirectOperatorConversion
+{
+  private static String FUNCTION_NAME = "HLL_SKETCH_ESTIMATE";
 
 Review comment:
   Added final here and in similar places


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] jon-wei commented on a change in pull request #8487: Add initial SQL support for non-expression sketch postaggs

[GitBox]

jon-wei commented on a change in pull request #8487: Add initial SQL support 
for non-expression sketch postaggs
URL: https://github.com/apache/incubator-druid/pull/8487#discussion_r335718581
 
 

 ##
 File path: 
extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/theta/sql/ThetaSketchSetBaseOperatorConversion.java
 ##
 @@ -0,0 +1,140 @@
+/*
+ * 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.aggregation.datasketches.theta.sql;
+
+import org.apache.calcite.rex.RexCall;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.SqlFunction;
+import org.apache.calcite.sql.SqlFunctionCategory;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.type.OperandTypes;
+import org.apache.calcite.sql.type.ReturnTypes;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.query.aggregation.PostAggregator;
+import 
org.apache.druid.query.aggregation.datasketches.theta.SketchSetPostAggregator;
+import org.apache.druid.sql.calcite.expression.DruidExpression;
+import org.apache.druid.sql.calcite.expression.OperatorConversions;
+import org.apache.druid.sql.calcite.expression.PostAggregatorVisitor;
+import org.apache.druid.sql.calcite.expression.SqlOperatorConversion;
+import org.apache.druid.sql.calcite.planner.Calcites;
+import org.apache.druid.sql.calcite.planner.PlannerContext;
+import org.apache.druid.sql.calcite.table.RowSignature;
+
+import javax.annotation.Nullable;
+import java.util.ArrayList;
+import java.util.List;
+
+public abstract class ThetaSketchSetBaseOperatorConversion implements 
SqlOperatorConversion
+{
+  public ThetaSketchSetBaseOperatorConversion()
+  {
+  }
+
+  @Override
+  public SqlOperator calciteOperator()
+  {
+return makeSqlFunction();
+  }
+
+  @Nullable
+  @Override
+  public DruidExpression toDruidExpression(
+  PlannerContext plannerContext,
+  RowSignature rowSignature,
+  RexNode rexNode
+  )
+  {
+return null;
+  }
+
+  @Nullable
+  @Override
+  public PostAggregator toPostAggregator(
+  PlannerContext plannerContext,
+  RowSignature rowSignature,
+  RexNode rexNode,
+  PostAggregatorVisitor postAggregatorVisitor
+  )
+  {
+final List operands = ((RexCall) rexNode).getOperands();
+final List inputPostAggs = new ArrayList<>();
+Integer size = null;
+
+int operandCounter = 0;
+for (RexNode operand : operands) {
+  final PostAggregator convertedPostAgg = 
OperatorConversions.toPostAggregator(
+  plannerContext,
+  rowSignature,
+  operand,
+  postAggregatorVisitor
+  );
+  if (convertedPostAgg == null) {
+if (operandCounter == 0) {
+  try {
+size = RexLiteral.intValue(operand);
+  }
+  catch (RuntimeException re) {
+return null;
+  }
+} else {
+  return null;
+}
+  } else {
+inputPostAggs.add(convertedPostAgg);
+operandCounter++;
+  }
+}
+
+return new SketchSetPostAggregator(
+postAggregatorVisitor.getOutputNamePrefix() + 
postAggregatorVisitor.getAndIncrementCounter(),
+getSetOperationName(),
+size,
+inputPostAggs
+);
+  }
+
+  private SqlFunction makeSqlFunction()
+  {
+return new SqlFunction(
+getFunctionName(),
+SqlKind.OTHER_FUNCTION,
+ReturnTypes.explicit(
+factory -> Calcites.createSqlType(factory, SqlTypeName.OTHER)
+),
+null,
+OperandTypes.VARIADIC,
 
 Review comment:
   Changed to suggested


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


With regards,
Apache Git Services

-
To unsubscribe, e-mail: 

[GitHub] [incubator-druid] jon-wei commented on a change in pull request #8487: Add initial SQL support for non-expression sketch postaggs

[GitBox]

jon-wei commented on a change in pull request #8487: Add initial SQL support 
for non-expression sketch postaggs
URL: https://github.com/apache/incubator-druid/pull/8487#discussion_r335718521
 
 

 ##
 File path: 
extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/hll/sql/HllSketchSetUnionOperatorConversion.java
 ##
 @@ -0,0 +1,137 @@
+/*
+ * 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.aggregation.datasketches.hll.sql;
+
+import org.apache.calcite.rex.RexCall;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.SqlFunction;
+import org.apache.calcite.sql.SqlFunctionCategory;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.type.OperandTypes;
+import org.apache.calcite.sql.type.ReturnTypes;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.druid.query.aggregation.PostAggregator;
+import 
org.apache.druid.query.aggregation.datasketches.hll.HllSketchUnionPostAggregator;
+import org.apache.druid.sql.calcite.expression.DruidExpression;
+import org.apache.druid.sql.calcite.expression.OperatorConversions;
+import org.apache.druid.sql.calcite.expression.PostAggregatorVisitor;
+import org.apache.druid.sql.calcite.expression.SqlOperatorConversion;
+import org.apache.druid.sql.calcite.planner.Calcites;
+import org.apache.druid.sql.calcite.planner.PlannerContext;
+import org.apache.druid.sql.calcite.table.RowSignature;
+
+import javax.annotation.Nullable;
+import java.util.ArrayList;
+import java.util.List;
+
+public class HllSketchSetUnionOperatorConversion implements 
SqlOperatorConversion
+{
+  private static final SqlFunction SQL_FUNCTION = new SqlFunction(
+  "HLL_SKETCH_UNION",
+  SqlKind.OTHER_FUNCTION,
+  ReturnTypes.explicit(
+  factory -> Calcites.createSqlType(factory, SqlTypeName.OTHER)
+  ),
+  null,
+  OperandTypes.VARIADIC,
 
 Review comment:
   Changed to suggested


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] jon-wei commented on a change in pull request #8487: Add initial SQL support for non-expression sketch postaggs

[GitBox]

jon-wei commented on a change in pull request #8487: Add initial SQL support 
for non-expression sketch postaggs
URL: https://github.com/apache/incubator-druid/pull/8487#discussion_r335718425
 
 

 ##
 File path: 
extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/hll/sql/HllSketchApproxCountDistinctSqlAggregator.java
 ##
 @@ -0,0 +1,211 @@
+/*
+ * 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.aggregation.datasketches.hll.sql;
+
+import org.apache.calcite.rel.core.AggregateCall;
+import org.apache.calcite.rel.core.Project;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.SqlAggFunction;
+import org.apache.calcite.sql.SqlFunctionCategory;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.type.InferTypes;
+import org.apache.calcite.sql.type.OperandTypes;
+import org.apache.calcite.sql.type.ReturnTypes;
+import org.apache.calcite.sql.type.SqlTypeFamily;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.druid.java.util.common.ISE;
+import org.apache.druid.query.aggregation.AggregatorFactory;
+import 
org.apache.druid.query.aggregation.datasketches.hll.HllSketchAggregatorFactory;
+import 
org.apache.druid.query.aggregation.datasketches.hll.HllSketchBuildAggregatorFactory;
+import 
org.apache.druid.query.aggregation.datasketches.hll.HllSketchMergeAggregatorFactory;
+import 
org.apache.druid.query.aggregation.post.FinalizingFieldAccessPostAggregator;
+import org.apache.druid.query.dimension.DefaultDimensionSpec;
+import org.apache.druid.query.dimension.DimensionSpec;
+import org.apache.druid.segment.VirtualColumn;
+import org.apache.druid.segment.column.ValueType;
+import org.apache.druid.sql.calcite.aggregation.Aggregation;
+import org.apache.druid.sql.calcite.aggregation.SqlAggregator;
+import org.apache.druid.sql.calcite.expression.DruidExpression;
+import org.apache.druid.sql.calcite.expression.Expressions;
+import org.apache.druid.sql.calcite.planner.Calcites;
+import org.apache.druid.sql.calcite.planner.PlannerContext;
+import org.apache.druid.sql.calcite.rel.VirtualColumnRegistry;
+import org.apache.druid.sql.calcite.table.RowSignature;
+
+import javax.annotation.Nullable;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+public class HllSketchApproxCountDistinctSqlAggregator implements SqlAggregator
+{
+  private static final SqlAggFunction FUNCTION_INSTANCE = new 
HllSketchApproxCountDistinctSqlAggFunction();
+  private static final String NAME = "APPROX_COUNT_DISTINCT_DS_HLL";
+  private static final boolean ROUND = true;
+
+  @Override
+  public SqlAggFunction calciteFunction()
+  {
+return FUNCTION_INSTANCE;
+  }
+
+  @Nullable
+  @Override
+  public Aggregation toDruidAggregation(
+  PlannerContext plannerContext,
+  RowSignature rowSignature,
+  VirtualColumnRegistry virtualColumnRegistry,
+  RexBuilder rexBuilder,
+  String name,
+  AggregateCall aggregateCall,
+  Project project,
+  List existingAggregations,
+  boolean finalizeAggregations
+  )
+  {
+// Don't use Aggregations.getArgumentsForSimpleAggregator, since it won't 
let us use direct column access
 
 Review comment:
   Refactored this to use a base abstract class (also did the same for theta 
sketches).
   
   I also renamed the aggregators that produce a sketch object to 
`*SketchObjectSqlAggregator` from `*SketchSqlAggregator`


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] jon-wei commented on a change in pull request #8487: Add initial SQL support for non-expression sketch postaggs

[GitBox]

jon-wei commented on a change in pull request #8487: Add initial SQL support 
for non-expression sketch postaggs
URL: https://github.com/apache/incubator-druid/pull/8487#discussion_r335718720
 
 

 ##
 File path: 
extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/hll/sql/HllSketchSetUnionOperatorConversion.java
 ##
 @@ -0,0 +1,137 @@
+/*
+ * 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.aggregation.datasketches.hll.sql;
+
+import org.apache.calcite.rex.RexCall;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.SqlFunction;
+import org.apache.calcite.sql.SqlFunctionCategory;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.type.OperandTypes;
+import org.apache.calcite.sql.type.ReturnTypes;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.druid.query.aggregation.PostAggregator;
+import 
org.apache.druid.query.aggregation.datasketches.hll.HllSketchUnionPostAggregator;
+import org.apache.druid.sql.calcite.expression.DruidExpression;
+import org.apache.druid.sql.calcite.expression.OperatorConversions;
+import org.apache.druid.sql.calcite.expression.PostAggregatorVisitor;
+import org.apache.druid.sql.calcite.expression.SqlOperatorConversion;
+import org.apache.druid.sql.calcite.planner.Calcites;
+import org.apache.druid.sql.calcite.planner.PlannerContext;
+import org.apache.druid.sql.calcite.table.RowSignature;
+
+import javax.annotation.Nullable;
+import java.util.ArrayList;
+import java.util.List;
+
+public class HllSketchSetUnionOperatorConversion implements 
SqlOperatorConversion
+{
+  private static final SqlFunction SQL_FUNCTION = new SqlFunction(
+  "HLL_SKETCH_UNION",
+  SqlKind.OTHER_FUNCTION,
+  ReturnTypes.explicit(
+  factory -> Calcites.createSqlType(factory, SqlTypeName.OTHER)
+  ),
+  null,
+  OperandTypes.VARIADIC,
+  SqlFunctionCategory.USER_DEFINED_FUNCTION
+  );
+
+  public HllSketchSetUnionOperatorConversion()
+  {
+  }
+
+  @Override
+  public SqlOperator calciteOperator()
+  {
+return SQL_FUNCTION;
+  }
+
+  @Nullable
+  @Override
+  public DruidExpression toDruidExpression(
+  PlannerContext plannerContext,
+  RowSignature rowSignature,
+  RexNode rexNode
+  )
+  {
+return null;
+  }
+
+  @Nullable
+  @Override
+  public PostAggregator toPostAggregator(
+  PlannerContext plannerContext,
+  RowSignature rowSignature,
+  RexNode rexNode,
+  PostAggregatorVisitor postAggregatorVisitor
+  )
+  {
+final List operands = ((RexCall) rexNode).getOperands();
+final List inputPostAggs = new ArrayList<>();
+Integer lgK = null;
+String tgtHllType = null;
+
+int operandCounter = 0;
+for (RexNode operand : operands) {
+  final PostAggregator convertedPostAgg = 
OperatorConversions.toPostAggregator(
+  plannerContext,
+  rowSignature,
+  operand,
+  postAggregatorVisitor
+  );
+  if (convertedPostAgg == null) {
+if (operandCounter == 0) {
+  try {
+lgK = RexLiteral.intValue(operand);
 
 Review comment:
   Added sqlkind checks here and in similar places


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] jon-wei commented on a change in pull request #8487: Add initial SQL support for non-expression sketch postaggs

[GitBox]

jon-wei commented on a change in pull request #8487: Add initial SQL support 
for non-expression sketch postaggs
URL: https://github.com/apache/incubator-druid/pull/8487#discussion_r335718831
 
 

 ##
 File path: 
extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/hll/sql/HllSketchSetUnionOperatorConversion.java
 ##
 @@ -0,0 +1,137 @@
+/*
+ * 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.aggregation.datasketches.hll.sql;
+
+import org.apache.calcite.rex.RexCall;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.SqlFunction;
+import org.apache.calcite.sql.SqlFunctionCategory;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.type.OperandTypes;
+import org.apache.calcite.sql.type.ReturnTypes;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.druid.query.aggregation.PostAggregator;
+import 
org.apache.druid.query.aggregation.datasketches.hll.HllSketchUnionPostAggregator;
+import org.apache.druid.sql.calcite.expression.DruidExpression;
+import org.apache.druid.sql.calcite.expression.OperatorConversions;
+import org.apache.druid.sql.calcite.expression.PostAggregatorVisitor;
+import org.apache.druid.sql.calcite.expression.SqlOperatorConversion;
+import org.apache.druid.sql.calcite.planner.Calcites;
+import org.apache.druid.sql.calcite.planner.PlannerContext;
+import org.apache.druid.sql.calcite.table.RowSignature;
+
+import javax.annotation.Nullable;
+import java.util.ArrayList;
+import java.util.List;
+
+public class HllSketchSetUnionOperatorConversion implements 
SqlOperatorConversion
+{
+  private static final SqlFunction SQL_FUNCTION = new SqlFunction(
+  "HLL_SKETCH_UNION",
+  SqlKind.OTHER_FUNCTION,
+  ReturnTypes.explicit(
+  factory -> Calcites.createSqlType(factory, SqlTypeName.OTHER)
+  ),
+  null,
+  OperandTypes.VARIADIC,
+  SqlFunctionCategory.USER_DEFINED_FUNCTION
+  );
+
+  public HllSketchSetUnionOperatorConversion()
+  {
+  }
+
+  @Override
+  public SqlOperator calciteOperator()
+  {
+return SQL_FUNCTION;
+  }
+
+  @Nullable
+  @Override
+  public DruidExpression toDruidExpression(
+  PlannerContext plannerContext,
+  RowSignature rowSignature,
+  RexNode rexNode
+  )
+  {
+return null;
+  }
+
+  @Nullable
+  @Override
+  public PostAggregator toPostAggregator(
+  PlannerContext plannerContext,
+  RowSignature rowSignature,
+  RexNode rexNode,
+  PostAggregatorVisitor postAggregatorVisitor
+  )
+  {
+final List operands = ((RexCall) rexNode).getOperands();
+final List inputPostAggs = new ArrayList<>();
+Integer lgK = null;
+String tgtHllType = null;
+
+int operandCounter = 0;
+for (RexNode operand : operands) {
+  final PostAggregator convertedPostAgg = 
OperatorConversions.toPostAggregator(
+  plannerContext,
+  rowSignature,
+  operand,
+  postAggregatorVisitor
+  );
+  if (convertedPostAgg == null) {
+if (operandCounter == 0) {
+  try {
+lgK = RexLiteral.intValue(operand);
+  }
+  catch (RuntimeException re) {
 
 Review comment:
   Narrowed this to `ClassCastException` here and elsewhere


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] jon-wei commented on a change in pull request #8487: Add initial SQL support for non-expression sketch postaggs

[GitBox]

jon-wei commented on a change in pull request #8487: Add initial SQL support 
for non-expression sketch postaggs
URL: https://github.com/apache/incubator-druid/pull/8487#discussion_r335718915
 
 

 ##
 File path: 
extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/theta/sql/ThetaSketchSetBaseOperatorConversion.java
 ##
 @@ -0,0 +1,140 @@
+/*
+ * 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.aggregation.datasketches.theta.sql;
+
+import org.apache.calcite.rex.RexCall;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.SqlFunction;
+import org.apache.calcite.sql.SqlFunctionCategory;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.type.OperandTypes;
+import org.apache.calcite.sql.type.ReturnTypes;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.query.aggregation.PostAggregator;
+import 
org.apache.druid.query.aggregation.datasketches.theta.SketchSetPostAggregator;
+import org.apache.druid.sql.calcite.expression.DruidExpression;
+import org.apache.druid.sql.calcite.expression.OperatorConversions;
+import org.apache.druid.sql.calcite.expression.PostAggregatorVisitor;
+import org.apache.druid.sql.calcite.expression.SqlOperatorConversion;
+import org.apache.druid.sql.calcite.planner.Calcites;
+import org.apache.druid.sql.calcite.planner.PlannerContext;
+import org.apache.druid.sql.calcite.table.RowSignature;
+
+import javax.annotation.Nullable;
+import java.util.ArrayList;
+import java.util.List;
+
+public abstract class ThetaSketchSetBaseOperatorConversion implements 
SqlOperatorConversion
+{
+  public ThetaSketchSetBaseOperatorConversion()
+  {
+  }
+
+  @Override
+  public SqlOperator calciteOperator()
+  {
+return makeSqlFunction();
+  }
+
+  @Nullable
+  @Override
+  public DruidExpression toDruidExpression(
+  PlannerContext plannerContext,
+  RowSignature rowSignature,
+  RexNode rexNode
+  )
+  {
+return null;
+  }
+
+  @Nullable
+  @Override
+  public PostAggregator toPostAggregator(
+  PlannerContext plannerContext,
+  RowSignature rowSignature,
+  RexNode rexNode,
+  PostAggregatorVisitor postAggregatorVisitor
+  )
+  {
+final List operands = ((RexCall) rexNode).getOperands();
+final List inputPostAggs = new ArrayList<>();
+Integer size = null;
+
+int operandCounter = 0;
+for (RexNode operand : operands) {
+  final PostAggregator convertedPostAgg = 
OperatorConversions.toPostAggregator(
+  plannerContext,
+  rowSignature,
+  operand,
+  postAggregatorVisitor
+  );
+  if (convertedPostAgg == null) {
+if (operandCounter == 0) {
+  try {
+size = RexLiteral.intValue(operand);
 
 Review comment:
   Fixed


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] jon-wei commented on a change in pull request #8487: Add initial SQL support for non-expression sketch postaggs

[GitBox]

jon-wei commented on a change in pull request #8487: Add initial SQL support 
for non-expression sketch postaggs
URL: https://github.com/apache/incubator-druid/pull/8487#discussion_r335718988
 
 

 ##
 File path: 
extensions-core/datasketches/src/main/java/org/apache/druid/query/aggregation/datasketches/theta/sql/ThetaSketchSetBaseOperatorConversion.java
 ##
 @@ -0,0 +1,140 @@
+/*
+ * 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.aggregation.datasketches.theta.sql;
+
+import org.apache.calcite.rex.RexCall;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.SqlFunction;
+import org.apache.calcite.sql.SqlFunctionCategory;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.type.OperandTypes;
+import org.apache.calcite.sql.type.ReturnTypes;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.query.aggregation.PostAggregator;
+import 
org.apache.druid.query.aggregation.datasketches.theta.SketchSetPostAggregator;
+import org.apache.druid.sql.calcite.expression.DruidExpression;
+import org.apache.druid.sql.calcite.expression.OperatorConversions;
+import org.apache.druid.sql.calcite.expression.PostAggregatorVisitor;
+import org.apache.druid.sql.calcite.expression.SqlOperatorConversion;
+import org.apache.druid.sql.calcite.planner.Calcites;
+import org.apache.druid.sql.calcite.planner.PlannerContext;
+import org.apache.druid.sql.calcite.table.RowSignature;
+
+import javax.annotation.Nullable;
+import java.util.ArrayList;
+import java.util.List;
+
+public abstract class ThetaSketchSetBaseOperatorConversion implements 
SqlOperatorConversion
+{
+  public ThetaSketchSetBaseOperatorConversion()
+  {
+  }
+
+  @Override
+  public SqlOperator calciteOperator()
+  {
+return makeSqlFunction();
+  }
+
+  @Nullable
+  @Override
+  public DruidExpression toDruidExpression(
+  PlannerContext plannerContext,
+  RowSignature rowSignature,
+  RexNode rexNode
+  )
+  {
+return null;
+  }
+
+  @Nullable
+  @Override
+  public PostAggregator toPostAggregator(
+  PlannerContext plannerContext,
+  RowSignature rowSignature,
+  RexNode rexNode,
+  PostAggregatorVisitor postAggregatorVisitor
+  )
+  {
+final List operands = ((RexCall) rexNode).getOperands();
+final List inputPostAggs = new ArrayList<>();
+Integer size = null;
+
+int operandCounter = 0;
+for (RexNode operand : operands) {
+  final PostAggregator convertedPostAgg = 
OperatorConversions.toPostAggregator(
+  plannerContext,
+  rowSignature,
+  operand,
+  postAggregatorVisitor
+  );
+  if (convertedPostAgg == null) {
+if (operandCounter == 0) {
+  try {
+size = RexLiteral.intValue(operand);
+  }
+  catch (RuntimeException re) {
+return null;
+  }
+} else {
+  return null;
+}
+  } else {
+inputPostAggs.add(convertedPostAgg);
+operandCounter++;
+  }
+}
+
+return new SketchSetPostAggregator(
+postAggregatorVisitor.getOutputNamePrefix() + 
postAggregatorVisitor.getAndIncrementCounter(),
+getSetOperationName(),
+size,
+inputPostAggs
+);
+  }
+
+  private SqlFunction makeSqlFunction()
+  {
+return new SqlFunction(
+getFunctionName(),
+SqlKind.OTHER_FUNCTION,
+ReturnTypes.explicit(
+factory -> Calcites.createSqlType(factory, SqlTypeName.OTHER)
+),
+null,
+OperandTypes.VARIADIC,
+SqlFunctionCategory.USER_DEFINED_FUNCTION
+);
+  }
+
+  public String getSetOperationName()
+  {
+throw new UnsupportedOperationException("getSetOperationName() is not 
implemented.");
 
 Review comment:
   Changed this to abstract method


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 que

[GitHub] [incubator-druid] gianm commented on issue #8686: Router management Proxy routing should have a way to route to any node in the cluster

[GitBox]

gianm commented on issue #8686: Router management Proxy routing should have a 
way to route to any node in the cluster
URL: 
https://github.com/apache/incubator-druid/issues/8686#issuecomment-542903041
 
 
   This sounds like an open proxy and therefore I am scared of it.
   
   In the context of the Druid Doctor I think it would be better to put the 
checks into the Router itself, and have it contact other nodes on the caller's 
behalf if necessary, rather than run an open proxy.
   
   But even then, the issue implies you are thinking about contacting all 
servers in a cluster as part of a health check. For large clusters this has 
potential scalability problems. It might be better for the servers to all 
self-check themselves, and report the results of the self-check to a central 
location. Maybe they can report a hash of the important properties, and if the 
hashes differ we know there is a proxy.


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] gianm commented on issue #8686: Router management Proxy routing should have a way to route to any node in the cluster

[GitBox]

gianm commented on issue #8686: Router management Proxy routing should have a 
way to route to any node in the cluster
URL: 
https://github.com/apache/incubator-druid/issues/8686#issuecomment-542903549
 
 
   > It might be better for the servers to all self-check themselves, and 
report the results of the self-check to a central location. 
   
   Of course this has scalability problems too 🙂
   
   I thought it might be worth considering, though, because I am worried that 
since the Doctor is user-driven, it could cause stampedes of broadcast blasts 
to all servers in the 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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] gianm edited a comment on issue #8686: Router management Proxy routing should have a way to route to any node in the cluster

[GitBox]

gianm edited a comment on issue #8686: Router management Proxy routing should 
have a way to route to any node in the cluster
URL: 
https://github.com/apache/incubator-druid/issues/8686#issuecomment-542903549
 
 
   > It might be better for the servers to all self-check themselves, and 
report the results of the self-check to a central location. 
   
   Of course this has scalability problems too 🙂
   
   I thought it might be worth considering, though, because I am worried that 
since the Doctor is user-driven, it could cause stampedes of broadcast blasts 
to all servers in the cluster if too many users use it at once.


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] xvrl commented on a change in pull request #6702: Add SelfDiscoveryResource; rename org.apache.druid.discovery.NodeType to NodeRole

[GitBox]

xvrl commented on a change in pull request #6702: Add SelfDiscoveryResource; 
rename org.apache.druid.discovery.NodeType to NodeRole
URL: https://github.com/apache/incubator-druid/pull/6702#discussion_r335746585
 
 

 ##
 File path: integration-tests/docker/docker-entrypoint.sh
 ##
 @@ -0,0 +1,27 @@
+#!/bin/bash -eu
+
+# 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.
+
+if [[ "$OSTYPE" == "darwin"* ]]; then
+  # On Mac OS X resolveip may not be available
+  export HOST_IP=$(dig +short $HOSTNAME | awk '{ print ; exit }')
 
 Review comment:
   why not use `dig +short $HOSTNAME` everywhere?


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] aditya-r-m commented on issue #8682: implement FiniteFirehoseFactory in InlineFirehose

[GitBox]

aditya-r-m commented on issue #8682: implement FiniteFirehoseFactory in 
InlineFirehose
URL: https://github.com/apache/incubator-druid/pull/8682#issuecomment-542927214
 
 
   @jihoonson implemented the suggested updates.


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] jnaous opened a new pull request #8687: Update tutorial-rollup.md

[GitBox]

jnaous opened a new pull request #8687: Update tutorial-rollup.md
URL: https://github.com/apache/incubator-druid/pull/8687
 
 
   At this point there hasn't yet been an explanation in the tutorial of what 
"segments" are
   


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] JulianJaffePinterest commented on issue #8685: Allow bootstrap.servers to be provided via a PasswordProvider for Kafka Ingestion

[GitBox]

JulianJaffePinterest commented on issue #8685: Allow bootstrap.servers to be 
provided via a PasswordProvider for Kafka Ingestion
URL: 
https://github.com/apache/incubator-druid/issues/8685#issuecomment-542942083
 
 
   I agree that allowing most properties to be specified via a Provider makes 
sense. I think we'd have to figure out a good solution to # or else users 
could encounter some very hard to debug Heisenbug-style errors though.


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] jihoonson opened a new pull request #8688: Remove commit() method Firehose

[GitBox]

jihoonson opened a new pull request #8688: Remove commit() method Firehose
URL: https://github.com/apache/incubator-druid/pull/8688
 
 
   ### Description
   
   `commit()` method was used only by the ancient kafka firehose which was 
removed in https://github.com/apache/incubator-druid/pull/8020. 
   
   
   
   This PR has:
   - [x] been self-reviewed.
  - [ ] using the [concurrency 
checklist](https://github.com/apache/incubator-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/incubator-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.
   - [ ] 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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] vogievetsky commented on issue #8686: Router management Proxy routing should have a way to route to any node in the cluster

[GitBox]

vogievetsky commented on issue #8686: Router management Proxy routing should 
have a way to route to any node in the cluster
URL: 
https://github.com/apache/incubator-druid/issues/8686#issuecomment-542959319
 
 
   The proxy could only forward to Druid nodes themselves so it is not open to 
everything. Also there is already an open proxy in the sampler (http firehose). 
I tried to implement it that way and it works for non TLS clusters, (for TLS 
clusters we would need to address: 
https://github.com/apache/incubator-druid/issues/8668
   
   From a security perspective my model is that if you have admin access to the 
management proxy enabled router you can pretty much do anything anyway at that 
point.
   
   From a scalability perspective I do not think it is a problem if this is a 
user initiated action (with a warning in the UI). Also remember that it would 
be OK for this to be slow, taking a 100ms pause between every request. People 
would be fine with it as long as there is visual feedback with a ticking 
`Contacting node 12 out of 343` that shows that progress is being made.


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] gianm commented on issue #8686: Router management Proxy routing should have a way to route to any node in the cluster

[GitBox]

gianm commented on issue #8686: Router management Proxy routing should have a 
way to route to any node in the cluster
URL: 
https://github.com/apache/incubator-druid/issues/8686#issuecomment-542964407
 
 
   > Also there is already an open proxy in the sampler (http firehose).
   
   I think it's different, because the http firehose at least can't be used 
unless you are an authenticated user and have write access to some datasource. 
(Not ideal but there is at least something that protects it.) The proxy route 
IIRC would be open to anyone anywhere. I believe it relies on the target server 
to do authentication.
   
   Limiting it to just Druid servers could lower the scariness levels enough.
   
   > From a scalability perspective I do not think it is a problem if this is a 
user initiated action (with a warning in the UI). Also remember that it would 
be OK for this to be slow, taking a 100ms pause between every request.
   
   I think with the pause, the scalability issue is less scary, so that sounds 
good. Or, we could also limit it to a certain number of servers. Probably if 
someone has, like, 100 servers, then they have figured out how to set their 
properties properly by then.


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] gianm merged pull request #8687: Update tutorial-rollup.md

[GitBox]

gianm merged pull request #8687: Update tutorial-rollup.md
URL: https://github.com/apache/incubator-druid/pull/8687
 
 
   


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


With regards,
Apache Git Services

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

[incubator-druid] branch master updated (89ce638 -> 9f4e11d)

[gian]

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/incubator-druid.git.


from 89ce638  More Kinesis resharding adjustments (#8671)
 add 9f4e11d  Update tutorial-rollup.md (#8687)

No new revisions were added by this update.

Summary of changes:
 docs/tutorials/tutorial-rollup.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] [incubator-druid] vogievetsky commented on issue #8686: Router management Proxy routing should have a way to route to any node in the cluster

[GitBox]

vogievetsky commented on issue #8686: Router management Proxy routing should 
have a way to route to any node in the cluster
URL: 
https://github.com/apache/incubator-druid/issues/8686#issuecomment-542968137
 
 
   Yeah just to be clear I never pictured it to be a proxy to an arbitrary 
destination only to other Druid servers. If there was some server uuid we could 
use that instead of a host port combo but right now from the sys table the best 
server id I can see is host:port
   
   
![image](https://user-images.githubusercontent.com/177816/66972578-dba7d900-f049-11e9-9424-8d5514db4ffd.png)
   
   The issue with having this check be encapsulated by a single backend API is 
that it might give someone the idea to hit it as part of a routine health 
monitoring which limits how heavy these tests might should be. I am very much 
picturing a 'deep' test suite (that can take its sweet time) and might be 
intensive on the cluster. Something you run a handful of times in the clusters 
lifetime (in leu of posting to the user-group). Something to catch issues like: 
https://groups.google.com/forum/#!topic/druid-user/w3Uy-dXZSII
   
   


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] lgtm-com[bot] commented on issue #8688: Remove commit() method Firehose

[GitBox]

lgtm-com[bot] commented on issue #8688: Remove commit() method Firehose
URL: https://github.com/apache/incubator-druid/pull/8688#issuecomment-542968321
 
 
   This pull request **fixes 2 alerts** when merging 
6038515d62b8271de839f8c5f9c59d8a002382c2 into 
89ce6384f57cb13c776cfcc9fc7b97126b834fac - [view on 
LGTM.com](https://lgtm.com/projects/g/apache/incubator-druid/rev/pr-91f811a236674d4366d900637d87181ee98ed9fe)
   
   **fixed alerts:**
   
   * 2 for Dereferenced variable may be null


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] fjy closed issue #8673: InlineFirehose does not work with index_parallel ingestion

[GitBox]

fjy closed issue #8673: InlineFirehose does not work with index_parallel 
ingestion
URL: https://github.com/apache/incubator-druid/issues/8673
 
 
   


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] fjy merged pull request #8682: implement FiniteFirehoseFactory in InlineFirehose

[GitBox]

fjy merged pull request #8682: implement FiniteFirehoseFactory in InlineFirehose
URL: https://github.com/apache/incubator-druid/pull/8682
 
 
   


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


With regards,
Apache Git Services

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

[incubator-druid] branch master updated (9f4e11d -> 75527f0)

[fjy]

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

fjy pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-druid.git.


from 9f4e11d  Update tutorial-rollup.md (#8687)
 add 75527f0  implement FiniteFirehoseFactory in InlineFirehose (#8682)

No new revisions were added by this update.

Summary of changes:
 .../realtime/firehose/InlineFirehoseFactory.java   | 30 --
 .../firehose/InlineFirehoseFactoryTest.java| 21 +++
 2 files changed, 49 insertions(+), 2 deletions(-)


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

[GitHub] [incubator-druid] jnaous commented on issue #8686: Router management Proxy routing should have a way to route to any node in the cluster

[GitBox]

jnaous commented on issue #8686: Router management Proxy routing should have a 
way to route to any node in the cluster
URL: 
https://github.com/apache/incubator-druid/issues/8686#issuecomment-542970766
 
 
   Something like that could be implemented by making the request async: when
   a check request is sent, servers return immediately with "Started". If
   tests are already running, and more requests come in, we continue to return
   "started". Then the healthcheck result is cached for a few minutes or
   whatever the right time is, during which the servers return the last
   healthcheck result.
   
   On Wed, Oct 16, 2019 at 7:25 PM Vadim Ogievetsky 
   wrote:
   
   > Yeah just to be clear I never pictured it to be a proxy to an arbitrary
   > destination only to other Druid servers. If there was some server uuid we
   > could use that instead of a host port combo but right now from the sys
   > table the best server id I can see is host:port
   >
   > [image: image]
   > 

   >
   > The issue with having this check be encapsulated by a single backend API
   > is that it might give someone the idea to hit it as part of a routine
   > health monitoring which limits how heavy these tests might should be. I am
   > very much picturing a 'deep' test suite (that can take its sweet time) and
   > might be intensive on the cluster. Something you run a handful of times in
   > the clusters lifetime (in leu of posting to the user-group). Something to
   > catch issues like:
   > https://groups.google.com/forum/#!topic/druid-user/w3Uy-dXZSII
   >
   > —
   > You are receiving this because you are subscribed to this thread.
   > Reply to this email directly, view it on GitHub
   > 
,
   > or unsubscribe
   > 

   > .
   >
   
   
   -- 
   Jad Naous
   Imply | VP R&D
   650-521-3425
   jad.na...@imply.io
   


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


With regards,
Apache Git Services

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

[GitHub] [incubator-druid] jnaous opened a new pull request #8689: Update tutorial-kerberos-hadoop.md

[GitBox]

jnaous opened a new pull request #8689: Update tutorial-kerberos-hadoop.md
URL: https://github.com/apache/incubator-druid/pull/8689
 
 
   Fix up what looks like a bad merge.
   


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


With regards,
Apache Git Services

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