[GitHub] [incubator-pinot] codecov-io edited a comment on issue #4133: add more types parsing for string coerceValueIntoField
codecov-io edited a comment on issue #4133: add more types parsing for string coerceValueIntoField URL: https://github.com/apache/incubator-pinot/pull/4133#issuecomment-484348066 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4133?src=pr=h1) Report > Merging [#4133](https://codecov.io/gh/apache/incubator-pinot/pull/4133?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/c0120d72d79b176ffc955496dba3662bb018afe4?src=pr=desc) will **increase** coverage by `7.28%`. > The diff coverage is `0%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/4133?src=pr=tree) ```diff @@ Coverage Diff @@ ## master#4133 +/- ## + Coverage 59.95% 67.24% +7.28% Complexity 20 20 Files 1161 1040 -121 Lines 5846751483-6984 Branches 8121 7210 -911 - Hits 3505234618 -434 + Misses2096714499-6468 + Partials 2448 2366 -82 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/4133?src=pr=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...a/org/apache/pinot/common/config/Deserializer.java](https://codecov.io/gh/apache/incubator-pinot/pull/4133/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL0Rlc2VyaWFsaXplci5qYXZh) | `54.86% <0%> (-2.28%)` | `0 <0> (ø)` | | | [...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4133/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==) | `75% <0%> (-25%)` | `0% <0%> (ø)` | | | [...egation/function/customobject/MinMaxRangePair.java](https://codecov.io/gh/apache/incubator-pinot/pull/4133/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9jdXN0b21vYmplY3QvTWluTWF4UmFuZ2VQYWlyLmphdmE=) | `75.86% <0%> (-24.14%)` | `0% <0%> (ø)` | | | [...ing/HelixExternalViewBasedTimeBoundaryService.java](https://codecov.io/gh/apache/incubator-pinot/pull/4133/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9IZWxpeEV4dGVybmFsVmlld0Jhc2VkVGltZUJvdW5kYXJ5U2VydmljZS5qYXZh) | `64.91% <0%> (-12.37%)` | `0% <0%> (ø)` | | | [...r/validation/RealtimeSegmentValidationManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4133/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci92YWxpZGF0aW9uL1JlYWx0aW1lU2VnbWVudFZhbGlkYXRpb25NYW5hZ2VyLmphdmE=) | `43.33% <0%> (-11.67%)` | `0% <0%> (ø)` | | | [...ot/core/segment/index/readers/BytesDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4133/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L2luZGV4L3JlYWRlcnMvQnl0ZXNEaWN0aW9uYXJ5LmphdmE=) | `91.66% <0%> (-8.34%)` | `0% <0%> (ø)` | | | [...impl/dictionary/DoubleOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4133/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT25IZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==) | `69.56% <0%> (-6%)` | `0% <0%> (ø)` | | | [...lix/core/minion/PinotHelixTaskResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4133/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9QaW5vdEhlbGl4VGFza1Jlc291cmNlTWFuYWdlci5qYXZh) | `73.25% <0%> (-4.66%)` | `0% <0%> (ø)` | | | [...pinot/core/query/pruner/AbstractSegmentPruner.java](https://codecov.io/gh/apache/incubator-pinot/pull/4133/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9wcnVuZXIvQWJzdHJhY3RTZWdtZW50UHJ1bmVyLmphdmE=) | `72.72% <0%> (-4.2%)` | `0% <0%> (ø)` | | | [...n/java/org/apache/pinot/common/utils/LLCUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/4133/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvTExDVXRpbHMuamF2YQ==) | `75% <0%> (-3.58%)` | `0% <0%> (ø)` | | | ... and [228 more](https://codecov.io/gh/apache/incubator-pinot/pull/4133/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4133?src=pr=continue). > **Legend** - [Click here to learn
[GitHub] [incubator-pinot] kishoreg commented on issue #4214: Support default BYTES (zero-length byte array) in aggregation function and aggregator
kishoreg commented on issue #4214: Support default BYTES (zero-length byte array) in aggregation function and aggregator URL: https://github.com/apache/incubator-pinot/pull/4214#issuecomment-494230068 This is equivalent of supporting NULL for BYTES column right? 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] katewang0710 commented on a change in pull request #4133: add more types parsing for string coerceValueIntoField
katewang0710 commented on a change in pull request #4133: add more types
parsing for string coerceValueIntoField
URL: https://github.com/apache/incubator-pinot/pull/4133#discussion_r285840014
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/config/Deserializer.java
##
@@ -349,17 +349,26 @@
} else if (value instanceof String) {
String stringValue = (String) value;
try {
-if (Integer.class.isAssignableFrom(fieldType) || int.class
-.isAssignableFrom(fieldType)) { // field instanceof int/Integer
+if (fieldType.isAssignableFrom(Integer.class) ||
fieldType.isAssignableFrom(int.class)){
destinationValue = Integer.parseInt(stringValue);
-} else if (Long.class.isAssignableFrom(fieldType)) { // field
instanceof long/Long
+} else if (fieldType.isAssignableFrom(Long.class) ||
fieldType.isAssignableFrom(long.class)){
destinationValue = Long.parseLong(stringValue);
-} else if (Boolean.class.isAssignableFrom(fieldType) ||
boolean.class.isAssignableFrom(fieldType)) {
+} else if (fieldType.isAssignableFrom(Boolean.class) ||
fieldType.isAssignableFrom(boolean.class)){
destinationValue = Boolean.parseBoolean(stringValue);
-} else if (Enum.class.isAssignableFrom(fieldType)) {
+} else if (Enum.class.isAssignableFrom(fieldType)){
destinationValue =
Enum.valueOf(fieldType.asSubclass(Enum.class), stringValue.toUpperCase());
-} else {
- // TODO Add float/double/short/char
+} else if (fieldType.isAssignableFrom(Float.class) ||
fieldType.isAssignableFrom(float.class)){
+ destinationValue = Float.parseFloat(stringValue);
+} else if (fieldType.isAssignableFrom(Double.class) ||
fieldType.isAssignableFrom(double.class)){
+ destinationValue = Double.parseDouble(stringValue);
+} else if (fieldType.isAssignableFrom(Short.class) ||
fieldType.isAssignableFrom(short.class)){
+ destinationValue = Short.parseShort(stringValue);
+} else if (fieldType.isAssignableFrom(Byte.class) ||
fieldType.isAssignableFrom(byte.class)){
+ destinationValue = stringValue.getBytes()[0];
Review comment:
Exactly! 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...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io edited a comment on issue #4218: Add RealtimeConsumptionCatchupServiceCallback
codecov-io edited a comment on issue #4218: Add RealtimeConsumptionCatchupServiceCallback URL: https://github.com/apache/incubator-pinot/pull/4218#issuecomment-493596443 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4218?src=pr=h1) Report > Merging [#4218](https://codecov.io/gh/apache/incubator-pinot/pull/4218?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/3e6733f820d8f3b0482849abe6e1705da38e18a9?src=pr=desc) will **decrease** coverage by `0.13%`. > The diff coverage is `36.58%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/4218?src=pr=tree) ```diff @@ Coverage Diff @@ ## master#4218 +/- ## - Coverage 67.35% 67.22% -0.14% Complexity 20 20 Files 1036 1040 +4 Lines 5153551513 -22 Branches 7220 7215 -5 - Hits 3471234629 -83 - Misses1443814524 +86 + Partials 2385 2360 -25 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/4218?src=pr=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...org/apache/pinot/common/utils/CommonConstants.java](https://codecov.io/gh/apache/incubator-pinot/pull/4218/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `45.65% <ø> (+10.86%)` | `0 <0> (ø)` | :arrow_down: | | [...g/apache/pinot/common/config/TableNameBuilder.java](https://codecov.io/gh/apache/incubator-pinot/pull/4218/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL1RhYmxlTmFtZUJ1aWxkZXIuamF2YQ==) | `96.15% <100%> (+0.15%)` | `0 <0> (ø)` | :arrow_down: | | [...pinot/server/starter/helix/HelixServerStarter.java](https://codecov.io/gh/apache/incubator-pinot/pull/4218/diff?src=pr=tree#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9IZWxpeFNlcnZlclN0YXJ0ZXIuamF2YQ==) | `54.98% <100%> (ø)` | `0 <0> (ø)` | :arrow_down: | | [...a/org/apache/pinot/common/utils/ServiceStatus.java](https://codecov.io/gh/apache/incubator-pinot/pull/4218/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvU2VydmljZVN0YXR1cy5qYXZh) | `73.5% <31.57%> (-9.84%)` | `0 <0> (ø)` | | | [...apache/pinot/common/metrics/ValidationMetrics.java](https://codecov.io/gh/apache/incubator-pinot/pull/4218/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9WYWxpZGF0aW9uTWV0cmljcy5qYXZh) | `20.28% <0%> (-59.43%)` | `0% <0%> (ø)` | | | [...ller/validation/OfflineSegmentIntervalChecker.java](https://codecov.io/gh/apache/incubator-pinot/pull/4218/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci92YWxpZGF0aW9uL09mZmxpbmVTZWdtZW50SW50ZXJ2YWxDaGVja2VyLmphdmE=) | `29.87% <0%> (-53.25%)` | `0% <0%> (ø)` | | | [...r/validation/RealtimeSegmentValidationManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4218/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci92YWxpZGF0aW9uL1JlYWx0aW1lU2VnbWVudFZhbGlkYXRpb25NYW5hZ2VyLmphdmE=) | `43.33% <0%> (-41.67%)` | `0% <0%> (ø)` | | | [...e/operator/dociditerators/MVScanDocIdIterator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4218/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZGl0ZXJhdG9ycy9NVlNjYW5Eb2NJZEl0ZXJhdG9yLmphdmE=) | `45.45% <0%> (-18.19%)` | `0% <0%> (ø)` | | | [.../impl/dictionary/LongOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4218/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09mZkhlYXBNdXRhYmxlRGljdGlvbmFyeS5qYXZh) | `83.92% <0%> (-8.8%)` | `0% <0%> (ø)` | | | [...ot/core/segment/index/readers/BytesDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4218/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L2luZGV4L3JlYWRlcnMvQnl0ZXNEaWN0aW9uYXJ5LmphdmE=) | `91.66% <0%> (-8.34%)` | `0% <0%> (ø)` | | | ... and [51 more](https://codecov.io/gh/apache/incubator-pinot/pull/4218/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4218?src=pr=continue). > **Legend** - [Click here to learn
[incubator-pinot] branch num-groups-limit deleted (was f48abe7)
This is an automated email from the ASF dual-hosted git repository. jackie pushed a change to branch num-groups-limit in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. was f48abe7 Increase the limit of groups in combine operator (inter-segments) The revisions that were on this branch are still contained in other references; therefore, this change does not discard any commits from the repository. - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch master updated: Increase the limit of groups in combine operator (inter-segments) (#4208)
This is an automated email from the ASF dual-hosted git repository.
jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
The following commit(s) were added to refs/heads/master by this push:
new fc51fdc Increase the limit of groups in combine operator
(inter-segments) (#4208)
fc51fdc is described below
commit fc51fdc01d2b93836346c723107ccbcd5dbc6cdc
Author: Xiaotian (Jackie) Jiang <1751+jackie-ji...@users.noreply.github.com>
AuthorDate: Mon May 20 18:12:47 2019 -0700
Increase the limit of groups in combine operator (inter-segments) (#4208)
Right now we limit number of groups stored both inner-segment and
inter-segments with the same limit. Once the segment results reach
the inner-segment limit, it will also reach the inter-segments
limit after combining the results in the combine operator. This
will cause the new groups from the other segment being dropped.
This might cause inconsistent result depending on the order of
segment execution.
The solution would be, increase the limit (multiply by factor 2) of
groups across segments to get consistent result regardless of the
order of execution. For most cases, most groups from each segment
should be the same, thus the total number of groups across segments
should be equal or slightly higher than the number of groups in
each segment. We still put a limit across segments to protect cases
where data is very skewed across different segments.
---
.../pinot/core/operator/CombineGroupByOperator.java | 17 +
.../org/apache/pinot/core/plan/CombinePlanNode.java | 1 +
.../pinot/core/plan/maker/InstancePlanMakerImplV2.java | 2 +-
3 files changed, 15 insertions(+), 5 deletions(-)
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/operator/CombineGroupByOperator.java
b/pinot-core/src/main/java/org/apache/pinot/core/operator/CombineGroupByOperator.java
index dee7fd7..99cf0c3 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/operator/CombineGroupByOperator.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/operator/CombineGroupByOperator.java
@@ -54,10 +54,17 @@ public class CombineGroupByOperator extends
BaseOperator _operators;
private final BrokerRequest _brokerRequest;
private final ExecutorService _executorService;
private final long _timeOutMs;
+ // Limit on number of groups stored for each segment, beyond which no new
group will be created
private final int _numGroupsLimit;
public CombineGroupByOperator(List operators, BrokerRequest
brokerRequest, ExecutorService executorService,
@@ -96,6 +103,7 @@ public class CombineGroupByOperator extends
BaseOperator resultsMap = new ConcurrentHashMap<>();
AtomicInteger numGroups = new AtomicInteger();
+int interSegmentNumGroupsLimit = _numGroupsLimit *
INTER_SEGMENT_NUM_GROUPS_LIMIT_FACTOR;
ConcurrentLinkedQueue mergedProcessingExceptions =
new ConcurrentLinkedQueue<>();
AggregationFunctionContext[] aggregationFunctionContexts =
@@ -134,8 +142,7 @@ public class CombineGroupByOperator extends
BaseOperator {
if (value == null) {
-if (numGroups.get() < _numGroupsLimit) {
- numGroups.getAndIncrement();
+if (numGroups.getAndIncrement() <
interSegmentNumGroupsLimit) {
value = new Object[numAggregationFunctions];
for (int i = 0; i < numAggregationFunctions; i++) {
value[i] =
aggregationGroupByResult.getResultForKey(groupKey, i);
@@ -198,8 +205,10 @@ public class CombineGroupByOperator extends
BaseOperator= _numGroupsLimit) {
+
+ // TODO: this value should be set in the inner-segment operators.
Setting it here might cause false positive as we
+ // are comparing number of groups across segments with the groups
limit for each segment.
+ if (resultsMap.size() >= _numGroupsLimit) {
mergedBlock.setNumGroupsLimitReached(true);
}
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/plan/CombinePlanNode.java
b/pinot-core/src/main/java/org/apache/pinot/core/plan/CombinePlanNode.java
index ee3c888..c0cce5a 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/plan/CombinePlanNode.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/plan/CombinePlanNode.java
@@ -56,6 +56,7 @@ public class CombinePlanNode implements PlanNode {
* @param brokerRequest Broker request
* @param executorService Executor service
* @param timeOutMs Time out in milliseconds for query execution (not for
planning phase)
+ * @param numGroupsLimit Limit of number of groups stored in each segment
*/
public CombinePlanNode(List planNodes, BrokerRequest
brokerRequest, ExecutorService executorService,
long timeOutMs, int numGroupsLimit) {
diff --git
[GitHub] [incubator-pinot] Jackie-Jiang merged pull request #4208: Increase the limit of groups in combine operator (inter-segments)
Jackie-Jiang merged pull request #4208: Increase the limit of groups in combine operator (inter-segments) URL: https://github.com/apache/incubator-pinot/pull/4208 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang edited a comment on issue #4214: Support default BYTES (zero-length byte array) in aggregation function and aggregator
Jackie-Jiang edited a comment on issue #4214: Support default BYTES (zero-length byte array) in aggregation function and aggregator URL: https://github.com/apache/incubator-pinot/pull/4214#issuecomment-494201220 @kishoreg Without this PR, in order to create default columns for serialized Object such as HyperLogLog, TDigest etc., user needs to serialize the default Object (empty HyperLogLog or TDigest) in order to get the default value and then put it into the schema. The default values are quite long for these Objects (HyperLogLog: "000800ac"; QDigest: "3faa0004b9467fff8000"; TDigest: "00017ff0fff04059"). With this PR, we directly treat empty byte array as null, so user doesn't need to specify such default values. Besides, this PR allows user to put empty byte array to skip a value which can save storage and reduce computation cost. E.g. for a HyperLogLog column, if there is no value inside, user can simply put an empty array instead of the serialized byte array. 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang edited a comment on issue #4214: Support default BYTES (zero-length byte array) in aggregation function and aggregator
Jackie-Jiang edited a comment on issue #4214: Support default BYTES (zero-length byte array) in aggregation function and aggregator URL: https://github.com/apache/incubator-pinot/pull/4214#issuecomment-494201220 @kishoreg Without this PR, in order to create default columns for serialized Object such as HyperLogLog, TDigest etc., user needs to serialize the default Object (empty HyperLogLog or TDigest) in order to get the default value and then put it into the schema. The default values are quite long for these Objects (HyperLogLog: "000800ac"; QDigest: "3faa0004b9467fff8000"; TDigest: "00017ff0fff04059"). With this PR, we directly treat empty byte array as null, so user doesn't need to specify such default values. Besides, this PR allows user to put empty byte array to skip a value which can save storage and reduce computation cost. E.g. for one record, there is not value in the HyperLogLog, user can simply put an empty array instead of the serialized byte array. 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on issue #4214: Support default BYTES (zero-length byte array) in aggregation function and aggregator
Jackie-Jiang commented on issue #4214: Support default BYTES (zero-length byte array) in aggregation function and aggregator URL: https://github.com/apache/incubator-pinot/pull/4214#issuecomment-494201220 @kishoreg Without this PR, in order to create default columns for serialized Object such as HyperLogLog, TDigest etc., user needs to serialize the default Object (empty HyperLogLog or TDigest) in order to get the default value and then put it into the schema. The default values are quite long for these Objects (HyperLogLog: "000800ac"; QDigest: "3faa0004b9467fff8000"; TDigest: "00017ff0fff04059"). With this PR, we directly treat empty byte array as null, so user doesn't need to specify such default values. 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] kishoreg commented on issue #4214: Support default BYTES (zero-length byte array) in aggregation function and aggregator
kishoreg commented on issue #4214: Support default BYTES (zero-length byte array) in aggregation function and aggregator URL: https://github.com/apache/incubator-pinot/pull/4214#issuecomment-494199425 Can you point us to a concrete query/usecase where this PR would help? What would be the behavior without 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io edited a comment on issue #4222: Add startup/shutdown checks for HelixServerStarter
codecov-io edited a comment on issue #4222: Add startup/shutdown checks for HelixServerStarter URL: https://github.com/apache/incubator-pinot/pull/4222#issuecomment-493717771 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4222?src=pr=h1) Report > Merging [#4222](https://codecov.io/gh/apache/incubator-pinot/pull/4222?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/d2087b365859c215a55900a47b3f9a9355808157?src=pr=desc) will **increase** coverage by `0.07%`. > The diff coverage is `27.33%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/4222?src=pr=tree) ```diff @@ Coverage Diff @@ ## master#4222 +/- ## + Coverage 67.15% 67.22% +0.07% Complexity 20 20 Files 1036 1039 +3 Lines 5153651528 -8 Branches 7220 7229 +9 + Hits 3460734639 +32 + Misses1455714518 -39 + Partials 2372 2371 -1 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/4222?src=pr=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...org/apache/pinot/common/utils/CommonConstants.java](https://codecov.io/gh/apache/incubator-pinot/pull/4222/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `45.65% <ø> (+10.86%)` | `0 <0> (ø)` | :arrow_down: | | [...pinot/server/starter/helix/HelixServerStarter.java](https://codecov.io/gh/apache/incubator-pinot/pull/4222/diff?src=pr=tree#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9IZWxpeFNlcnZlclN0YXJ0ZXIuamF2YQ==) | `46.28% <27.33%> (-8.7%)` | `0 <0> (ø)` | | | [...er/validation/BrokerResourceValidationManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4222/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci92YWxpZGF0aW9uL0Jyb2tlclJlc291cmNlVmFsaWRhdGlvbk1hbmFnZXIuamF2YQ==) | `25% <0%> (-56.25%)` | `0% <0%> (ø)` | | | [...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4222/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==) | `50% <0%> (-50%)` | `0% <0%> (ø)` | | | [.../apache/pinot/core/transport/DataTableHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/4222/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS90cmFuc3BvcnQvRGF0YVRhYmxlSGFuZGxlci5qYXZh) | `88% <0%> (-12%)` | `0% <0%> (ø)` | | | [...elix/core/relocation/RealtimeSegmentRelocator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4222/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlbG9jYXRpb24vUmVhbHRpbWVTZWdtZW50UmVsb2NhdG9yLmphdmE=) | `73.62% <0%> (-8.8%)` | `0% <0%> (ø)` | | | [...pache/pinot/core/util/SortedRangeIntersection.java](https://codecov.io/gh/apache/incubator-pinot/pull/4222/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL1NvcnRlZFJhbmdlSW50ZXJzZWN0aW9uLmphdmE=) | `83.82% <0%> (-7.36%)` | `0% <0%> (ø)` | | | [...impl/dictionary/FloatOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4222/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPZmZIZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==) | `83.63% <0%> (-7.28%)` | `0% <0%> (ø)` | | | [...mpl/dictionary/DoubleOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4222/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT2ZmSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=) | `65.45% <0%> (-7.28%)` | `0% <0%> (ø)` | | | [.../apache/pinot/common/config/RealtimeTagConfig.java](https://codecov.io/gh/apache/incubator-pinot/pull/4222/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL1JlYWx0aW1lVGFnQ29uZmlnLmphdmE=) | `93.33% <0%> (-6.67%)` | `0% <0%> (ø)` | | | ... and [37 more](https://codecov.io/gh/apache/incubator-pinot/pull/4222/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4222?src=pr=continue). > **Legend** - [Click here to learn
[GitHub] [incubator-pinot] akshayrai commented on a change in pull request #4221: [TE] Code cleanup - Pull out all validation checks from translator into validator modules
akshayrai commented on a change in pull request #4221: [TE] Code cleanup - Pull
out all validation checks from translator into validator modules
URL: https://github.com/apache/incubator-pinot/pull/4221#discussion_r285804479
##
File path:
thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/validators/ConfigValidator.java
##
@@ -29,17 +29,24 @@
*/
interface ConfigValidator {
/**
- * Validate the configuration. Thrown a validation exception if validation
failed.
+ * Validate the configuration
* @param config the config
- * @throws ValidationException the validation exception with error message
+ * @throws IllegalArgumentException exception with error message
*/
- void validateConfig(T config) throws ValidationException;
+ void validateConfig(T config) throws IllegalArgumentException;
/**
- * Validate the configuration. Thrown a validation exception if validation
failed.
+ * Validate the yaml configuration
+ * @param config the config
+ * @throws IllegalArgumentException exception with error message
+ */
+ void validateConfig(Map config) throws
IllegalArgumentException;
Review comment:
Updated!
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...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] akshayrai commented on a change in pull request #4221: [TE] Code cleanup - Pull out all validation checks from translator into validator modules
akshayrai commented on a change in pull request #4221: [TE] Code cleanup - Pull
out all validation checks from translator into validator modules
URL: https://github.com/apache/incubator-pinot/pull/4221#discussion_r285804473
##
File path:
thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/yaml/YamlDetectionConfigTranslator.java
##
@@ -77,7 +81,7 @@ public YamlDetectionConfigTranslator
withExistingDetectionConfig(DetectionConfig
* Fill in common fields of detection config. Properties of the pipeline is
filled by the subclass.
*/
public DetectionConfigDTO generateDetectionConfig() {
-validateYAML(yamlConfig);
+detectionValidator.validateConfig(yamlConfig);
Review comment:
Yes. I have already made these changes in the translator cleanup PR which I
will raise soon.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] akshayrai commented on a change in pull request #4221: [TE] Code cleanup - Pull out all validation checks from translator into validator modules
akshayrai commented on a change in pull request #4221: [TE] Code cleanup - Pull
out all validation checks from translator into validator modules
URL: https://github.com/apache/incubator-pinot/pull/4221#discussion_r285804483
##
File path:
thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/yaml/YamlResource.java
##
@@ -160,7 +156,12 @@ private DetectionConfigDTO
buildDetectionConfigFromYaml(long tuningStartTime, lo
tuningStartTime = tuningEndTime - TimeUnit.DAYS.toMillis(28);
}
-YamlDetectionConfigTranslator translator =
this.translatorLoader.from(yamlConfig, this.provider);
+YamlDetectionConfigTranslator translator;
+try {
+ translator = this.translatorLoader.from(yamlConfig, this.provider);
+} catch (Exception e) {
+ throw new IllegalArgumentException("Unable to instantiate the detection
pipeline. Please verify the pipelineType");
Review comment:
Done!
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...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang merged pull request #4198: Enhance the support for BYTES data type
Jackie-Jiang merged pull request #4198: Enhance the support for BYTES data type URL: https://github.com/apache/incubator-pinot/pull/4198 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch master updated: Enhance the support for BYTES data type (#4198)
This is an automated email from the ASF dual-hosted git repository.
jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
The following commit(s) were added to refs/heads/master by this push:
new bbf91c7 Enhance the support for BYTES data type (#4198)
bbf91c7 is described below
commit bbf91c7210abf6399006b8770632e2c9ec7e159d
Author: Xiaotian (Jackie) Jiang <1751+jackie-ji...@users.noreply.github.com>
AuthorDate: Mon May 20 16:29:14 2019 -0700
Enhance the support for BYTES data type (#4198)
1. Add BytesUtils to handle encode/decode for Hex string
2. Add FieldSpec.DataType.convert() to convert string value to data type
3. Support Hex string for index(), indexOf(), inRange() in all BYTES
dictionaries
4. Add MutableDictionary.compare(dictId1, dictId2) to help the sort process
and support sorting byte[]
5. Support returning Hex string in group-by for byte[] column
6. In MutableSegmentImpl, uniform the sort for all types, reduce the memory
usage especially for STRING and BYTES columns
---
.../org/apache/pinot/common/data/FieldSpec.java| 54 +++
.../org/apache/pinot/common/utils/BytesUtils.java | 56 +++
.../pinot/common/utils/primitive/ByteArray.java| 14 +-
.../org/apache/pinot/common/data/SchemaTest.java | 8 +-
.../indexsegment/mutable/MutableSegmentImpl.java | 174 -
.../groupby/DictionaryBasedGroupKeyGenerator.java | 6 +-
.../core/query/pruner/AbstractSegmentPruner.java | 25 ++-
...SelectionSingleValueColumnWithDictIterator.java | 4 +-
.../iterator/StringSelectionColumnIterator.java| 5 +-
.../converter/stats/RealtimeColumnStatistics.java | 51 +++---
.../dictionary/BaseOffHeapMutableDictionary.java | 30
.../dictionary/BaseOnHeapMutableDictionary.java| 1 -
.../dictionary/BytesOffHeapMutableDictionary.java | 48 ++
.../dictionary/BytesOnHeapMutableDictionary.java | 48 ++
.../dictionary/DoubleOffHeapMutableDictionary.java | 47 +++---
.../dictionary/DoubleOnHeapMutableDictionary.java | 5 +
.../dictionary/FloatOffHeapMutableDictionary.java | 48 +++---
.../dictionary/FloatOnHeapMutableDictionary.java | 5 +
.../dictionary/IntOffHeapMutableDictionary.java| 48 +++---
.../dictionary/IntOnHeapMutableDictionary.java | 5 +
.../dictionary/LongOffHeapMutableDictionary.java | 48 +++---
.../dictionary/LongOnHeapMutableDictionary.java| 5 +
.../impl/dictionary/MutableDictionary.java | 5 +
.../dictionary/StringOffHeapMutableDictionary.java | 15 +-
.../dictionary/StringOnHeapMutableDictionary.java | 19 +--
.../core/segment/index/readers/BaseDictionary.java | 1 -
.../segment/index/readers/BytesDictionary.java | 10 +-
.../pinot/core/util/FixedIntArrayOffHeapIdMap.java | 8 +-
.../impl/dictionary/MutableDictionaryTest.java | 3 +-
.../readers/ImmutableDictionaryReaderTest.java | 1 +
.../segments/v1/creator/DictionariesTest.java | 39 -
31 files changed, 387 insertions(+), 449 deletions(-)
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/data/FieldSpec.java
b/pinot-common/src/main/java/org/apache/pinot/common/data/FieldSpec.java
index 00192a6..beac069 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/data/FieldSpec.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/data/FieldSpec.java
@@ -22,11 +22,9 @@ import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import javax.annotation.Nullable;
import org.apache.avro.Schema.Type;
-import org.apache.commons.codec.DecoderException;
-import org.apache.commons.codec.binary.Hex;
-import org.apache.pinot.common.Utils;
import org.apache.pinot.common.config.ConfigKey;
import org.apache.pinot.common.config.ConfigNodeLifecycleAware;
+import org.apache.pinot.common.utils.BytesUtils;
import org.apache.pinot.common.utils.EqualityUtils;
import org.apache.pinot.common.utils.JsonUtils;
@@ -168,7 +166,7 @@ public abstract class FieldSpec implements
Comparable, ConfigNodeLife
*/
protected static String getStringValue(Object value) {
if (value instanceof byte[]) {
- return Hex.encodeHexString((byte[]) value);
+ return BytesUtils.toHexString((byte[]) value);
} else {
return value.toString();
}
@@ -187,26 +185,7 @@ public abstract class FieldSpec implements
Comparable, ConfigNodeLife
private static Object getDefaultNullValue(FieldType fieldType, DataType
dataType,
@Nullable String stringDefaultNullValue) {
if (stringDefaultNullValue != null) {
- switch (dataType) {
-case INT:
- return Integer.valueOf(stringDefaultNullValue);
-case LONG:
- return Long.valueOf(stringDefaultNullValue);
-case FLOAT:
- return Float.valueOf(stringDefaultNullValue);
-case DOUBLE:
- return
[incubator-pinot] branch bytes-enhancement deleted (was 791f804)
This is an automated email from the ASF dual-hosted git repository. jackie pushed a change to branch bytes-enhancement in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. was 791f804 Enhance the support for BYTES data type The revisions that were on this branch are still contained in other references; therefore, this change does not discard any commits from the repository. - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] kishoreg commented on issue #4218: Add RealtimeConsumptionCatchupServiceCallback
kishoreg commented on issue #4218: Add RealtimeConsumptionCatchupServiceCallback URL: https://github.com/apache/incubator-pinot/pull/4218#issuecomment-494185128 Agree with providing a simple wait for now but let's keep the default to 0 and allow users to override if needed. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on issue #4208: Increase the limit of groups in combine operator (inter-segments)
Jackie-Jiang commented on issue #4208: Increase the limit of groups in combine operator (inter-segments) URL: https://github.com/apache/incubator-pinot/pull/4208#issuecomment-494184960 @kishoreg Can you have another look on the 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang closed pull request #2829: Add query option 'attachValueInForColumns' to automatically attach VALUE_IN UDF to MV group-by columns
Jackie-Jiang closed pull request #2829: Add query option 'attachValueInForColumns' to automatically attach VALUE_IN UDF to MV group-by columns URL: https://github.com/apache/incubator-pinot/pull/2829 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #4222: Add startup/shutdown checks for HelixServerStarter
Jackie-Jiang commented on a change in pull request #4222: Add startup/shutdown
checks for HelixServerStarter
URL: https://github.com/apache/incubator-pinot/pull/4222#discussion_r285802018
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/CommonConstants.java
##
@@ -171,13 +171,8 @@ public ServerType getServerType() {
public static final String CONFIG_OF_REQUEST_HANDLER_FACTORY_CLASS =
"pinot.server.requestHandlerFactory.class";
public static final String CONFIG_OF_NETTY_PORT =
"pinot.server.netty.port";
public static final String CONFIG_OF_ADMIN_API_PORT =
"pinot.server.adminapi.port";
-public static final String CONFIG_OF_STARTER_ENABLE_SEGMENTS_LOADING_CHECK
=
Review comment:
Discussed offline, log warnings for usage of deprecated config keys
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...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch server_restart updated (fb005da -> f91eaa3)
This is an automated email from the ASF dual-hosted git repository. jackie pushed a change to branch server_restart in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. from fb005da Add startup/shutdown checks for HelixServerStarter add f91eaa3 Address comment No new revisions were added by this update. Summary of changes: .../org/apache/pinot/common/utils/CommonConstants.java| 15 +++ .../pinot/server/starter/helix/HelixServerStarter.java| 9 + 2 files changed, 24 insertions(+) - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io edited a comment on issue #4222: Add startup/shutdown checks for HelixServerStarter
codecov-io edited a comment on issue #4222: Add startup/shutdown checks for HelixServerStarter URL: https://github.com/apache/incubator-pinot/pull/4222#issuecomment-493717771 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4222?src=pr=h1) Report > Merging [#4222](https://codecov.io/gh/apache/incubator-pinot/pull/4222?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/d2087b365859c215a55900a47b3f9a9355808157?src=pr=desc) will **decrease** coverage by `0.01%`. > The diff coverage is `26.66%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/4222?src=pr=tree) ```diff @@ Coverage Diff @@ ## master#4222 +/- ## - Coverage 67.15% 67.13% -0.02% Complexity 20 20 Files 1036 1039 +3 Lines 5153651524 -12 Branches 7220 7227 +7 - Hits 3460734592 -15 - Misses1455714563 +6 + Partials 2372 2369 -3 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/4222?src=pr=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...org/apache/pinot/common/utils/CommonConstants.java](https://codecov.io/gh/apache/incubator-pinot/pull/4222/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `45.65% <ø> (+10.86%)` | `0 <0> (ø)` | :arrow_down: | | [...pinot/server/starter/helix/HelixServerStarter.java](https://codecov.io/gh/apache/incubator-pinot/pull/4222/diff?src=pr=tree#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9IZWxpeFNlcnZlclN0YXJ0ZXIuamF2YQ==) | `46.22% <26.66%> (-8.76%)` | `0 <0> (ø)` | | | [...r/validation/RealtimeSegmentValidationManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4222/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci92YWxpZGF0aW9uL1JlYWx0aW1lU2VnbWVudFZhbGlkYXRpb25NYW5hZ2VyLmphdmE=) | `43.33% <0%> (-15%)` | `0% <0%> (ø)` | | | [...e/operator/dociditerators/SortedDocIdIterator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4222/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZGl0ZXJhdG9ycy9Tb3J0ZWREb2NJZEl0ZXJhdG9yLmphdmE=) | `47.22% <0%> (-13.89%)` | `0% <0%> (ø)` | | | [...impl/dictionary/DoubleOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4222/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT25IZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==) | `68.88% <0%> (-13.34%)` | `0% <0%> (ø)` | | | [...impl/dictionary/FloatOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4222/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPZmZIZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==) | `83.63% <0%> (-7.28%)` | `0% <0%> (ø)` | | | [...e/impl/dictionary/LongOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4222/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09uSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=) | `88.88% <0%> (-6.67%)` | `0% <0%> (ø)` | | | [...nsport/netty/PooledNettyClientResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4222/diff?src=pr=tree#diff-cGlub3QtdHJhbnNwb3J0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC90cmFuc3BvcnQvbmV0dHkvUG9vbGVkTmV0dHlDbGllbnRSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `85.41% <0%> (-4.17%)` | `0% <0%> (ø)` | | | [...inot/core/operator/filter/FilterOperatorUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/4222/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvRmlsdGVyT3BlcmF0b3JVdGlscy5qYXZh) | `84.37% <0%> (-3.13%)` | `0% <0%> (ø)` | | | [...ot/core/query/pruner/ColumnValueSegmentPruner.java](https://codecov.io/gh/apache/incubator-pinot/pull/4222/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9wcnVuZXIvQ29sdW1uVmFsdWVTZWdtZW50UHJ1bmVyLmphdmE=) | `87.32% <0%> (-2.82%)` | `0% <0%> (ø)` | | | ... and [25 more](https://codecov.io/gh/apache/incubator-pinot/pull/4222/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4222?src=pr=continue). > **Legend**
[GitHub] [incubator-pinot] Jackie-Jiang commented on issue #4225: Make Pinot schema evolution easier
Jackie-Jiang commented on issue #4225: Make Pinot schema evolution easier URL: https://github.com/apache/incubator-pinot/issues/4225#issuecomment-494165508 @icefury71 The reload should be quite fast without impacting other tables. Please give it a try. I would not recommend generating the columns on the fly as all the columns in the schema are supposed to exist physically. 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang closed issue #4029: Failure to retrieve data after adding column to table schema.
Jackie-Jiang closed issue #4029: Failure to retrieve data after adding column to table schema. URL: https://github.com/apache/incubator-pinot/issues/4029 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on issue #74: About refreshing table after schema changed
Jackie-Jiang commented on issue #74: About refreshing table after schema changed
URL: https://github.com/apache/incubator-pinot/issues/74#issuecomment-494164396
You may use the reload API:
**{controllerHost:port}/tables/{tableName}/segments/{segmentName}/reload** for
single segment reload or
**{controllerHost:port}/tables/{tableName}/segments/reload** for whole table
reload.
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...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang closed issue #74: About refreshing table after schema changed
Jackie-Jiang closed issue #74: About refreshing table after schema changed URL: https://github.com/apache/incubator-pinot/issues/74 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] icefury71 commented on issue #4225: Make Pinot schema evolution easier
icefury71 commented on issue #4225: Make Pinot schema evolution easier URL: https://github.com/apache/incubator-pinot/issues/4225#issuecomment-494164063 @Jackie-Jiang We haven't benchmarked that yet - will do that. The initial suspicion was reloading segments will also take a non-trivial amount of time. But yes, will update this thread with some numbers. 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch distribute_tasks updated (a4aca9e -> e26b461)
This is an automated email from the ASF dual-hosted git repository. xhsun pushed a change to branch distribute_tasks in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. from a4aca9e [TE] randomization when creating the task add e26b461 [TE] Distribute job load to add random delays No new revisions were added by this update. Summary of changes: .../org/apache/pinot/thirdeye/detection/DetectionPipelineJob.java| 5 +++-- .../org/apache/pinot/thirdeye/detection/alert/DetectionAlertJob.java | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #4218: Add RealtimeConsumptionCatchupServiceCallback
Jackie-Jiang commented on a change in pull request #4218: Add
RealtimeConsumptionCatchupServiceCallback
URL: https://github.com/apache/incubator-pinot/pull/4218#discussion_r285779496
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/CommonConstants.java
##
@@ -174,6 +174,8 @@ public ServerType getServerType() {
public static final String CONFIG_OF_STARTER_ENABLE_SEGMENTS_LOADING_CHECK
=
"pinot.server.starter.enableSegmentsLoadingCheck";
public static final String CONFIG_OF_STARTER_TIMEOUT_IN_SECONDS =
"pinot.server.starter.timeoutInSeconds";
+public static final String
CONFIG_OF_STARTER_REALTIME_CONSUMPTION_CATCHUP_WAIT_TIME_SECONDS =
Review comment:
I would put this along with the
**CONFIG_OF_SERVER_MIN_RESOURCE_PERCENT_FOR_START** since both of them are for
the status check instead of the starter. Maybe rename it to
"pinot.server.startup.realtimeConsumptionCatchupTimeMs"?
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...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #4218: Add RealtimeConsumptionCatchupServiceCallback
Jackie-Jiang commented on a change in pull request #4218: Add
RealtimeConsumptionCatchupServiceCallback
URL: https://github.com/apache/incubator-pinot/pull/4218#discussion_r285780063
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/ServiceStatus.java
##
@@ -127,34 +129,129 @@ public String getStatusDescription() {
return statusDescription.toString();
}
}
+
+ public static abstract class BaseServiceStatusCallback implements
ServiceStatusCallback {
Review comment:
Yes, directly implement ServiceStatusCallback
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...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io commented on issue #4226: Fix link for KafkaConsumerFactory in PluggableStreams doc
codecov-io commented on issue #4226: Fix link for KafkaConsumerFactory in PluggableStreams doc URL: https://github.com/apache/incubator-pinot/pull/4226#issuecomment-494162310 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4226?src=pr=h1) Report > Merging [#4226](https://codecov.io/gh/apache/incubator-pinot/pull/4226?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/d2087b365859c215a55900a47b3f9a9355808157?src=pr=desc) will **decrease** coverage by `0.06%`. > The diff coverage is `n/a`. [](https://codecov.io/gh/apache/incubator-pinot/pull/4226?src=pr=tree) ```diff @@ Coverage Diff @@ ## master#4226 +/- ## - Coverage 67.15% 67.08% -0.07% Complexity 20 20 Files 1036 1036 Lines 5153651536 Branches 7220 7220 - Hits 3460734571 -36 - Misses1455714591 +34 - Partials 2372 2374 +2 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/4226?src=pr=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...r/validation/RealtimeSegmentValidationManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4226/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci92YWxpZGF0aW9uL1JlYWx0aW1lU2VnbWVudFZhbGlkYXRpb25NYW5hZ2VyLmphdmE=) | `43.33% <0%> (-15%)` | `0% <0%> (ø)` | | | [...esthandler/ConnectionPoolBrokerRequestHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/4226/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcmVxdWVzdGhhbmRsZXIvQ29ubmVjdGlvblBvb2xCcm9rZXJSZXF1ZXN0SGFuZGxlci5qYXZh) | `77.63% <0%> (-10.56%)` | `0% <0%> (ø)` | | | [...elix/core/relocation/RealtimeSegmentRelocator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4226/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlbG9jYXRpb24vUmVhbHRpbWVTZWdtZW50UmVsb2NhdG9yLmphdmE=) | `73.62% <0%> (-8.8%)` | `0% <0%> (ø)` | | | [.../apache/pinot/common/config/RealtimeTagConfig.java](https://codecov.io/gh/apache/incubator-pinot/pull/4226/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL1JlYWx0aW1lVGFnQ29uZmlnLmphdmE=) | `93.33% <0%> (-6.67%)` | `0% <0%> (ø)` | | | [...impl/dictionary/DoubleOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4226/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT25IZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==) | `75.55% <0%> (-6.67%)` | `0% <0%> (ø)` | | | [...impl/dictionary/FloatOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4226/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPZmZIZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==) | `87.27% <0%> (-3.64%)` | `0% <0%> (ø)` | | | [...der/HighLevelConsumerBasedRoutingTableBuilder.java](https://codecov.io/gh/apache/incubator-pinot/pull/4226/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9idWlsZGVyL0hpZ2hMZXZlbENvbnN1bWVyQmFzZWRSb3V0aW5nVGFibGVCdWlsZGVyLmphdmE=) | `90.9% <0%> (-3.04%)` | `0% <0%> (ø)` | | | [...ot/core/query/pruner/ColumnValueSegmentPruner.java](https://codecov.io/gh/apache/incubator-pinot/pull/4226/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9wcnVuZXIvQ29sdW1uVmFsdWVTZWdtZW50UHJ1bmVyLmphdmE=) | `87.32% <0%> (-2.82%)` | `0% <0%> (ø)` | | | [...broker/broker/helix/LiveInstanceChangeHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/4226/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0xpdmVJbnN0YW5jZUNoYW5nZUhhbmRsZXIuamF2YQ==) | `57.44% <0%> (-2.13%)` | `0% <0%> (ø)` | | | [...inot/core/operator/docidsets/AndBlockDocIdSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/4226/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZHNldHMvQW5kQmxvY2tEb2NJZFNldC5qYXZh) | `55.55% <0%> (-2.09%)` | `0% <0%> (ø)` | | | ... and [19 more](https://codecov.io/gh/apache/incubator-pinot/pull/4226/diff?src=pr=tree-more) | | -- [Continue to review full report at
[GitHub] [incubator-pinot] npawar merged pull request #4211: StreamPartitionAssignmentStrategy interface to plug in various partition assignment strategies (uniform, replica group based)
npawar merged pull request #4211: StreamPartitionAssignmentStrategy interface to plug in various partition assignment strategies (uniform, replica group based) URL: https://github.com/apache/incubator-pinot/pull/4211 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch master updated: StreamPartitionAssignmentStrategy interface to plug in various partition assignment strategies (uniform, replica group based) (#4211)
This is an automated email from the ASF dual-hosted git repository.
nehapawar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 7a39165 StreamPartitionAssignmentStrategy interface to plug in
various partition assignment strategies (uniform, replica group based) (#4211)
7a39165 is described below
commit 7a391659faf15010b9594886a281592886ca1d62
Author: Neha Pawar
AuthorDate: Mon May 20 14:37:11 2019 -0700
StreamPartitionAssignmentStrategy interface to plug in various partition
assignment strategies (uniform, replica group based) (#4211)
Phase 1 - Step 2 of https://github.com/apache/incubator-pinot/issues/4192
---
...roupBasedStreamPartitionAssignmentStrategy.java | 82
.../StreamPartitionAssignmentGenerator.java| 51 +---
.../StreamPartitionAssignmentStrategy.java | 39 ++
.../StreamPartitionAssignmentStrategyFactory.java | 46 +++
.../UniformStreamPartitionAssignmentStrategy.java | 66 ++
...icaGroupBasedStreamPartitionAssignmentTest.java | 140 +
.../StreamPartitionAssignmentGeneratorTest.java| 45 ++-
.../UniformStreamPartitionAssignmentTest.java | 94 ++
.../ReplicaGroupRebalanceSegmentStrategy.java | 11 +-
9 files changed, 518 insertions(+), 56 deletions(-)
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/partition/ReplicaGroupBasedStreamPartitionAssignmentStrategy.java
b/pinot-common/src/main/java/org/apache/pinot/common/partition/ReplicaGroupBasedStreamPartitionAssignmentStrategy.java
new file mode 100644
index 000..43fe69a
--- /dev/null
+++
b/pinot-common/src/main/java/org/apache/pinot/common/partition/ReplicaGroupBasedStreamPartitionAssignmentStrategy.java
@@ -0,0 +1,82 @@
+/**
+ * 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.pinot.common.partition;
+
+import com.google.common.annotations.VisibleForTesting;
+import java.util.ArrayList;
+import java.util.List;
+import javax.annotation.Nonnull;
+import org.apache.helix.HelixManager;
+import org.apache.pinot.common.exception.InvalidConfigException;
+
+
+/**
+ * Replica group based partition assignment strategy for realtime partitions
+ */
+public class ReplicaGroupBasedStreamPartitionAssignmentStrategy implements
StreamPartitionAssignmentStrategy {
+
+ /**
+ * Fetches the replica group partition assignment znode and assigns
partitions across the replica groups.
+ * A vertical slice will be picked form the replica group sets for a
partition, based on the formula:
+ * vertical slice = partition % numInstancesPerReplicaGroup
+ */
+ @Override
+ public PartitionAssignment getStreamPartitionAssignment(HelixManager
helixManager, @Nonnull String tableNameWithType,
+ @Nonnull List partitions, int numReplicas, List
instances) throws InvalidConfigException {
+
+ReplicaGroupPartitionAssignment replicaGroupPartitionAssignment =
+getReplicaGroupPartitionAssignment(helixManager, tableNameWithType);
+if (replicaGroupPartitionAssignment == null) {
+ throw new InvalidConfigException("ReplicaGroupPartitionAssignment is
null for table:" + tableNameWithType);
+}
+int numReplicaGroups =
replicaGroupPartitionAssignment.getNumReplicaGroups();
+// TODO: we might move to a model of not having the same
numInstancesPerReplicaGroup.
+// We would have to handle numInstancesInReplicaGroup on a replica group
by replica group basis, and uniformly assign withing each replica group
+int numInstancesPerReplicaGroup =
replicaGroupPartitionAssignment.getInstancesFromReplicaGroup(0, 0).size();
+
+PartitionAssignment streamPartitionAssignment = new
PartitionAssignment(tableNameWithType);
+
+List> verticalSlices = new
ArrayList<>(numInstancesPerReplicaGroup);
+for (int i = 0; i < numInstancesPerReplicaGroup; i++) {
+ verticalSlices.add(new ArrayList<>(numReplicaGroups));
+}
+
+for (int replicaGroupNumber = 0; replicaGroupNumber < numReplicaGroups;
replicaGroupNumber++) {
+ List instancesFromReplicaGroup =
+
[incubator-pinot] branch server_restart updated (630245a -> fb005da)
This is an automated email from the ASF dual-hosted git repository. jackie pushed a change to branch server_restart in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. discard 630245a Add startup/shutdown checks for HelixServerStarter add fb005da Add startup/shutdown checks for HelixServerStarter This update added new revisions after undoing existing revisions. That is to say, some revisions that were in the old version of the branch are not in the new version. This situation occurs when a user --force pushes a change and generates a repository containing something like this: * -- * -- B -- O -- O -- O (630245a) \ N -- N -- N refs/heads/server_restart (fb005da) You should already have received notification emails for all of the O revisions, and so the following emails describe only the N revisions from the common base, B. Any revisions marked "omit" are not gone; other references still refer to them. Any revisions marked "discard" are gone forever. No new revisions were added by this update. Summary of changes: .../server/starter/helix/HelixServerStarter.java | 28 ++ 1 file changed, 13 insertions(+), 15 deletions(-) - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] npawar commented on a change in pull request #4211: StreamPartitionAssignmentStrategy interface to plug in various partition assignment strategies (uniform, replica group ba
npawar commented on a change in pull request #4211:
StreamPartitionAssignmentStrategy interface to plug in various partition
assignment strategies (uniform, replica group based)
URL: https://github.com/apache/incubator-pinot/pull/4211#discussion_r285774010
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/partition/StreamPartitionAssignmentStrategy.java
##
@@ -0,0 +1,38 @@
+/**
+ * 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.pinot.common.partition;
+
+import java.util.List;
+import javax.annotation.Nonnull;
+import org.apache.helix.HelixManager;
+import org.apache.pinot.common.exception.InvalidConfigException;
+
+
+/**
+ * Creates a partition assignment for the partitions of a realtime table
+ */
+// TODO: Unify the interfaces for {@link
org.apache.pinot.controller.helix.core.sharding.SegmentAssignmentStrategy} and
{@link StreamPartitionAssignmentStrategy}
+public interface StreamPartitionAssignmentStrategy {
+
+ /**
+ * Given the list of partitions and replicas, come up with a {@link
PartitionAssignment}
+ */
+ PartitionAssignment getStreamPartitionAssignment(HelixManager helixManager,
@Nonnull String tableNameWithType,
Review comment:
Added it as a TODO for now, to be done when we decide to use it. Should be
fairly easy to bring it in. Did not add it right now, because it would be fair
bit of computation each time to compute from ideal state, and better to avoid
it until we will actually want to use it
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #4222: Add startup/shutdown checks for HelixServerStarter
Jackie-Jiang commented on a change in pull request #4222: Add startup/shutdown
checks for HelixServerStarter
URL: https://github.com/apache/incubator-pinot/pull/4222#discussion_r285773257
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/CommonConstants.java
##
@@ -171,13 +171,8 @@ public ServerType getServerType() {
public static final String CONFIG_OF_REQUEST_HANDLER_FACTORY_CLASS =
"pinot.server.requestHandlerFactory.class";
public static final String CONFIG_OF_NETTY_PORT =
"pinot.server.netty.port";
public static final String CONFIG_OF_ADMIN_API_PORT =
"pinot.server.adminapi.port";
-public static final String CONFIG_OF_STARTER_ENABLE_SEGMENTS_LOADING_CHECK
=
Review comment:
IMO we should not keep unused constants in the code base, it's better to put
them in the release log.
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...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io edited a comment on issue #4211: StreamPartitionAssignmentStrategy interface to plug in various partition assignment strategies (uniform, replica group based)
codecov-io edited a comment on issue #4211: StreamPartitionAssignmentStrategy interface to plug in various partition assignment strategies (uniform, replica group based) URL: https://github.com/apache/incubator-pinot/pull/4211#issuecomment-492875911 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4211?src=pr=h1) Report > Merging [#4211](https://codecov.io/gh/apache/incubator-pinot/pull/4211?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/40df151e79882a9debedc2971e71625659e4f24f?src=pr=desc) will **decrease** coverage by `0.11%`. > The diff coverage is `92.72%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/4211?src=pr=tree) ```diff @@ Coverage Diff @@ ## master#4211 +/- ## - Coverage 67.23% 67.12% -0.12% Complexity 20 20 Files 1035 1039 +4 Lines 5151551570 +55 Branches 7220 7226 +6 - Hits 3463834615 -23 - Misses1450214584 +82 + Partials 2375 2371 -4 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/4211?src=pr=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [.../partition/StreamPartitionAssignmentGenerator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4211/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcGFydGl0aW9uL1N0cmVhbVBhcnRpdGlvbkFzc2lnbm1lbnRHZW5lcmF0b3IuamF2YQ==) | `93.87% <100%> (-1.21%)` | `0 <0> (ø)` | | | [...tion/UniformStreamPartitionAssignmentStrategy.java](https://codecov.io/gh/apache/incubator-pinot/pull/4211/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcGFydGl0aW9uL1VuaWZvcm1TdHJlYW1QYXJ0aXRpb25Bc3NpZ25tZW50U3RyYXRlZ3kuamF2YQ==) | `100% <100%> (ø)` | `0 <0> (?)` | | | [...ebalance/ReplicaGroupRebalanceSegmentStrategy.java](https://codecov.io/gh/apache/incubator-pinot/pull/4211/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYmFsYW5jZS9SZXBsaWNhR3JvdXBSZWJhbGFuY2VTZWdtZW50U3RyYXRlZ3kuamF2YQ==) | `82.25% <100%> (-0.16%)` | `0 <0> (ø)` | | | [...caGroupBasedStreamPartitionAssignmentStrategy.java](https://codecov.io/gh/apache/incubator-pinot/pull/4211/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcGFydGl0aW9uL1JlcGxpY2FHcm91cEJhc2VkU3RyZWFtUGFydGl0aW9uQXNzaWdubWVudFN0cmF0ZWd5LmphdmE=) | `87.5% <87.5%> (ø)` | `0 <0> (?)` | | | [...tion/StreamPartitionAssignmentStrategyFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/4211/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcGFydGl0aW9uL1N0cmVhbVBhcnRpdGlvbkFzc2lnbm1lbnRTdHJhdGVneUZhY3RvcnkuamF2YQ==) | `88.88% <88.88%> (ø)` | `0 <0> (?)` | | | [...r/validation/RealtimeSegmentValidationManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4211/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci92YWxpZGF0aW9uL1JlYWx0aW1lU2VnbWVudFZhbGlkYXRpb25NYW5hZ2VyLmphdmE=) | `43.33% <0%> (-38.34%)` | `0% <0%> (ø)` | | | [...he/pinot/core/query/pruner/ValidSegmentPruner.java](https://codecov.io/gh/apache/incubator-pinot/pull/4211/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9wcnVuZXIvVmFsaWRTZWdtZW50UHJ1bmVyLmphdmE=) | `57.14% <0%> (-28.58%)` | `0% <0%> (ø)` | | | [...apache/pinot/common/metrics/ValidationMetrics.java](https://codecov.io/gh/apache/incubator-pinot/pull/4211/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9WYWxpZGF0aW9uTWV0cmljcy5qYXZh) | `20.28% <0%> (-23.19%)` | `0% <0%> (ø)` | | | [...e/operator/dociditerators/SortedDocIdIterator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4211/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZGl0ZXJhdG9ycy9Tb3J0ZWREb2NJZEl0ZXJhdG9yLmphdmE=) | `55.55% <0%> (-8.34%)` | `0% <0%> (ø)` | | | [...pache/pinot/core/util/SortedRangeIntersection.java](https://codecov.io/gh/apache/incubator-pinot/pull/4211/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL1NvcnRlZFJhbmdlSW50ZXJzZWN0aW9uLmphdmE=) | `83.82% <0%> (-7.36%)` | `0% <0%> (ø)` | | | ... and [31 more](https://codecov.io/gh/apache/incubator-pinot/pull/4211/diff?src=pr=tree-more) | | -- [Continue to review full report at
[GitHub] [incubator-pinot] jihaozh opened a new pull request #4227: [TE] Fix MySql data source to read the epoch time stamps
jihaozh opened a new pull request #4227: [TE] Fix MySql data source to read the epoch time stamps URL: https://github.com/apache/incubator-pinot/pull/4227 MySQL data source will throw an exception when reading the epoch timestamps dataset. This PR fixes the bug. 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #4133: add more types parsing for string coerceValueIntoField
Jackie-Jiang commented on a change in pull request #4133: add more types
parsing for string coerceValueIntoField
URL: https://github.com/apache/incubator-pinot/pull/4133#discussion_r285767997
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/config/Deserializer.java
##
@@ -349,17 +349,26 @@
} else if (value instanceof String) {
String stringValue = (String) value;
try {
-if (Integer.class.isAssignableFrom(fieldType) || int.class
-.isAssignableFrom(fieldType)) { // field instanceof int/Integer
+if (fieldType.isAssignableFrom(Integer.class) ||
fieldType.isAssignableFrom(int.class)){
destinationValue = Integer.parseInt(stringValue);
-} else if (Long.class.isAssignableFrom(fieldType)) { // field
instanceof long/Long
+} else if (fieldType.isAssignableFrom(Long.class) ||
fieldType.isAssignableFrom(long.class)){
destinationValue = Long.parseLong(stringValue);
-} else if (Boolean.class.isAssignableFrom(fieldType) ||
boolean.class.isAssignableFrom(fieldType)) {
+} else if (fieldType.isAssignableFrom(Boolean.class) ||
fieldType.isAssignableFrom(boolean.class)){
destinationValue = Boolean.parseBoolean(stringValue);
-} else if (Enum.class.isAssignableFrom(fieldType)) {
+} else if (Enum.class.isAssignableFrom(fieldType)){
destinationValue =
Enum.valueOf(fieldType.asSubclass(Enum.class), stringValue.toUpperCase());
-} else {
- // TODO Add float/double/short/char
+} else if (fieldType.isAssignableFrom(Float.class) ||
fieldType.isAssignableFrom(float.class)){
+ destinationValue = Float.parseFloat(stringValue);
+} else if (fieldType.isAssignableFrom(Double.class) ||
fieldType.isAssignableFrom(double.class)){
+ destinationValue = Double.parseDouble(stringValue);
+} else if (fieldType.isAssignableFrom(Short.class) ||
fieldType.isAssignableFrom(short.class)){
+ destinationValue = Short.parseShort(stringValue);
+} else if (fieldType.isAssignableFrom(Byte.class) ||
fieldType.isAssignableFrom(byte.class)){
+ destinationValue = stringValue.getBytes()[0];
Review comment:
Byte.parseByte(stringValue)?
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...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on issue #4225: Make Pinot schema evolution easier
mayankshriv commented on issue #4225: Make Pinot schema evolution easier URL: https://github.com/apache/incubator-pinot/issues/4225#issuecomment-494152235 Just to clarify, I don't mean the virtual column semantics as they are defined today, but leverage parts of that solution to dynamically add a column that provides default values without having to generate one entry per row of the same value. 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on issue #4225: Make Pinot schema evolution easier
mayankshriv commented on issue #4225: Make Pinot schema evolution easier URL: https://github.com/apache/incubator-pinot/issues/4225#issuecomment-494150493 We could also explore dynamically adding virtual columns to provide default values, to the consuming segments as well. Is there a fundamental reason why that is not possible/hard for consuming segments, that I couldn't gather from #151? @Jackie-Jiang What do you think about the virtual columns approach? Or if you have any other ideas? 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] npawar opened a new pull request #4226: Fix link for KafkaConsumerFactory in PluggableStreams doc
npawar opened a new pull request #4226: Fix link for KafkaConsumerFactory in PluggableStreams doc URL: https://github.com/apache/incubator-pinot/pull/4226 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] icefury71 opened a new issue #4225: Make Pinot schema evolution easier
icefury71 opened a new issue #4225: Make Pinot schema evolution easier URL: https://github.com/apache/incubator-pinot/issues/4225 This has been referenced in a few issues already: https://github.com/apache/incubator-pinot/issues/74 https://github.com/apache/incubator-pinot/issues/4029 But I'm creating a new issue to highlight the end-end problem. Here are the current steps to perform schema evolution in Pinot: 1) Create a new schema with default values for new columns being added 2) Update schema using Controller API 3) Rolling restart the Pinot servers to "reload" the segments to reflect the default value. In general, restarting a Pinot server is very expensive (can take anywhere between 5 to 30 mins depending on number of segments). If we need to evolve the schema frequently, this becomes a huge operational overhead. An alternate way to resolve this issue is to backfill the old segments but this is an expensive process as well. **A better approach:** a) Segments which have been committed / ONLINE We can try to lookup the new schema "on the fly" during query processing using some technique (for eg: @mayankshriv suggested using virtual columns for old segments populated with default value). That way we don't depend on any server restart. b) Segments which are currently open / CONSUMING We need to solve this issue: https://github.com/apache/incubator-pinot/issues/151 (how to reflect new schema in an open segment). 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] xiaohui-sun commented on a change in pull request #4221: [TE] Code cleanup - Pull out all validation checks from translator into validator modules
xiaohui-sun commented on a change in pull request #4221: [TE] Code cleanup -
Pull out all validation checks from translator into validator modules
URL: https://github.com/apache/incubator-pinot/pull/4221#discussion_r285754262
##
File path:
thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/yaml/YamlResource.java
##
@@ -160,7 +156,12 @@ private DetectionConfigDTO
buildDetectionConfigFromYaml(long tuningStartTime, lo
tuningStartTime = tuningEndTime - TimeUnit.DAYS.toMillis(28);
}
-YamlDetectionConfigTranslator translator =
this.translatorLoader.from(yamlConfig, this.provider);
+YamlDetectionConfigTranslator translator;
+try {
+ translator = this.translatorLoader.from(yamlConfig, this.provider);
+} catch (Exception e) {
+ throw new IllegalArgumentException("Unable to instantiate the detection
pipeline. Please verify the pipelineType");
Review comment:
Can you pass the original exception out?
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...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] npawar commented on issue #4218: Add RealtimeConsumptionCatchupServiceCallback
npawar commented on issue #4218: Add RealtimeConsumptionCatchupServiceCallback URL: https://github.com/apache/incubator-pinot/pull/4218#issuecomment-494139930 > @npawar I have a few questions. > > What is the impact on the latency and when does it kick in and for what kind of use cases? > How will the user know whats the right wait-time? We might still see the same issue if the server has not caught up within the wait time right? > > One of the concerns here is that we stop the entire server from serving any other segment. > > Looks like we need two things > > * limit the number of threads for consumption so that it does not hog all the cpu during bootstrap/server startup > * Have the logic in the state transition for OFFLINE -> CONSUMING to not ack Helix until it's caught up. This has the added benefit that query results are up to date across all replicas. > > I am ok with the temporary fix but was wondering what would be the right fix that requires minimal configuration from the user. The impact we've observed is usually for high ingestion rate realtime tables. The tables can usually take 2-3 minutes to completely catchup after server restart. During this time, we not only see high latencies/slow queries/timeouts because of cpu bottleneck, but also the results returned by that server will not be in sync with what other replicas return. We decided to have a simple approach of static wait time to begin with, until we can get more information about the consumption rate. Yes we will still see the issue if the server has not caught up in the configured time. We plan to include consumption rate related stats in server stats. Once we have those, we will remove the static wait. We will then wait until consumption rate stabilizes to the average rate or until idle. I can make the static wait to default 0 if having the default 5 is a problem? 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jamesyfshao opened a new issue #4224: migrate to dropwizard metrics from yammer
jamesyfshao opened a new issue #4224: migrate to dropwizard metrics from yammer URL: https://github.com/apache/incubator-pinot/issues/4224 Pinot currently is using yammer 2.2.0 for metrics reporting. This library development has been discontinued for multiple years now ([last updates](https://github.com/infusionsoft/yammer-metrics/commits/master) from 2016/2017) and the [version we are using is from 2012](https://github.com/infusionsoft/yammer-metrics/releases/tag/v2.2.0). Consider that yammer library publish company has switched to a very similar and mostly-compatible library ([dropwizard](https://metrics.dropwizard.io/4.0.0/)), we should work on switching to this new library for getting up-to-date bug fix and help other Pinot users to plug-in their own metrics reporting tools easier. 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] kishoreg commented on issue #4218: Add RealtimeConsumptionCatchupServiceCallback
kishoreg commented on issue #4218: Add RealtimeConsumptionCatchupServiceCallback URL: https://github.com/apache/incubator-pinot/pull/4218#issuecomment-494096953 @npawar I have a few questions. What is the impact on the latency and when does it kick in and for what kind of use cases? How will the user know whats the right wait-time? We might still see the same issue if the server has not caught up within the wait time right? One of the concerns here is that we stop the entire server from serving any other segment. Looks like we need two things - limit the number of threads for consumption so that it does not hog all the cpu during bootstrap/server startup - Have the logic in the state transition for OFFLINE -> CONSUMING to not ack Helix until it's caught up. This has the added benefit that query results are up to date across all replicas. I am ok with the temporary fix but was wondering what would be the right fix that requires minimal configuration from the user. 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jihaozh commented on a change in pull request #4221: [TE] Code cleanup - Pull out all validation checks from translator into validator modules
jihaozh commented on a change in pull request #4221: [TE] Code cleanup - Pull
out all validation checks from translator into validator modules
URL: https://github.com/apache/incubator-pinot/pull/4221#discussion_r285710466
##
File path:
thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/yaml/YamlDetectionConfigTranslator.java
##
@@ -77,7 +81,7 @@ public YamlDetectionConfigTranslator
withExistingDetectionConfig(DetectionConfig
* Fill in common fields of detection config. Properties of the pipeline is
filled by the subclass.
*/
public DetectionConfigDTO generateDetectionConfig() {
-validateYAML(yamlConfig);
+detectionValidator.validateConfig(yamlConfig);
Review comment:
How about calling detectionValidator in
CompositePipelineConfigTranslator.java's constructor. After this refactoring,
each YamlDetectionConfigTranslator implementation has a corresponding
DetectionConfigValidator to handle different YAML formats. Because
YamlDetectionConfigTranslator is loaded dynamically using reflection, it can
call the respective validator.
For example, if we plan to add EntityLevelPipelineConfigTranslator, it can
call EntityLevelDetectionValidator to validate the YAML.
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...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jihaozh commented on a change in pull request #4221: [TE] Code cleanup - Pull out all validation checks from translator into validator modules
jihaozh commented on a change in pull request #4221: [TE] Code cleanup - Pull
out all validation checks from translator into validator modules
URL: https://github.com/apache/incubator-pinot/pull/4221#discussion_r285700662
##
File path:
thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/validators/ConfigValidator.java
##
@@ -29,17 +29,24 @@
*/
interface ConfigValidator {
/**
- * Validate the configuration. Thrown a validation exception if validation
failed.
+ * Validate the configuration
* @param config the config
- * @throws ValidationException the validation exception with error message
+ * @throws IllegalArgumentException exception with error message
*/
- void validateConfig(T config) throws ValidationException;
+ void validateConfig(T config) throws IllegalArgumentException;
/**
- * Validate the configuration. Thrown a validation exception if validation
failed.
+ * Validate the yaml configuration
+ * @param config the config
+ * @throws IllegalArgumentException exception with error message
+ */
+ void validateConfig(Map config) throws
IllegalArgumentException;
Review comment:
Could we rename this method to validateYaml? It is generally a good practice
to refrain from overloading methods with multiple signatures that have the same
number of parameters. It's can be confusing and Java compiler sometimes
handles this in an unexpected way. see more here
http://www.informit.com/articles/article.aspx?p=31551=4
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...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on issue #4218: Add RealtimeConsumptionCatchupServiceCallback
mcvsubbu commented on issue #4218: Add RealtimeConsumptionCatchupServiceCallback URL: https://github.com/apache/incubator-pinot/pull/4218#issuecomment-494091417 lgtm, @Jackie-Jiang had some comments on class hierarchy. I am neutral on that. 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] npawar commented on a change in pull request #4218: Add RealtimeConsumptionCatchupServiceCallback
npawar commented on a change in pull request #4218: Add
RealtimeConsumptionCatchupServiceCallback
URL: https://github.com/apache/incubator-pinot/pull/4218#discussion_r285703765
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/ServiceStatus.java
##
@@ -127,34 +129,129 @@ public String getStatusDescription() {
return statusDescription.toString();
}
}
+
+ public static abstract class BaseServiceStatusCallback implements
ServiceStatusCallback {
+private final String _clusterName;
+final String _instanceName;
+private final HelixAdmin _helixAdmin;
+private final HelixDataAccessor _helixDataAccessor;
+
+String _statusDescription = STATUS_DESCRIPTION_INIT;
+
+BaseServiceStatusCallback(HelixManager helixManager, String clusterName,
String instanceName) {
+ _helixAdmin = helixManager.getClusterManagmentTool();
+ _helixDataAccessor = helixManager.getHelixDataAccessor();
+ _clusterName = clusterName;
+ _instanceName = instanceName;
+}
+
+@Override
+public synchronized String getStatusDescription() {
+ return _statusDescription;
+}
+
+protected IdealState getResourceIdealState(String resourceName) {
+ return _helixAdmin.getResourceIdealState(_clusterName, resourceName);
+}
+
+protected List getResourcesInCluster() {
+ return _helixAdmin.getResourcesInCluster(_clusterName);
+}
+
+protected ExternalView getResourceExternalView(String resourceName) {
+ return _helixAdmin.getResourceExternalView(_clusterName, resourceName);
+}
+
+protected CurrentState getCurrentState(String resourceName) {
+ PropertyKey.Builder keyBuilder = _helixDataAccessor.keyBuilder();
+ LiveInstance liveInstance =
_helixDataAccessor.getProperty(keyBuilder.liveInstance(_instanceName));
+ String sessionId = liveInstance.getSessionId();
+ return
_helixDataAccessor.getProperty(keyBuilder.currentState(_instanceName,
sessionId, resourceName));
+}
+ }
+
+ /**
+ * Service status callback that checks whether realtime consumption has
caught up
+ * TODO: In this initial version, we are simply adding a configurable static
wait time
+ * This can be made smarter:
+ * 1) Keep track of average consumption rate for table in server stats
+ * 2) Monitor consumption rate during startup, report GOOD when it
stabilizes to average rate
+ * 3) Monitor consumption rate during startup, report GOOD if it is idle
+ */
+ public static class RealtimeConsumptionCatchupServiceStatusCallback extends
BaseServiceStatusCallback {
+
+private final Map> _tableToConsumingSegmentsMap;
+private long _endWaitTime = 0;
+
+/**
+ * Realtime consumption catchup service which adds a static wait time for
+ */
+public RealtimeConsumptionCatchupServiceStatusCallback(HelixManager
helixManager, String clusterName, String instanceName,
+long realtimeConsumptionCatchupWaitTimeSeconds) {
+ super(helixManager, clusterName, instanceName);
+
+ _tableToConsumingSegmentsMap = new HashMap<>();
+ for (String resourceName : getResourcesInCluster()) {
+if (TableNameBuilder.isRealtimeTableResource(resourceName)) {
+
+ IdealState idealState = getResourceIdealState(resourceName);
+ if (idealState.isEnabled()) {
+List consumingSegments = new ArrayList<>();
+for (String segmentName : idealState.getPartitionSet()) {
+ Map instanceStateMap =
idealState.getInstanceStateMap(segmentName);
+ if
(CommonConstants.Helix.StateModel.RealtimeSegmentOnlineOfflineStateModel.CONSUMING.equals(
+ instanceStateMap.get(instanceName))) {
+consumingSegments.add(segmentName);
+ }
+}
+if (!consumingSegments.isEmpty()) {
+ _tableToConsumingSegmentsMap.put(resourceName,
consumingSegments);
+}
+ }
+}
+ }
+
+ if (_tableToConsumingSegmentsMap.isEmpty()) {
+LOGGER.info(
+"No consuming segments to monitor on instance. Setting realtime
consumption catchup wait time to 0ms");
+ } else {
+long startTime = System.currentTimeMillis();
+_endWaitTime = startTime +
TimeUnit.SECONDS.toMillis(realtimeConsumptionCatchupWaitTimeSeconds);
+LOGGER.info("Monitoring realtime consumption catchup for tables:{}.
Will wait for time:{} ms",
+_tableToConsumingSegmentsMap.keySet(),
realtimeConsumptionCatchupWaitTimeSeconds);
+ }
+}
+
+@Override
+public Status getServiceStatus() {
+ long now = System.currentTimeMillis();
+ if (now < _endWaitTime) {
+_statusDescription = String.format("Waiting for CONSUMING segments to
catchup, timeRemaining=%dms", _endWaitTime - System.currentTimeMillis());
+return Status.STARTING;
+ }
+ _statusDescription = "Wait time for CONSUMING segments catchup
[GitHub] [incubator-pinot] npawar commented on a change in pull request #4218: Add RealtimeConsumptionCatchupServiceCallback
npawar commented on a change in pull request #4218: Add
RealtimeConsumptionCatchupServiceCallback
URL: https://github.com/apache/incubator-pinot/pull/4218#discussion_r285702265
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/ServiceStatus.java
##
@@ -127,34 +129,129 @@ public String getStatusDescription() {
return statusDescription.toString();
}
}
+
+ public static abstract class BaseServiceStatusCallback implements
ServiceStatusCallback {
+private final String _clusterName;
+final String _instanceName;
+private final HelixAdmin _helixAdmin;
+private final HelixDataAccessor _helixDataAccessor;
+
+String _statusDescription = STATUS_DESCRIPTION_INIT;
+
+BaseServiceStatusCallback(HelixManager helixManager, String clusterName,
String instanceName) {
+ _helixAdmin = helixManager.getClusterManagmentTool();
+ _helixDataAccessor = helixManager.getHelixDataAccessor();
+ _clusterName = clusterName;
+ _instanceName = instanceName;
+}
+
+@Override
+public synchronized String getStatusDescription() {
+ return _statusDescription;
+}
+
+protected IdealState getResourceIdealState(String resourceName) {
+ return _helixAdmin.getResourceIdealState(_clusterName, resourceName);
+}
+
+protected List getResourcesInCluster() {
+ return _helixAdmin.getResourcesInCluster(_clusterName);
+}
+
+protected ExternalView getResourceExternalView(String resourceName) {
+ return _helixAdmin.getResourceExternalView(_clusterName, resourceName);
+}
+
+protected CurrentState getCurrentState(String resourceName) {
+ PropertyKey.Builder keyBuilder = _helixDataAccessor.keyBuilder();
+ LiveInstance liveInstance =
_helixDataAccessor.getProperty(keyBuilder.liveInstance(_instanceName));
+ String sessionId = liveInstance.getSessionId();
+ return
_helixDataAccessor.getProperty(keyBuilder.currentState(_instanceName,
sessionId, resourceName));
+}
+ }
+
+ /**
+ * Service status callback that checks whether realtime consumption has
caught up
+ * TODO: In this initial version, we are simply adding a configurable static
wait time
+ * This can be made smarter:
+ * 1) Keep track of average consumption rate for table in server stats
+ * 2) Monitor consumption rate during startup, report GOOD when it
stabilizes to average rate
+ * 3) Monitor consumption rate during startup, report GOOD if it is idle
+ */
+ public static class RealtimeConsumptionCatchupServiceStatusCallback extends
BaseServiceStatusCallback {
+
+private final Map> _tableToConsumingSegmentsMap;
+private long _endWaitTime = 0;
+
+/**
+ * Realtime consumption catchup service which adds a static wait time for
+ */
+public RealtimeConsumptionCatchupServiceStatusCallback(HelixManager
helixManager, String clusterName, String instanceName,
+long realtimeConsumptionCatchupWaitTimeSeconds) {
+ super(helixManager, clusterName, instanceName);
+
+ _tableToConsumingSegmentsMap = new HashMap<>();
+ for (String resourceName : getResourcesInCluster()) {
+if (TableNameBuilder.isRealtimeTableResource(resourceName)) {
+
+ IdealState idealState = getResourceIdealState(resourceName);
+ if (idealState.isEnabled()) {
+List consumingSegments = new ArrayList<>();
+for (String segmentName : idealState.getPartitionSet()) {
+ Map instanceStateMap =
idealState.getInstanceStateMap(segmentName);
+ if
(CommonConstants.Helix.StateModel.RealtimeSegmentOnlineOfflineStateModel.CONSUMING.equals(
+ instanceStateMap.get(instanceName))) {
+consumingSegments.add(segmentName);
+ }
+}
+if (!consumingSegments.isEmpty()) {
+ _tableToConsumingSegmentsMap.put(resourceName,
consumingSegments);
+}
+ }
+}
+ }
+
+ if (_tableToConsumingSegmentsMap.isEmpty()) {
+LOGGER.info(
+"No consuming segments to monitor on instance. Setting realtime
consumption catchup wait time to 0ms");
+ } else {
+long startTime = System.currentTimeMillis();
+_endWaitTime = startTime +
TimeUnit.SECONDS.toMillis(realtimeConsumptionCatchupWaitTimeSeconds);
+LOGGER.info("Monitoring realtime consumption catchup for tables:{}.
Will wait for time:{} ms",
+_tableToConsumingSegmentsMap.keySet(),
realtimeConsumptionCatchupWaitTimeSeconds);
+ }
+}
+
+@Override
+public Status getServiceStatus() {
+ long now = System.currentTimeMillis();
Review comment:
Worked on all suggestions
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
[incubator-pinot] branch create-lead-controller-resource updated (22dad4f -> 46f2275)
This is an automated email from the ASF dual-hosted git repository. jlli pushed a change to branch create-lead-controller-resource in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. omit 22dad4f Create leadControllerResource in Helix cluster add a9a075c [TE] Expose last exception when early terminate (#4178) add 1df6fb7 [TE] anomaly detector interface change and implementation for rule-based detection (#4176) add b4c9bdc Pin down Pinot version to 0.1.0; enables you to build thirdeye without building pinot (#4179) add e5029b6 Minor improvements as encountered while studying replica groups (#4180) add 0680437 Refactor HelixExternalViewBasedTimeBoundaryService to support all time units (#4156) add ac19af7 Clean up unused variable in ControllerLeadershipManager (#4181) add e1e54cf Generate inverted index in purge task if it exists (#4182) add 04042c7 [TE] Use strict strategy in model mapper for spec class (#4185) add 5920969 Added service status logs to indicate more znode information (#4184) add db84cbb [TE] Fix current value in HoltWinters baseline (#4186) add 84a34a2 Migration to log4j2 (#4139) add 0706138 [TE] fix mock data source (#4189) add 29b918f Randomly choose segments that need to be moved (#4191) add d499ba1 [TE] pass predicted time series thought out the detection pipeline (#4190) add 578f42e In DataTypeTransformer, support filling in default null value for empty array input (#4196) add 18d628b Track "freshness" timestamp across consuming segments (#3979) add 1ac071f [TE] Update anomaly merge logic (#4201) add 74ddc3a [TE] Threshold filter on current value of an anomaly (#4203) add e279dc9 [TE] Evaluation metrics calculation & store (#4202) add ca0b5b8 Commment out deleteTaskQueue in Test testStopAndResumeTaskQueue (#4207) add 40df151 Creation/deletion flow for replica groups sets in realtime (#4193) add 3cca15f Add integration test for BrokerResourceValidationManager (#4096) add 3e6733f Add SimpleAvroMessageDecoder which allows passing in Avro schema directly (#4197) add f50e87f [TE] Model evaluator interface & Implementation (#4209) add aed4b56 Minor fix for star-tree creation logs (#4215) add c8d68ae [TE] Grouper Interface - Group/Summarize Metric level anomalies (#4212) add f9df139 [TE] frontend - harleyjj/create-alert - break yaml editor into two components (#4206) add 45c59f6 [TE] frontend - harleyjj/screenshot - update screenshot route to match new Anomalies route (#4194) add d2087b3 [TE] pinot - harleyjj/screenshot - update configs for new screenshot (#4195) add 46f2275 Create leadControllerResource in Helix cluster This update added new revisions after undoing existing revisions. That is to say, some revisions that were in the old version of the branch are not in the new version. This situation occurs when a user --force pushes a change and generates a repository containing something like this: * -- * -- B -- O -- O -- O (22dad4f) \ N -- N -- N refs/heads/create-lead-controller-resource (46f2275) You should already have received notification emails for all of the O revisions, and so the following emails describe only the N revisions from the common base, B. Any revisions marked "omit" are not gone; other references still refer to them. Any revisions marked "discard" are gone forever. No new revisions were added by this update. Summary of changes: pinot-api/pom.xml | 4 - pinot-api/src/test/resources/log4j.properties | 6 - pinot-api/src/test/resources/log4j2.xml| 35 +++ .../requesthandler/BaseBrokerRequestHandler.java | 5 +- .../HelixExternalViewBasedTimeBoundaryService.java | 118 + .../broker/routing/RoutingTableBuilderFactory.java | 2 +- .../pinot/broker/routing/TimeBoundaryService.java | 15 +- .../builder/GeneratorBasedRoutingTableBuilder.java | 22 +- .../PartitionAwareOfflineRoutingTableBuilder.java | 9 +- .../broker/broker/HelixBrokerStarterTest.java | 7 +- ...ixExternalViewBasedTimeBoundaryServiceTest.java | 166 .../broker/routing/TimeBoundaryServiceTest.java| 130 - pinot-broker/src/test/resources/log4j.properties | 6 - pinot-broker/src/test/resources/log4j2.xml | 35 +++ pinot-common/pom.xml | 12 +- .../org/apache/pinot/common/data/FieldSpec.java| 26 +- .../apache/pinot/common/metadata/RowMetadata.java | 30 +-- .../pinot/common/metadata/ZKMetadataProvider.java | 13 +- .../common/partition/PartitionAssignment.java | 4 +- .../partition/ReplicaGroupPartitionAssignment.java | 4 +- .../ReplicaGroupPartitionAssignmentGenerator.java | 8 +- .../response/broker/BrokerResponseNative.java | 27 +-
[GitHub] [incubator-pinot] jihaozh merged pull request #4195: [TE] pinot - harleyjj/screenshot - update configs for new screenshot
jihaozh merged pull request #4195: [TE] pinot - harleyjj/screenshot - update configs for new screenshot URL: https://github.com/apache/incubator-pinot/pull/4195 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch master updated: [TE] pinot - harleyjj/screenshot - update configs for new screenshot (#4195)
This is an automated email from the ASF dual-hosted git repository.
jihao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
The following commit(s) were added to refs/heads/master by this push:
new d2087b3 [TE] pinot - harleyjj/screenshot - update configs for new
screenshot (#4195)
d2087b3 is described below
commit d2087b365859c215a55900a47b3f9a9355808157
Author: Harley Jackson
AuthorDate: Mon May 20 10:19:07 2019 -0700
[TE] pinot - harleyjj/screenshot - update configs for new screenshot (#4195)
---
.../thirdeye-pinot/src/main/resources/scripts/getGraphPnj.js | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/thirdeye/thirdeye-pinot/src/main/resources/scripts/getGraphPnj.js
b/thirdeye/thirdeye-pinot/src/main/resources/scripts/getGraphPnj.js
index edd8f52..644ee0e 100644
--- a/thirdeye/thirdeye-pinot/src/main/resources/scripts/getGraphPnj.js
+++ b/thirdeye/thirdeye-pinot/src/main/resources/scripts/getGraphPnj.js
@@ -11,6 +11,9 @@ var options = {
output: args[2],
width: args[3],
height: args[4],
+ cookieName: args[5],
+ cookieValue: args[6],
+ domain: args[7]
};
page.viewportSize = {
@@ -21,6 +24,11 @@ page.settings.resourceTimeout = 18;
page.onResourceTimeout = function(error) {
phantom.exit(1);
};
+page.addCookie({
+ 'name' : options.cookieName,
+ 'value' : options.cookieValue,
+ 'domain' : options.domain
+});
var requestsArray = [];
@@ -62,7 +70,7 @@ page.onLoadFinished = function(status) {
clearInterval(interval);
var clipRect = page.evaluate(function () {
- return
document.querySelector('#anomaly-screenshot').getBoundingClientRect();
+ return
document.querySelector('.timeseries-chart').getBoundingClientRect();
});
page.clipRect = {
top:clipRect.top,
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch master updated: [TE] frontend - harleyjj/screenshot - update screenshot route to match new Anomalies route (#4194)
This is an automated email from the ASF dual-hosted git repository.
jihao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 45c59f6 [TE] frontend - harleyjj/screenshot - update screenshot route
to match new Anomalies route (#4194)
45c59f6 is described below
commit 45c59f618c7feea0e50eeb579d604c21404b83b7
Author: Harley Jackson
AuthorDate: Mon May 20 10:18:39 2019 -0700
[TE] frontend - harleyjj/screenshot - update screenshot route to match new
Anomalies route (#4194)
Please note that the file
thirdeye/thirdeye-pinot/src/main/resources/scripts/getGraphPnj.js must be
updated for the screenshot to function with the upgrades in this PR.
getGraphPnj.js updates will be in a separate pull request
Updates screenshot graph to new format consistent with the rest of Thirdeye
Updates comments in Anomaly Summary component
---
.../pods/components/anomaly-summary/component.js | 8 +-
.../pods/components/timeseries-chart/component.js | 21 ++-
.../app/pods/screenshot/controller.js | 191 +
.../thirdeye-frontend/app/pods/screenshot/route.js | 30 +++-
.../app/pods/screenshot/template.hbs | 20 +--
5 files changed, 176 insertions(+), 94 deletions(-)
diff --git
a/thirdeye/thirdeye-frontend/app/pods/components/anomaly-summary/component.js
b/thirdeye/thirdeye-frontend/app/pods/components/anomaly-summary/component.js
index 6e90868..2d1e7eb 100644
---
a/thirdeye/thirdeye-frontend/app/pods/components/anomaly-summary/component.js
+++
b/thirdeye/thirdeye-frontend/app/pods/components/anomaly-summary/component.js
@@ -25,7 +25,7 @@ import columns from
'thirdeye-frontend/shared/anomaliesTableColumns';
import moment from 'moment';
import _ from 'lodash';
-const TABLE_DATE_FORMAT = 'MMM DD, hh:mm A'; // format for anomaly table
+const TABLE_DATE_FORMAT = 'MMM DD, hh:mm A'; // format for anomaly table and
legend
export default Component.extend({
/**
@@ -45,15 +45,15 @@ export default Component.extend({
*/
anomalyData: {},
/**
- * Anomaly data, fetched using the anomalyId
+ * current time series
*/
current: null,
/**
- * Anomaly data, fetched using the anomalyId
+ * predicted time series
*/
predicted: null,
/**
- * List of associated classes
+ * imported color mapping for graph
*/
colorMapping: colorMapping,
zoom: {
diff --git
a/thirdeye/thirdeye-frontend/app/pods/components/timeseries-chart/component.js
b/thirdeye/thirdeye-frontend/app/pods/components/timeseries-chart/component.js
index 92d014c..dacb827 100644
---
a/thirdeye/thirdeye-frontend/app/pods/components/timeseries-chart/component.js
+++
b/thirdeye/thirdeye-frontend/app/pods/components/timeseries-chart/component.js
@@ -1,4 +1,4 @@
-import { debounce } from '@ember/runloop';
+import { debounce, later } from '@ember/runloop';
import Component from '@ember/component';
import c3 from 'c3';
import d3 from 'd3';
@@ -210,6 +210,25 @@ export default Component.extend({
}
},
+ didRender(){
+this._super(...arguments);
+
+later(() => {
+ this.notifyPhantomJS();
+});
+ },
+
+ /**
+ * Checks if the page is being viewed from phantomJS
+ * and notifies it that the page is rendered and ready
+ * for a screenshot
+ */
+ notifyPhantomJS() {
+if (typeof window.callPhantom === 'function') {
+ window.callPhantom({message: 'ready'});
+}
+ },
+
didInsertElement() {
this._super(...arguments);
diff --git a/thirdeye/thirdeye-frontend/app/pods/screenshot/controller.js
b/thirdeye/thirdeye-frontend/app/pods/screenshot/controller.js
index 2a19853..0072ad4 100644
--- a/thirdeye/thirdeye-frontend/app/pods/screenshot/controller.js
+++ b/thirdeye/thirdeye-frontend/app/pods/screenshot/controller.js
@@ -1,93 +1,136 @@
-import { alias } from '@ember/object/computed';
-import { computed } from '@ember/object';
+import { colorMapping, makeTime } from 'thirdeye-frontend/utils/rca-utils';
+import {
+ get,
+ computed,
+ getProperties
+} from '@ember/object';
import Controller from '@ember/controller';
+import { humanizeFloat } from 'thirdeye-frontend/utils/utils';
import moment from 'moment';
+import _ from 'lodash';
-export default Controller.extend({
- // Default Legend text and color
- legendText: {
-dotted: {
- text: 'Expected',
- color: 'orange'
-},
-solid: {
- text: 'Current',
- color: 'blue'
-}
- },
+const TABLE_DATE_FORMAT = 'MMM DD, hh:mm A'; // format for anomaly table and
legend
+export default Controller.extend({
/**
- * Padding to be added to the graph
+ * Anomaly data, fetched using the anomalyId
*/
- screenshotPadding: {
-left: 50,
-right: 100
- },
+ anomalyData: {},
+ /**
+ * current time series
+ */
+ current: null,
+ /**
+ * predicted time series
+ */
+ predicted:
[GitHub] [incubator-pinot] jihaozh merged pull request #4194: [TE] frontend - harleyjj/screenshot - update screenshot route to matc…
jihaozh merged pull request #4194: [TE] frontend - harleyjj/screenshot - update screenshot route to matc… URL: https://github.com/apache/incubator-pinot/pull/4194 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch master updated: [TE] frontend - harleyjj/create-alert - break yaml editor into two components (#4206)
This is an automated email from the ASF dual-hosted git repository.
jihao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
The following commit(s) were added to refs/heads/master by this push:
new f9df139 [TE] frontend - harleyjj/create-alert - break yaml editor
into two components (#4206)
f9df139 is described below
commit f9df1393c3e962b3ce5bcaf839f43ddb713492a3
Author: Harley Jackson
AuthorDate: Mon May 20 10:18:00 2019 -0700
[TE] frontend - harleyjj/create-alert - break yaml editor into two
components (#4206)
breaks yaml-editor into two separate components: detection-yaml and
subscription-yaml
Installs new components on create-alert
Moves preview directly onto create-alert and migrates shared props to
controller
These actions will help prepare the create-alert route for forms (full ui).
The next step is to install new components on edit alert.
---
.../pods/components/detection-yaml/component.js| 291 +
.../pods/components/detection-yaml/template.hbs| 58
.../pods/components/subscription-yaml/component.js | 264 +++
.../pods/components/subscription-yaml/template.hbs | 67 +
.../thirdeye-frontend/app/pods/home/index/route.js | 2 +-
.../app/pods/home/share-dashboard/route.js | 2 +-
.../app/pods/self-serve/create-alert/controller.js | 82 +-
.../app/pods/self-serve/create-alert/route.js | 14 +-
.../app/pods/self-serve/create-alert/template.hbs | 67 -
.../app/pods/services/api/anomalies/service.js | 6 +-
10 files changed, 825 insertions(+), 28 deletions(-)
diff --git
a/thirdeye/thirdeye-frontend/app/pods/components/detection-yaml/component.js
b/thirdeye/thirdeye-frontend/app/pods/components/detection-yaml/component.js
new file mode 100644
index 000..bf60125
--- /dev/null
+++ b/thirdeye/thirdeye-frontend/app/pods/components/detection-yaml/component.js
@@ -0,0 +1,291 @@
+/**
+ * Component to render the detection configuration yaml editor.
+ * @module components/detection-editor
+ * @property {Number} alertId - alertId needed in edit mode, to submit changes
to alert config
+ * @property {boolean} isEditMode - to activate the edit mode
+ * @property {String} detectionYaml - the detection yaml
+ * @property {function} setDetectionYaml - updates the detectionYaml in parent
+ * @example
+ {{detection-yaml
+ alertId=model.alertId
+ isEditMode=true
+ detectionYaml=detectionYaml
+ setDetectionYaml=updateDetectionYaml
+ }}
+ * @authors lohuynh and hjackson
+ */
+
+import Component from '@ember/component';
+import {computed, set, get, getProperties, setProperties} from '@ember/object';
+import {checkStatus} from 'thirdeye-frontend/utils/utils';
+import {yamlAlertProps, toastOptions} from 'thirdeye-frontend/utils/constants';
+import yamljs from 'yamljs';
+import RSVP from "rsvp";
+import fetch from 'fetch';
+import {
+ selfServeApiGraph, selfServeApiCommon
+} from 'thirdeye-frontend/utils/api/self-serve';
+import {inject as service} from '@ember/service';
+import config from 'thirdeye-frontend/config/environment';
+
+export default Component.extend({
+ classNames: ['yaml-editor'],
+ notifications: service('toast'),
+ /**
+ * Properties we expect to receive for the yaml-editor
+ */
+ currentMetric: null,
+ isYamlParseable: true,
+ alertTitle: 'Define detection configuration',
+ isEditMode: false,
+ disableYamlSave: true,
+ detectionMsg: '', //General alert failures
+ detectionYaml: null,// The YAML for the anomaly detection
+ currentYamlAlertOriginal: yamlAlertProps,
+ alertId: null, // only needed in edit mode
+ setDetectionYaml: null, // bubble up detectionYaml changes to parent
+
+
+
+ /**
+ * sets Yaml value displayed to contents of detectionYaml or
currentYamlAlertOriginal
+ * @method currentYamlAlert
+ * @return {String}
+ */
+ currentYamlAlert: computed(
+'detectionYaml',
+function() {
+ const inputYaml = get(this, 'detectionYaml');
+ return inputYaml || get(this, 'currentYamlAlertOriginal');
+}
+ ),
+
+ isDetectionMsg: computed(
+'detectionMsg',
+function() {
+ const detectionMsg = get(this, 'detectionMsg');
+ return detectionMsg !== '';
+}
+ ),
+
+ /**
+ * Calls api's for specific metric's autocomplete
+ * @method _loadAutocompleteById
+ * @return Promise
+ */
+ _loadAutocompleteById(metricId) {
+const promiseHash = {
+ filters: fetch(selfServeApiGraph.metricFilters(metricId)).then(res =>
checkStatus(res, 'get', true)),
+ dimensions: fetch(selfServeApiGraph.metricDimensions(metricId)).then(res
=> checkStatus(res, 'get', true))
+};
+return RSVP.hash(promiseHash);
+ },
+
+ /**
+ * Get autocomplete suggestions from relevant api
+ * @method _buildYamlSuggestions
+ * @return Promise
+ */
+ _buildYamlSuggestions(currentMetric, yamlAsObject,
[GitHub] [incubator-pinot] jihaozh merged pull request #4206: [TE] frontend - harleyjj/create-alert - break yaml editor into two co…
jihaozh merged pull request #4206: [TE] frontend - harleyjj/create-alert - break yaml editor into two co… URL: https://github.com/apache/incubator-pinot/pull/4206 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4222: Add startup/shutdown checks for HelixServerStarter
jackjlli commented on a change in pull request #4222: Add startup/shutdown
checks for HelixServerStarter
URL: https://github.com/apache/incubator-pinot/pull/4222#discussion_r285690354
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/utils/CommonConstants.java
##
@@ -171,13 +171,8 @@ public ServerType getServerType() {
public static final String CONFIG_OF_REQUEST_HANDLER_FACTORY_CLASS =
"pinot.server.requestHandlerFactory.class";
public static final String CONFIG_OF_NETTY_PORT =
"pinot.server.netty.port";
public static final String CONFIG_OF_ADMIN_API_PORT =
"pinot.server.adminapi.port";
-public static final String CONFIG_OF_STARTER_ENABLE_SEGMENTS_LOADING_CHECK
=
Review comment:
Can we log these configs as `deprecated` in the server log when startup to
notify these configs have been deprecated? Otherwise, it'll be hard for open
source to notice this changes.
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...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch master updated: [TE] Grouper Interface - Group/Summarize Metric level anomalies (#4212)
This is an automated email from the ASF dual-hosted git repository.
akshayrai09 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
The following commit(s) were added to refs/heads/master by this push:
new c8d68ae [TE] Grouper Interface - Group/Summarize Metric level
anomalies (#4212)
c8d68ae is described below
commit c8d68ae67599d1d35727c385385ae7383843c063
Author: Akshay Rai
AuthorDate: Mon May 20 10:06:28 2019 -0700
[TE] Grouper Interface - Group/Summarize Metric level anomalies (#4212)
* Grouper interface takes a list of anomalies and returns a list of
enriched metric level anomalies.
* Refer to MockGrouper class for a reference implementation
---
.../detection/annotation/DetectionTag.java | 3 +-
.../thirdeye/detection/spi/components/Grouper.java | 2 -
.../thirdeye/detection/wrapper/GrouperWrapper.java | 99 ++
.../yaml/CompositePipelineConfigTranslator.java| 40 +--
.../thirdeye/detection/components/MockGrouper.java | 72 +
.../detection/components/MockGrouperTest.java | 97 +
.../thirdeye/detection/spec/MockGrouperSpec.java | 34 ++
.../CompositePipelineConfigTranslatorTest.java | 12 ++-
.../compositePipelineTranslatorTestResult-1.json | 115 +
.../thirdeye/detection/yaml/pipeline-config-1.yaml | 5 +
.../thirdeye/detection/yaml/pipeline-config-3.yaml | 31 ++
11 files changed, 457 insertions(+), 53 deletions(-)
diff --git
a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/annotation/DetectionTag.java
b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/annotation/DetectionTag.java
index 1b2b6eb..c30120d 100644
---
a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/annotation/DetectionTag.java
+++
b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/annotation/DetectionTag.java
@@ -23,5 +23,6 @@ public enum DetectionTag {
ALGORITHM_DETECTION,
RULE_DETECTION,
ALGORITHM_FILTER,
- RULE_FILTER
+ RULE_FILTER,
+ GROUPER
}
diff --git
a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/spi/components/Grouper.java
b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/spi/components/Grouper.java
index 45c124a..e81ae79 100644
---
a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/spi/components/Grouper.java
+++
b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/spi/components/Grouper.java
@@ -21,8 +21,6 @@ package org.apache.pinot.thirdeye.detection.spi.components;
import org.apache.pinot.thirdeye.datalayer.dto.MergedAnomalyResultDTO;
import org.apache.pinot.thirdeye.detection.spec.AbstractSpec;
-import org.apache.pinot.thirdeye.detection.spi.model.InputData;
-import org.apache.pinot.thirdeye.detection.spi.model.InputDataSpec;
import java.util.List;
diff --git
a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/wrapper/GrouperWrapper.java
b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/wrapper/GrouperWrapper.java
new file mode 100644
index 000..64b1c87
--- /dev/null
+++
b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/wrapper/GrouperWrapper.java
@@ -0,0 +1,99 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.pinot.thirdeye.detection.wrapper;
+
+import com.google.common.base.Preconditions;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import org.apache.commons.collections.MapUtils;
+import org.apache.pinot.thirdeye.datalayer.dto.DetectionConfigDTO;
+import org.apache.pinot.thirdeye.datalayer.dto.MergedAnomalyResultDTO;
+import org.apache.pinot.thirdeye.detection.ConfigUtils;
+import org.apache.pinot.thirdeye.detection.DataProvider;
+import org.apache.pinot.thirdeye.detection.DetectionPipeline;
+import org.apache.pinot.thirdeye.detection.DetectionPipelineResult;
+import org.apache.pinot.thirdeye.detection.DetectionUtils;
+import
[GitHub] [incubator-pinot] akshayrai merged pull request #4212: [TE] Grouper Interface - Group/Summarize Metric level anomalies
akshayrai merged pull request #4212: [TE] Grouper Interface - Group/Summarize Metric level anomalies URL: https://github.com/apache/incubator-pinot/pull/4212 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] katewang0710 commented on issue #4133: add more types parsing for string coerceValueIntoField
katewang0710 commented on issue #4133: add more types parsing for string coerceValueIntoField URL: https://github.com/apache/incubator-pinot/pull/4133#issuecomment-494039960 @kishoreg @Jackie-Jiang Looks like all the tests have passed. Can we merge this change? If anything needs to change please let me know. 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] ananthdurai commented on issue #3998: Upgrade to use Kafka release 2.1.1
ananthdurai commented on issue #3998: Upgrade to use Kafka release 2.1.1 URL: https://github.com/apache/incubator-pinot/issues/3998#issuecomment-493858043 Hi, any update on the Kafka 2.X support? We are running a different version of Kafka to maintain Pinot, would like to move along with the rest of the infrastructure. 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] ananthdurai closed issue #4223: Support Kafka 2.x consumers
ananthdurai closed issue #4223: Support Kafka 2.x consumers URL: https://github.com/apache/incubator-pinot/issues/4223 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] ananthdurai commented on issue #4223: Support Kafka 2.x consumers
ananthdurai commented on issue #4223: Support Kafka 2.x consumers URL: https://github.com/apache/incubator-pinot/issues/4223#issuecomment-493857725 Closing the issue as found it is a duplicate of https://github.com/apache/incubator-pinot/issues/3998 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] ananthdurai opened a new issue #4223: Support Kafka 2.x consumers
ananthdurai opened a new issue #4223: Support Kafka 2.x consumers URL: https://github.com/apache/incubator-pinot/issues/4223 Pinot is dependent on Kafka 0.9 Kafka protocol, which far behind the latest version. Pluggable support for adopting different Kafka version will help to upgrade/ contribute Pinot with the rest of the infrastructure. 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...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
