[GitHub] [incubator-pinot] snleee opened a new issue #4596: Add documentation on schema

2019-09-06 Thread GitBox
snleee opened a new issue #4596: Add documentation on schema
URL: https://github.com/apache/incubator-pinot/issues/4596
 
 
   We currently don't have a good documentation on schema. We should add one at 
https://pinot.readthedocs.io/en/latest
   
   1. What are dimensions, metrics and time column
   2. What kind of types are supported for each type
   3. What kind of other options that we have (`defaultValue, single vs multi 
value...etc`)


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 #4595: Implement all transform function APIs for MapValueTransformFunction

2019-09-06 Thread GitBox
codecov-io edited a comment on issue #4595: Implement all transform function 
APIs for MapValueTransformFunction
URL: https://github.com/apache/incubator-pinot/pull/4595#issuecomment-529062008
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4595?src=pr&el=h1) 
Report
   > Merging 
[#4595](https://codecov.io/gh/apache/incubator-pinot/pull/4595?src=pr&el=desc) 
into 
[master](https://codecov.io/gh/apache/incubator-pinot/commit/68092ab9eb83af173d725ec685c22ba4eb5bacf9?src=pr&el=desc)
 will **decrease** coverage by `21.6%`.
   > The diff coverage is `41.53%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-pinot/pull/4595/graphs/tree.svg?width=650&token=4ibza2ugkz&height=150&src=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4595?src=pr&el=tree)
   
   ```diff
   @@  Coverage Diff  @@
   ## master#4595   +/-   ##
   =
   - Coverage 64.23%   42.62%   -21.61% 
   =
 Files  1072 1072   
 Lines 5570255749   +47 
 Branches   8131 8138+7 
   =
   - Hits  3577823764-12014 
   - Misses1727629870+12594 
   + Partials   2648 2115  -533
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4595?src=pr&el=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[...form/function/TimeConversionTransformFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4595/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVGltZUNvbnZlcnNpb25UcmFuc2Zvcm1GdW5jdGlvbi5qYXZh)
 | `72.22% <ø> (-27.78%)` | `0 <0> (ø)` | |
   | 
[.../function/DateTimeConversionTransformFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4595/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vRGF0ZVRpbWVDb252ZXJzaW9uVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==)
 | `41.02% <ø> (-15.39%)` | `0 <0> (ø)` | |
   | 
[...r/transform/function/ValueInTransformFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4595/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVmFsdWVJblRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=)
 | `22.4% <ø> (-16.81%)` | `0 <0> (ø)` | |
   | 
[...orm/function/SingleParamMathTransformFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4595/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vU2luZ2xlUGFyYW1NYXRoVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==)
 | `0% <ø> (-88.47%)` | `0 <0> (ø)` | |
   | 
[...r/transform/function/TransformFunctionFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/4595/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVHJhbnNmb3JtRnVuY3Rpb25GYWN0b3J5LmphdmE=)
 | `65.51% <100%> (-3.58%)` | `0 <0> (ø)` | |
   | 
[.../transform/function/MapValueTransformFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4595/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vTWFwVmFsdWVUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh)
 | `42.85% <35.59%> (-37.92%)` | `0 <0> (ø)` | |
   | 
[.../org/apache/pinot/common/http/MultiGetRequest.java](https://codecov.io/gh/apache/incubator-pinot/pull/4595/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vaHR0cC9NdWx0aUdldFJlcXVlc3QuamF2YQ==)
 | `0% <0%> (-100%)` | `0% <0%> (ø)` | |
   | 
[...che/pinot/common/restlet/resources/TablesList.java](https://codecov.io/gh/apache/incubator-pinot/pull/4595/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVzdGxldC9yZXNvdXJjZXMvVGFibGVzTGlzdC5qYXZh)
 | `0% <0%> (-100%)` | `0% <0%> (ø)` | |
   | 
[...che/pinot/pql/parsers/pql2/ast/OptionsAstNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/4595/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9wcWwyL2FzdC9PcHRpb25zQXN0Tm9kZS5qYXZh)
 | `0% <0%> (-100%)` | `0% <0%> (ø)` | |
   | 
[...ot/core/query/scheduler/TableBasedGroupMapper.java](https://codecov.io/gh/apache/incubator-pinot/pull/4595/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9zY2hlZHVsZXIvVGFibGVCYXNlZEdyb3VwTWFwcGVyLmphdmE=)
 | `0% <0%> (-100%)` | `0% <0%> (ø)` | |
   | ... and [583 
more](https://codecov.io/gh/apache/incubator-pinot/pull/4595/diff?src=pr&el=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4595?sr

[incubator-pinot] branch map_transform updated (7068e7d -> 416e8d4)

2019-09-06 Thread jackie
This is an automated email from the ASF dual-hosted git repository.

jackie pushed a change to branch map_transform
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git.


 discard 7068e7d  Implement all transform function APIs for 
MapValueTransformFunction
 add 416e8d4  Implement all transform function APIs for 
MapValueTransformFunction

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   (7068e7d)
\
 N -- N -- N   refs/heads/map_transform (416e8d4)

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:
 .../DateTimeConversionTransformFunction.java   |  8 --
 .../function/MapValueTransformFunction.java| 11 
 .../function/SingleParamMathTransformFunction.java |  3 --
 .../function/TimeConversionTransformFunction.java  |  4 ++-
 .../function/TransformFunctionFactory.java | 20 +
 .../function/ValueInTransformFunction.java |  4 ++-
 .../tests/MapTypeClusterIntegrationTest.java   | 33 --
 7 files changed, 57 insertions(+), 26 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 #4595: Implement all transform function APIs for MapValueTransformFunction

2019-09-06 Thread GitBox
Jackie-Jiang commented on a change in pull request #4595: Implement all 
transform function APIs for MapValueTransformFunction
URL: https://github.com/apache/incubator-pinot/pull/4595#discussion_r321951994
 
 

 ##
 File path: 
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/MapValueTransformFunction.java
 ##
 @@ -29,20 +31,25 @@
 
 
 /**
- * map_value(keyColName, 'keyName', valColName)
+ * The MAP_VALUE transform function takes 3 arguments: keyColumn, keyValue, 
valueColumn, where keyColumn and valueColumn
+ * are dictionary-encoded multi-value columns, and keyValue must be a literal 
(number or string). In order to make
+ * MAP_VALUE transform function work, the keyValue provided must exist in the 
keyColumn. To ensure that, the query can
+ * have a filter on the keyColumn, e.g. SELECT mapValue(key, 'myKey', value) 
FROM myTable WHERE key = 'myKey'.
  */
 public class MapValueTransformFunction extends BaseTransformFunction {
+  public static final String FUNCTION_NAME = "mapValue";
 
 Review comment:
   We already have `valueIn`, `timeConvert`, `dateTimeConvert`. Prefer keeping 
them the same way.


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 #4595: Implement all transform function APIs for MapValueTransformFunction

2019-09-06 Thread GitBox
codecov-io commented on issue #4595: Implement all transform function APIs for 
MapValueTransformFunction
URL: https://github.com/apache/incubator-pinot/pull/4595#issuecomment-529062008
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4595?src=pr&el=h1) 
Report
   > Merging 
[#4595](https://codecov.io/gh/apache/incubator-pinot/pull/4595?src=pr&el=desc) 
into 
[master](https://codecov.io/gh/apache/incubator-pinot/commit/68092ab9eb83af173d725ec685c22ba4eb5bacf9?src=pr&el=desc)
 will **increase** coverage by `<.01%`.
   > The diff coverage is `33.89%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-pinot/pull/4595/graphs/tree.svg?width=650&token=4ibza2ugkz&height=150&src=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4595?src=pr&el=tree)
   
   ```diff
   @@ Coverage Diff  @@
   ## master#4595  +/-   ##
   
   + Coverage 64.23%   64.23%   +<.01% 
 Complexity   32   32  
   
 Files  1072 1072  
 Lines 5570255746  +44 
 Branches   8131 8138   +7 
   
   + Hits  3577835807  +29 
   - Misses1727617291  +15 
 Partials   2648 2648
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4595?src=pr&el=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[.../transform/function/MapValueTransformFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4595/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vTWFwVmFsdWVUcmFuc2Zvcm1GdW5jdGlvbi5qYXZh)
 | `41.42% <33.89%> (-39.35%)` | `0 <0> (ø)` | |
   | 
[...elix/core/relocation/RealtimeSegmentRelocator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4595/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlbG9jYXRpb24vUmVhbHRpbWVTZWdtZW50UmVsb2NhdG9yLmphdmE=)
 | `35% <0%> (-10%)` | `0% <0%> (ø)` | |
   | 
[...impl/dictionary/FloatOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4595/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPZmZIZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==)
 | `83.92% <0%> (-5.36%)` | `0% <0%> (ø)` | |
   | 
[...org/apache/pinot/client/DynamicBrokerSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/4595/diff?src=pr&el=tree#diff-cGlub3QtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jbGllbnQvRHluYW1pY0Jyb2tlclNlbGVjdG9yLmphdmE=)
 | `69.69% <0%> (-3.04%)` | `0% <0%> (ø)` | |
   | 
[.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/4595/diff?src=pr&el=tree#diff-cGlub3QtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jbGllbnQvQ29ubmVjdGlvbi5qYXZh)
 | `47.61% <0%> (-2.39%)` | `0% <0%> (ø)` | |
   | 
[...ot/common/protocols/SegmentCompletionProtocol.java](https://codecov.io/gh/apache/incubator-pinot/pull/4595/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcHJvdG9jb2xzL1NlZ21lbnRDb21wbGV0aW9uUHJvdG9jb2wuamF2YQ==)
 | `87.28% <0%> (-0.58%)` | `0% <0%> (ø)` | |
   | 
[...ache/pinot/common/metadata/ZKMetadataProvider.java](https://codecov.io/gh/apache/incubator-pinot/pull/4595/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0YWRhdGEvWktNZXRhZGF0YVByb3ZpZGVyLmphdmE=)
 | `67.41% <0%> (ø)` | `0% <0%> (ø)` | :arrow_down: |
   | 
[...e/impl/dictionary/LongOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4595/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09uSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=)
 | `89.13% <0%> (ø)` | `0% <0%> (ø)` | :arrow_down: |
   | 
[...r/helix/SegmentOnlineOfflineStateModelFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/4595/diff?src=pr&el=tree#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9TZWdtZW50T25saW5lT2ZmbGluZVN0YXRlTW9kZWxGYWN0b3J5LmphdmE=)
 | `61.16% <0%> (+0.97%)` | `0% <0%> (ø)` | :arrow_down: |
   | 
[...lix/core/realtime/PinotRealtimeSegmentManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4595/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL1Bpbm90UmVhbHRpbWVTZWdtZW50TWFuYWdlci5qYXZh)
 | `74.87% <0%> (+1.02%)` | `0% <0%> (ø)` | :arrow_down: |
   | ... and [9 
more](https://codecov.io/gh/apache/incubator-pinot/pull/4595/diff?src=pr&el=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://c

[GitHub] [incubator-pinot] kishoreg commented on a change in pull request #4595: Implement all transform function APIs for MapValueTransformFunction

2019-09-06 Thread GitBox
kishoreg commented on a change in pull request #4595: Implement all transform 
function APIs for MapValueTransformFunction
URL: https://github.com/apache/incubator-pinot/pull/4595#discussion_r321950598
 
 

 ##
 File path: 
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/MapValueTransformFunction.java
 ##
 @@ -29,20 +31,25 @@
 
 
 /**
- * map_value(keyColName, 'keyName', valColName)
+ * The MAP_VALUE transform function takes 3 arguments: keyColumn, keyValue, 
valueColumn, where keyColumn and valueColumn
+ * are dictionary-encoded multi-value columns, and keyValue must be a literal 
(number or string). In order to make
+ * MAP_VALUE transform function work, the keyValue provided must exist in the 
keyColumn. To ensure that, the query can
+ * have a filter on the keyColumn, e.g. SELECT mapValue(key, 'myKey', value) 
FROM myTable WHERE key = 'myKey'.
  */
 public class MapValueTransformFunction extends BaseTransformFunction {
+  public static final String FUNCTION_NAME = "mapValue";
 
 Review comment:
   why camel case. its common to use _ as the separator in sql udf's. See 
presto udf's for example 
https://prestodb.github.io/docs/current/functions/string.html


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 #4585: Presence vector

2019-09-06 Thread GitBox
codecov-io edited a comment on issue #4585: Presence vector
URL: https://github.com/apache/incubator-pinot/pull/4585#issuecomment-528121925
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4585?src=pr&el=h1) 
Report
   > Merging 
[#4585](https://codecov.io/gh/apache/incubator-pinot/pull/4585?src=pr&el=desc) 
into 
[master](https://codecov.io/gh/apache/incubator-pinot/commit/a1c5af7034d20070bd8f1c87c9ac2774d8044f1c?src=pr&el=desc)
 will **decrease** coverage by `8%`.
   > The diff coverage is `93.4%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-pinot/pull/4585/graphs/tree.svg?width=650&token=4ibza2ugkz&height=150&src=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4585?src=pr&el=tree)
   
   ```diff
   @@ Coverage Diff  @@
   ## master#4585  +/-   ##
   
   - Coverage 64.42%   56.41%   -8.01% 
 Complexity   32   32  
   
 Files  1072 1075   +3 
 Lines 5563855785 +147 
 Branches   8112 8142  +30 
   
   - Hits  3584231470-4372 
   - Misses1714221822+4680 
   + Partials   2654 2493 -161
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4585?src=pr&el=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[...e/pinot/core/segment/creator/impl/V1Constants.java](https://codecov.io/gh/apache/incubator-pinot/pull/4585/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L2NyZWF0b3IvaW1wbC9WMUNvbnN0YW50cy5qYXZh)
 | `33.33% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | 
[...org/apache/pinot/common/utils/CommonConstants.java](https://codecov.io/gh/apache/incubator-pinot/pull/4585/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=)
 | `34.61% <ø> (-7.7%)` | `0 <0> (ø)` | |
   | 
[...pinot/core/segment/store/ColumnIndexDirectory.java](https://codecov.io/gh/apache/incubator-pinot/pull/4585/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3N0b3JlL0NvbHVtbkluZGV4RGlyZWN0b3J5LmphdmE=)
 | `93.33% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | 
[.../java/org/apache/pinot/core/common/DataSource.java](https://codecov.io/gh/apache/incubator-pinot/pull/4585/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9jb21tb24vRGF0YVNvdXJjZS5qYXZh)
 | `100% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | 
[...re/startree/v2/store/StarTreeMetricDataSource.java](https://codecov.io/gh/apache/incubator-pinot/pull/4585/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS92Mi9zdG9yZS9TdGFyVHJlZU1ldHJpY0RhdGFTb3VyY2UuamF2YQ==)
 | `41.66% <0%> (-14.86%)` | `0 <0> (ø)` | |
   | 
[...ent/virtualcolumn/VirtualColumnIndexContainer.java](https://codecov.io/gh/apache/incubator-pinot/pull/4585/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3ZpcnR1YWxjb2x1bW4vVmlydHVhbENvbHVtbkluZGV4Q29udGFpbmVyLmphdmE=)
 | `80% <0%> (-20%)` | `0 <0> (ø)` | |
   | 
[...startree/v2/store/StarTreeDimensionDataSource.java](https://codecov.io/gh/apache/incubator-pinot/pull/4585/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS92Mi9zdG9yZS9TdGFyVHJlZURpbWVuc2lvbkRhdGFTb3VyY2UuamF2YQ==)
 | `65.21% <0%> (-7.51%)` | `0 <0> (ø)` | |
   | 
[...t/index/loader/bloomfilter/BloomFilterHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/4585/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L2luZGV4L2xvYWRlci9ibG9vbWZpbHRlci9CbG9vbUZpbHRlckhhbmRsZXIuamF2YQ==)
 | `15% <100%> (-56.67%)` | `0 <0> (ø)` | |
   | 
[...ache/pinot/core/segment/store/ColumnIndexType.java](https://codecov.io/gh/apache/incubator-pinot/pull/4585/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L3N0b3JlL0NvbHVtbkluZGV4VHlwZS5qYXZh)
 | `85.71% <100%> (+1.09%)` | `0 <0> (ø)` | :arrow_down: |
   | 
[...re/segment/index/data/source/ColumnDataSource.java](https://codecov.io/gh/apache/incubator-pinot/pull/4585/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L2luZGV4L2RhdGEvc291cmNlL0NvbHVtbkRhdGFTb3VyY2UuamF2YQ==)
 | `93.47% <100%> (-1.98%)` | `0 <0> (ø)` | |
   | ... and [306 
more](https://codecov.io/gh/apache/incubator-pinot/pull/4585/diff?src=pr&el=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4585?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)

[incubator-pinot] branch map_transform created (now 7068e7d)

2019-09-06 Thread jackie
This is an automated email from the ASF dual-hosted git repository.

jackie pushed a change to branch map_transform
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git.


  at 7068e7d  Implement all transform function APIs for 
MapValueTransformFunction

This branch includes the following new commits:

 new 7068e7d  Implement all transform function APIs for 
MapValueTransformFunction

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



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



[GitHub] [incubator-pinot] Jackie-Jiang opened a new pull request #4595: Implement all transform function APIs for MapValueTransformFunction

2019-09-06 Thread GitBox
Jackie-Jiang opened a new pull request #4595: Implement all transform function 
APIs for MapValueTransformFunction
URL: https://github.com/apache/incubator-pinot/pull/4595
 
 
   - Change function name to "mapValue" to follow the convention
   - Add documentation for MapValue transform function
   - Add more checks on the arguments
   - Enhance the test


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] 01/01: Implement all transform function APIs for MapValueTransformFunction

2019-09-06 Thread jackie
This is an automated email from the ASF dual-hosted git repository.

jackie pushed a commit to branch map_transform
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git

commit 7068e7d1a11c46415f6dff04ff0d467179ac34ba
Author: Jackie (Xiaotian) Jiang 
AuthorDate: Fri Sep 6 18:19:14 2019 -0700

Implement all transform function APIs for MapValueTransformFunction

- Change function name to "mapValue" to follow the convention
- Add documentation for MapValue transform function
- Add more checks on the arguments
- Enhance the test
---
 .../function/MapValueTransformFunction.java| 142 +++---
 .../tests/MapTypeClusterIntegrationTest.java   | 208 -
 2 files changed, 193 insertions(+), 157 deletions(-)

diff --git 
a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/MapValueTransformFunction.java
 
b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/MapValueTransformFunction.java
index 8053d3a..b451a6e 100644
--- 
a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/MapValueTransformFunction.java
+++ 
b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/MapValueTransformFunction.java
@@ -18,9 +18,11 @@
  */
 package org.apache.pinot.core.operator.transform.function;
 
+import com.google.common.base.Preconditions;
 import java.util.List;
 import java.util.Map;
 import javax.annotation.Nonnull;
+import org.apache.pinot.common.data.FieldSpec.DataType;
 import org.apache.pinot.core.common.DataSource;
 import org.apache.pinot.core.operator.blocks.ProjectionBlock;
 import org.apache.pinot.core.operator.transform.TransformResultMetadata;
@@ -29,20 +31,25 @@ import 
org.apache.pinot.core.segment.index.readers.Dictionary;
 
 
 /**
- * map_value(keyColName, 'keyName', valColName)
+ * The MAP_VALUE transform function takes 3 arguments: keyColumn, keyValue, 
valueColumn, where keyColumn and valueColumn
+ * are dictionary-encoded multi-value columns, and keyValue must be a literal 
(number or string). In order to make
+ * MAP_VALUE transform function work, the keyValue provided must exist in the 
keyColumn. To ensure that, the query can
+ * have a filter on the keyColumn, e.g. SELECT mapValue(key, 'myKey', value) 
FROM myTable WHERE key = 'myKey'.
  */
 public class MapValueTransformFunction extends BaseTransformFunction {
+  public static final String FUNCTION_NAME = "mapValue";
 
-  public static final String FUNCTION_NAME = "map_value";
-
-  private TransformResultMetadata _resultMetadata;
   private TransformFunction _keyColumnFunction;
-  private String _keyName;
-  private TransformFunction _valueColumnFunction;
-  private int[][] _keyDictIds;
-  private int[][] _valueDictIds;
   private int _inputKeyDictId;
-  private int[] _outputValueDictIds;
+  private TransformFunction _valueColumnFunction;
+  private TransformResultMetadata _resultMetadata;
+
+  private int[] _dictIds;
+  private int[] _intValues;
+  private long[] _longValues;
+  private float[] _floatValues;
+  private double[] _doubleValues;
+  private String[] _stringValues;
 
   @Override
   public String getName() {
@@ -51,17 +58,27 @@ public class MapValueTransformFunction extends 
BaseTransformFunction {
 
   @Override
   public void init(@Nonnull List arguments, @Nonnull 
Map dataSourceMap) {
-int numArguments = arguments.size();
-if (numArguments != 3) {
-  throw new IllegalArgumentException("3 arguments are required for 
MAP_VALUE transform function map_value(keyColName, 'keyName', valColName)");
-}
+Preconditions.checkArgument(arguments.size() == 3,
+"3 arguments are required for MAP_VALUE transform function: keyColumn, 
keyValue, valueColumn, e.g. mapValue(key, 'myKey', value)");
+
 _keyColumnFunction = arguments.get(0);
-_keyName = ((LiteralTransformFunction) arguments.get(1)).getLiteral();
+TransformResultMetadata keyColumnMetadata = 
_keyColumnFunction.getResultMetadata();
+Preconditions.checkState(!keyColumnMetadata.isSingleValue() && 
keyColumnMetadata.hasDictionary(),
+"Key column must be dictionary-encoded multi-value column");
+
+TransformFunction keyValueFunction = arguments.get(1);
+Preconditions.checkState(keyValueFunction instanceof 
LiteralTransformFunction,
+"Key value must be a literal (number or string)");
+String keyValue = ((LiteralTransformFunction) 
keyValueFunction).getLiteral();
+_inputKeyDictId = _keyColumnFunction.getDictionary().indexOf(keyValue);
+Preconditions.checkState(_inputKeyDictId >= 0,
+"Key value must exist in the key column. Add filter on key column if 
necessary, e.g. SELECT mapValue(key, 'myKey', value) FROM myTable WHERE key = 
'myKey'");
+
 _valueColumnFunction = arguments.get(2);
 TransformResultMetadata valueColumnMetadata = 
_valueColumnFunction.getResultMetadata();
-_resultMetadata =
-new TransformResultMetadata(valueColumnMetada

[incubator-pinot] branch k8s-quickstart updated (53c2383 -> ef03017)

2019-09-06 Thread xiangfu
This is an automated email from the ASF dual-hosted git repository.

xiangfu pushed a change to branch k8s-quickstart
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git.


from 53c2383  Adding license
 add ef03017  address comments

No new revisions were added by this update.

Summary of changes:
 kubernetes/examples/gke/skaffold/cleanup.sh | 10 +-
 kubernetes/examples/gke/skaffold/setup.sh   | 13 ++---
 2 files changed, 11 insertions(+), 12 deletions(-)


-
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 #4583: Support default value for Byte column

2019-09-06 Thread GitBox
jackjlli commented on a change in pull request #4583: Support default value for 
Byte column
URL: https://github.com/apache/incubator-pinot/pull/4583#discussion_r321940046
 
 

 ##
 File path: 
pinot-core/src/main/java/org/apache/pinot/core/segment/index/loader/defaultcolumn/BaseDefaultColumnHandler.java
 ##
 @@ -303,6 +304,10 @@ protected void createColumnV1Indices(String column)
 dictionaryElementSize = 
StringUtil.encodeUtf8(stringDefaultValue).length;
 sortedArray = new String[]{stringDefaultValue};
 break;
+  case BYTES:
+Preconditions.checkState(defaultValue instanceof byte[]);
+sortedArray = new ByteArray[] {new ByteArray((byte[]) defaultValue)};
 
 Review comment:
   Actually the value is already of byte array type. There is no need to decode 
it again. We can document what/how to put a default value for byte column.


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 #4583: Support default value for Byte column

2019-09-06 Thread GitBox
jackjlli commented on a change in pull request #4583: Support default value for 
Byte column
URL: https://github.com/apache/incubator-pinot/pull/4583#discussion_r321940046
 
 

 ##
 File path: 
pinot-core/src/main/java/org/apache/pinot/core/segment/index/loader/defaultcolumn/BaseDefaultColumnHandler.java
 ##
 @@ -303,6 +304,10 @@ protected void createColumnV1Indices(String column)
 dictionaryElementSize = 
StringUtil.encodeUtf8(stringDefaultValue).length;
 sortedArray = new String[]{stringDefaultValue};
 break;
+  case BYTES:
+Preconditions.checkState(defaultValue instanceof byte[]);
+sortedArray = new ByteArray[] {new ByteArray((byte[]) defaultValue)};
 
 Review comment:
   Actually the value is already of byte array type. We can document what/how 
to put a default value for byte column.


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 #4583: Support default value for Byte column

2019-09-06 Thread GitBox
jackjlli commented on a change in pull request #4583: Support default value for 
Byte column
URL: https://github.com/apache/incubator-pinot/pull/4583#discussion_r321939628
 
 

 ##
 File path: pinot-core/src/test/resources/data/newColumnsSchema3.json
 ##
 @@ -16,6 +16,11 @@
 {
   "dataType": "DOUBLE",
   "name": "newDoubleMetric"
+},
+{
+  "dataType": "BYTES",
+  "defaultNullValue": "000800ac",
 
 Review comment:
   Yes, this is helix string of the 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] jackjlli commented on issue #4568: Replace Hashmap to Object2IntOpenHashMap in OnHeapStringDictionary

2019-09-06 Thread GitBox
jackjlli commented on issue #4568: Replace Hashmap to Object2IntOpenHashMap in 
OnHeapStringDictionary
URL: https://github.com/apache/incubator-pinot/pull/4568#issuecomment-529028012
 
 
   I've updated the test and here are the results comparing 
`Object2IntOpenHashMap` with `HashMap`.
   
   `Object2IntOpenHashMap`:
   ```
   Total time for building segment: 22239
   Total time for 1000 lookups: 1876ms
   Memory usage: 2168980704
   
DictSize,TimeTaken(ms),SegmentSize,NumLookups,Min,Max,Mean,StdDev,Median,Skewness,Kurtosis,Variance,BufferSize
   200,1876,1029251120,1000,1000.0,1000.0,1000.0,0.0,1000.0,NaN,NaN,0.0
   ```
   ```
   Total time for building segment: 22667
   Total time for 1000 lookups: 1901ms
   Memory usage: 2168993176
   
DictSize,TimeTaken(ms),SegmentSize,NumLookups,Min,Max,Mean,StdDev,Median,Skewness,Kurtosis,Variance,BufferSize
   200,1901,1029251120,1000,1000.0,1000.0,1000.0,0.0,1000.0,NaN,NaN,0.0
   ```
   ```
   Total time for building segment: 22174
   Total time for 1000 lookups: 1880ms
   Memory usage: 2168980824
   
DictSize,TimeTaken(ms),SegmentSize,NumLookups,Min,Max,Mean,StdDev,Median,Skewness,Kurtosis,Variance,BufferSize
   200,1880,1029251120,1000,1000.0,1000.0,1000.0,0.0,1000.0,NaN,NaN,0.0
   ```
   
   `HashMap`:
   ```
   Total time for building segment: 22343
   Total time for 1000 lookups: 2294ms
   Memory usage: 2248214504
   
DictSize,TimeTaken(ms),SegmentSize,NumLookups,Min,Max,Mean,StdDev,Median,Skewness,Kurtosis,Variance,BufferSize
   200,2294,1029251120,1000,1000.0,1000.0,1000.0,0.0,1000.0,NaN,NaN,0.0
   ```
   ```
   Total time for building segment: 24396
   Total time for 1000 lookups: 2244ms
   Memory usage: 2248201544
   
DictSize,TimeTaken(ms),SegmentSize,NumLookups,Min,Max,Mean,StdDev,Median,Skewness,Kurtosis,Variance,BufferSize
   200,2244,1029251120,1000,1000.0,1000.0,1000.0,0.0,1000.0,NaN,NaN,0.0
   ```
   ```
   Total time for building segment: 23854
   Total time for 1000 lookups: 2216ms
   Memory usage: 2248211648
   
DictSize,TimeTaken(ms),SegmentSize,NumLookups,Min,Max,Mean,StdDev,Median,Skewness,Kurtosis,Variance,BufferSize
   200,2216,1029251120,1000,1000.0,1000.0,1000.0,0.0,1000.0,NaN,NaN,0.0
   ```
   As shown above, `Object2IntOpenHashMap` uses less memory and it takes less 
time on lookups.


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 #4593: Do not update download URL and crypter when segment is not refreshed

2019-09-06 Thread GitBox
codecov-io commented on issue #4593: Do not update download URL and crypter 
when segment is not refreshed
URL: https://github.com/apache/incubator-pinot/pull/4593#issuecomment-529022606
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4593?src=pr&el=h1) 
Report
   > Merging 
[#4593](https://codecov.io/gh/apache/incubator-pinot/pull/4593?src=pr&el=desc) 
into 
[master](https://codecov.io/gh/apache/incubator-pinot/commit/68092ab9eb83af173d725ec685c22ba4eb5bacf9?src=pr&el=desc)
 will **increase** coverage by `0.15%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-pinot/pull/4593/graphs/tree.svg?width=650&token=4ibza2ugkz&height=150&src=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4593?src=pr&el=tree)
   
   ```diff
   @@ Coverage Diff  @@
   ## master#4593  +/-   ##
   
   + Coverage 64.23%   64.38%   +0.15% 
 Complexity   32   32  
   
 Files  1072 1072  
 Lines 5570255703   +1 
 Branches   8131 8131  
   
   + Hits  3577835867  +89 
   + Misses1727617188  -88 
 Partials   2648 2648
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4593?src=pr&el=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4593/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==)
 | `62.37% <100%> (-0.03%)` | `0 <0> (ø)` | |
   | 
[...apache/pinot/controller/api/upload/ZKOperator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4593/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvdXBsb2FkL1pLT3BlcmF0b3IuamF2YQ==)
 | `62.63% <100%> (-0.41%)` | `0 <0> (ø)` | |
   | 
[...impl/dictionary/DoubleOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4593/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT25IZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==)
 | `60.86% <0%> (-13.05%)` | `0% <0%> (ø)` | |
   | 
[...impl/dictionary/FloatOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4593/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPZmZIZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==)
 | `80.35% <0%> (-8.93%)` | `0% <0%> (ø)` | |
   | 
[...e/impl/dictionary/LongOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4593/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09uSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=)
 | `82.6% <0%> (-6.53%)` | `0% <0%> (ø)` | |
   | 
[...elix/core/relocation/RealtimeSegmentRelocator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4593/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlbG9jYXRpb24vUmVhbHRpbWVTZWdtZW50UmVsb2NhdG9yLmphdmE=)
 | `40% <0%> (-5%)` | `0% <0%> (ø)` | |
   | 
[...troller/helix/core/retention/RetentionManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4593/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JldGVudGlvbi9SZXRlbnRpb25NYW5hZ2VyLmphdmE=)
 | `75% <0%> (-4.17%)` | `0% <0%> (ø)` | |
   | 
[...roker/requesthandler/BaseBrokerRequestHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/4593/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcmVxdWVzdGhhbmRsZXIvQmFzZUJyb2tlclJlcXVlc3RIYW5kbGVyLmphdmE=)
 | `74.16% <0%> (-3.34%)` | `0% <0%> (ø)` | |
   | 
[.../broker/routing/HelixExternalViewBasedRouting.java](https://codecov.io/gh/apache/incubator-pinot/pull/4593/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9IZWxpeEV4dGVybmFsVmlld0Jhc2VkUm91dGluZy5qYXZh)
 | `87.85% <0%> (-3.12%)` | `0% <0%> (ø)` | |
   | ... and [25 
more](https://codecov.io/gh/apache/incubator-pinot/pull/4593/diff?src=pr&el=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4593?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/459

[GitHub] [incubator-pinot] icefury71 commented on a change in pull request #4585: Presence vector

2019-09-06 Thread GitBox
icefury71 commented on a change in pull request #4585: Presence vector
URL: https://github.com/apache/incubator-pinot/pull/4585#discussion_r321862314
 
 

 ##
 File path: 
pinot-core/src/main/java/org/apache/pinot/core/data/recordtransformer/NullValueTransformer.java
 ##
 @@ -33,17 +36,35 @@ public NullValueTransformer(Schema schema) {
 
   @Override
   public GenericRow transform(GenericRow record) {
+Set nullColumnNamesSet = null;
+
+// Clear out the 'null_fields' value in case the Generic row is reused
+record.putField(NULL_FIELDS, null);
+
 for (FieldSpec fieldSpec : _fieldSpecs) {
   String fieldName = fieldSpec.getName();
   // Do not allow default value for time column
   if (record.getValue(fieldName) == null && fieldSpec.getFieldType() != 
FieldSpec.FieldType.TIME) {
+if (nullColumnNamesSet == null) {
+  nullColumnNamesSet = new HashSet<>();
+}
+
+// Only handle null columns for non-virtual columns
+if (!fieldSpec.isVirtualColumn()) {
+  nullColumnNamesSet.add(fieldName);
+}
+
 if (fieldSpec.isSingleValueField()) {
   record.putField(fieldName, fieldSpec.getDefaultNullValue());
 } else {
   record.putField(fieldName, new 
Object[]{fieldSpec.getDefaultNullValue()});
 }
   }
 }
+
+if (nullColumnNamesSet != null) {
+  record.putField(NULL_FIELDS, String.join(",", nullColumnNamesSet));
 
 Review comment:
   I think the concern is the size of a Java object which is potentially bigger 
in this case. 
   
   One optimization here is that if we have a fixed order of column names (in 
the Schema), we can simply use a BitSet to keep track of null columns (which 
will typically be around 64 bytes ish). This can save a lot of effort in other 
parts of the code base as well.


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 opened a new pull request #4594: [TE] fix create alert error message null pointer exception

2019-09-06 Thread GitBox
jihaozh opened a new pull request #4594: [TE] fix create alert error message 
null pointer exception
URL: https://github.com/apache/incubator-pinot/pull/4594
 
 
   This PR fixes the create alert error message null pointer exception.


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 opened a new pull request #4593: Do not update download URL and crypter when segment is not refreshed

2019-09-06 Thread GitBox
Jackie-Jiang opened a new pull request #4593: Do not update download URL and 
crypter when segment is not refreshed
URL: https://github.com/apache/incubator-pinot/pull/4593
 
 
   When segment is not refreshed (same crc so no need to refresh),
   segment file won't be persisted, so should not change download URL
   and crypter field in ZK metadata


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 fix_download_uri created (now 7dea9a1)

2019-09-06 Thread jackie
This is an automated email from the ASF dual-hosted git repository.

jackie pushed a change to branch fix_download_uri
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git.


  at 7dea9a1  Do not update download URL and crypter when segment is not 
refreshed

No new revisions were added by this update.


-
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 #4545: Add APIs to get leader for all tables or a given table

2019-09-06 Thread GitBox
codecov-io edited a comment on issue #4545: Add APIs to get leader for all 
tables or a given table
URL: https://github.com/apache/incubator-pinot/pull/4545#issuecomment-523237858
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4545?src=pr&el=h1) 
Report
   > Merging 
[#4545](https://codecov.io/gh/apache/incubator-pinot/pull/4545?src=pr&el=desc) 
into 
[master](https://codecov.io/gh/apache/incubator-pinot/commit/d62f467adc3f33bfe44017835659a920731c36a3?src=pr&el=desc)
 will **increase** coverage by `20.95%`.
   > The diff coverage is `26.96%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-pinot/pull/4545/graphs/tree.svg?width=650&token=4ibza2ugkz&height=150&src=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4545?src=pr&el=tree)
   
   ```diff
   @@  Coverage Diff  @@
   ## master#4545   +/-   ##
   =
   + Coverage 43.45%   64.41%   +20.95% 
   - Complexity0   32   +32 
   =
 Files  1073 1073   
 Lines 5596155768  -193 
 Branches   8184 8139   -45 
   =
   + Hits  2432035925+11605 
   + Misses2950517193-12312 
   - Partials   2136 2650  +514
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4545?src=pr&el=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[...ache/pinot/controller/api/resources/Constants.java](https://codecov.io/gh/apache/incubator-pinot/pull/4545/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL0NvbnN0YW50cy5qYXZh)
 | `28.57% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | 
[...apache/pinot/controller/LeadControllerManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4545/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9MZWFkQ29udHJvbGxlck1hbmFnZXIuamF2YQ==)
 | `75% <ø> (+8.7%)` | `0 <0> (ø)` | :arrow_down: |
   | 
[.../resources/PinotLeadControllerRestletResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/4545/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90TGVhZENvbnRyb2xsZXJSZXN0bGV0UmVzb3VyY2UuamF2YQ==)
 | `0% <0%> (ø)` | `0 <0> (?)` | |
   | 
[.../pinot/common/utils/helix/LeadControllerUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/4545/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvaGVsaXgvTGVhZENvbnRyb2xsZXJVdGlscy5qYXZh)
 | `95.83% <100%> (+15.83%)` | `0 <0> (ø)` | :arrow_down: |
   | 
[...pinot/server/realtime/ControllerLeaderLocator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4545/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VydmVyL3JlYWx0aW1lL0NvbnRyb2xsZXJMZWFkZXJMb2NhdG9yLmphdmE=)
 | `94.87% <100%> (+39.92%)` | `0 <0> (ø)` | :arrow_down: |
   | 
[...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4545/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==)
 | `50% <0%> (-50%)` | `0% <0%> (ø)` | |
   | 
[...he/pinot/core/query/pruner/ValidSegmentPruner.java](https://codecov.io/gh/apache/incubator-pinot/pull/4545/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9wcnVuZXIvVmFsaWRTZWdtZW50UHJ1bmVyLmphdmE=)
 | `57.14% <0%> (-28.58%)` | `0% <0%> (ø)` | |
   | 
[...egation/function/customobject/MinMaxRangePair.java](https://codecov.io/gh/apache/incubator-pinot/pull/4545/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9jdXN0b21vYmplY3QvTWluTWF4UmFuZ2VQYWlyLmphdmE=)
 | `75.86% <0%> (-24.14%)` | `0% <0%> (ø)` | |
   | 
[...er/api/resources/LLCSegmentCompletionHandlers.java](https://codecov.io/gh/apache/incubator-pinot/pull/4545/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL0xMQ1NlZ21lbnRDb21wbGV0aW9uSGFuZGxlcnMuamF2YQ==)
 | `35.1% <0%> (-20%)` | `0% <0%> (ø)` | |
   | 
[...broker/broker/helix/LiveInstanceChangeHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/4545/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0xpdmVJbnN0YW5jZUNoYW5nZUhhbmRsZXIuamF2YQ==)
 | `60% <0%> (-13.34%)` | `0% <0%> (ø)` | |
   | ... and [614 
more](https://codecov.io/gh/apache/incubator-pinot/pull/4545/diff?src=pr&el=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codeco

[GitHub] [incubator-pinot] icefury71 commented on a change in pull request #4585: Presence vector

2019-09-06 Thread GitBox
icefury71 commented on a change in pull request #4585: Presence vector
URL: https://github.com/apache/incubator-pinot/pull/4585#discussion_r321870818
 
 

 ##
 File path: 
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/ClusterIntegrationTestUtils.java
 ##
 @@ -283,6 +283,7 @@ public static void buildSegmentsFromAvro(List 
avroFiles, int baseSegmentIn
   TarGzCompressionUtils
   .createTarGzOfDirectory(segmentFile.getAbsolutePath(), new 
File(tarDir, segmentName).getAbsolutePath());
 } catch (Exception e) {
+  e.printStackTrace();
 
 Review comment:
   This seems a mistake. Will remove this.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


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 a change in pull request #4585: Presence vector

2019-09-06 Thread GitBox
icefury71 commented on a change in pull request #4585: Presence vector
URL: https://github.com/apache/incubator-pinot/pull/4585#discussion_r321870731
 
 

 ##
 File path: pinot-core/src/test/resources/data/test_presence_vector_data.json
 ##
 @@ -0,0 +1,12 @@
+{
 
 Review comment:
   I wanted to leave one column non-null (in this case description). 


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 #4568: Replace Hashmap to Object2IntOpenHashMap in OnHeapStringDictionary

2019-09-06 Thread GitBox
codecov-io edited a comment on issue #4568: Replace Hashmap to 
Object2IntOpenHashMap in OnHeapStringDictionary
URL: https://github.com/apache/incubator-pinot/pull/4568#issuecomment-526762827
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4568?src=pr&el=h1) 
Report
   > :exclamation: No coverage uploaded for pull request base 
(`master@e98efcb`). [Click here to learn what that 
means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
   > The diff coverage is `62.5%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-pinot/pull/4568/graphs/tree.svg?width=650&token=4ibza2ugkz&height=150&src=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4568?src=pr&el=tree)
   
   ```diff
   @@Coverage Diff@@
   ## master#4568   +/-   ##
   =
 Coverage  ?   64.44%   
 Complexity?   32   
   =
 Files ? 1072   
 Lines ?55703   
 Branches  ? 8130   
   =
 Hits  ?35896   
 Misses?17160   
 Partials  ? 2647
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4568?src=pr&el=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[.../segment/index/readers/OnHeapStringDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4568/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L2luZGV4L3JlYWRlcnMvT25IZWFwU3RyaW5nRGljdGlvbmFyeS5qYXZh)
 | `60.71% <62.5%> (ø)` | `0 <0> (?)` | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4568?src=pr&el=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4568?src=pr&el=footer).
 Last update 
[e98efcb...246c829](https://codecov.io/gh/apache/incubator-pinot/pull/4568?src=pr&el=lastupdated).
 Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


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 #4590: Disallowing multiple inputs in preprocess

2019-09-06 Thread GitBox
codecov-io commented on issue #4590: Disallowing multiple inputs in preprocess
URL: https://github.com/apache/incubator-pinot/pull/4590#issuecomment-528972966
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4590?src=pr&el=h1) 
Report
   > Merging 
[#4590](https://codecov.io/gh/apache/incubator-pinot/pull/4590?src=pr&el=desc) 
into 
[master](https://codecov.io/gh/apache/incubator-pinot/commit/68092ab9eb83af173d725ec685c22ba4eb5bacf9?src=pr&el=desc)
 will **increase** coverage by `0.11%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-pinot/pull/4590/graphs/tree.svg?width=650&token=4ibza2ugkz&height=150&src=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4590?src=pr&el=tree)
   
   ```diff
   @@ Coverage Diff  @@
   ## master#4590  +/-   ##
   
   + Coverage 64.23%   64.34%   +0.11% 
 Complexity   32   32  
   
 Files  1072 1072  
 Lines 5570255702  
 Branches   8131 8131  
   
   + Hits  3577835840  +62 
   + Misses1727617219  -57 
   + Partials   2648 2643   -5
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4590?src=pr&el=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[...elix/core/relocation/RealtimeSegmentRelocator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4590/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlbG9jYXRpb24vUmVhbHRpbWVTZWdtZW50UmVsb2NhdG9yLmphdmE=)
 | `22.5% <0%> (-22.5%)` | `0% <0%> (ø)` | |
   | 
[...ache/pinot/common/utils/TarGzCompressionUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/4590/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvVGFyR3pDb21wcmVzc2lvblV0aWxzLmphdmE=)
 | `68.23% <0%> (-12.95%)` | `0% <0%> (ø)` | |
   | 
[...impl/dictionary/DoubleOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4590/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT25IZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==)
 | `65.21% <0%> (-8.7%)` | `0% <0%> (ø)` | |
   | 
[.../apache/pinot/common/config/RealtimeTagConfig.java](https://codecov.io/gh/apache/incubator-pinot/pull/4590/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL1JlYWx0aW1lVGFnQ29uZmlnLmphdmE=)
 | `93.33% <0%> (-6.67%)` | `0% <0%> (ø)` | |
   | 
[.../impl/dictionary/FloatOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4590/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPbkhlYXBNdXRhYmxlRGljdGlvbmFyeS5qYXZh)
 | `78.26% <0%> (-4.35%)` | `0% <0%> (ø)` | |
   | 
[...troller/helix/core/retention/RetentionManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4590/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JldGVudGlvbi9SZXRlbnRpb25NYW5hZ2VyLmphdmE=)
 | `75% <0%> (-4.17%)` | `0% <0%> (ø)` | |
   | 
[...org/apache/pinot/client/DynamicBrokerSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/4590/diff?src=pr&el=tree#diff-cGlub3QtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jbGllbnQvRHluYW1pY0Jyb2tlclNlbGVjdG9yLmphdmE=)
 | `69.69% <0%> (-3.04%)` | `0% <0%> (ø)` | |
   | 
[.../broker/routing/HelixExternalViewBasedRouting.java](https://codecov.io/gh/apache/incubator-pinot/pull/4590/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9IZWxpeEV4dGVybmFsVmlld0Jhc2VkUm91dGluZy5qYXZh)
 | `88.47% <0%> (-2.5%)` | `0% <0%> (ø)` | |
   | 
[.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/4590/diff?src=pr&el=tree#diff-cGlub3QtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jbGllbnQvQ29ubmVjdGlvbi5qYXZh)
 | `47.61% <0%> (-2.39%)` | `0% <0%> (ø)` | |
   | 
[...pby/NoDictionarySingleColumnGroupKeyGenerator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4590/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9ncm91cGJ5L05vRGljdGlvbmFyeVNpbmdsZUNvbHVtbkdyb3VwS2V5R2VuZXJhdG9yLmphdmE=)
 | `87.5% <0%> (-0.97%)` | `0% <0%> (ø)` | |
   | ... and [25 
more](https://codecov.io/gh/apache/incubator-pinot/pull/4590/diff?src=pr&el=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4590?src=pr&el=continue).
   > **Legend** - [Click he

[GitHub] [incubator-pinot] codecov-io commented on issue #4591: Deleting preprocess output post segment creation

2019-09-06 Thread GitBox
codecov-io commented on issue #4591: Deleting preprocess output post segment 
creation
URL: https://github.com/apache/incubator-pinot/pull/4591#issuecomment-528971712
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4591?src=pr&el=h1) 
Report
   > Merging 
[#4591](https://codecov.io/gh/apache/incubator-pinot/pull/4591?src=pr&el=desc) 
into 
[master](https://codecov.io/gh/apache/incubator-pinot/commit/68092ab9eb83af173d725ec685c22ba4eb5bacf9?src=pr&el=desc)
 will **increase** coverage by `0.22%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-pinot/pull/4591/graphs/tree.svg?width=650&token=4ibza2ugkz&height=150&src=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4591?src=pr&el=tree)
   
   ```diff
   @@ Coverage Diff  @@
   ## master#4591  +/-   ##
   
   + Coverage 64.23%   64.45%   +0.22% 
 Complexity   32   32  
   
 Files  1072 1072  
 Lines 5570255702  
 Branches   8131 8131  
   
   + Hits  3577835903 +125 
   + Misses1727617148 -128 
   - Partials   2648 2651   +3
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4591?src=pr&el=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4591/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==)
 | `50% <0%> (-25%)` | `0% <0%> (ø)` | |
   | 
[...elix/core/relocation/RealtimeSegmentRelocator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4591/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlbG9jYXRpb24vUmVhbHRpbWVTZWdtZW50UmVsb2NhdG9yLmphdmE=)
 | `22.5% <0%> (-22.5%)` | `0% <0%> (ø)` | |
   | 
[...ache/pinot/common/utils/TarGzCompressionUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/4591/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvVGFyR3pDb21wcmVzc2lvblV0aWxzLmphdmE=)
 | `68.23% <0%> (-12.95%)` | `0% <0%> (ø)` | |
   | 
[...thandler/SingleConnectionBrokerRequestHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/4591/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcmVxdWVzdGhhbmRsZXIvU2luZ2xlQ29ubmVjdGlvbkJyb2tlclJlcXVlc3RIYW5kbGVyLmphdmE=)
 | `79.54% <0%> (-9.1%)` | `0% <0%> (ø)` | |
   | 
[...impl/dictionary/DoubleOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4591/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT25IZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==)
 | `65.21% <0%> (-8.7%)` | `0% <0%> (ø)` | |
   | 
[...n/java/org/apache/pinot/core/transport/Server.java](https://codecov.io/gh/apache/incubator-pinot/pull/4591/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS90cmFuc3BvcnQvU2VydmVyLmphdmE=)
 | `59.25% <0%> (-7.41%)` | `0% <0%> (ø)` | |
   | 
[.../apache/pinot/common/config/RealtimeTagConfig.java](https://codecov.io/gh/apache/incubator-pinot/pull/4591/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL1JlYWx0aW1lVGFnQ29uZmlnLmphdmE=)
 | `93.33% <0%> (-6.67%)` | `0% <0%> (ø)` | |
   | 
[...e/impl/dictionary/LongOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4591/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09uSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=)
 | `82.6% <0%> (-6.53%)` | `0% <0%> (ø)` | |
   | 
[...impl/dictionary/FloatOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4591/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPZmZIZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==)
 | `83.92% <0%> (-5.36%)` | `0% <0%> (ø)` | |
   | 
[...troller/helix/core/retention/RetentionManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4591/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JldGVudGlvbi9SZXRlbnRpb25NYW5hZ2VyLmphdmE=)
 | `75% <0%> (-4.17%)` | `0% <0%> (ø)` | |
   | ... and [28 
more](https://codecov.io/gh/apache/incubator-pinot/pull/4591/diff?src=pr&el=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/45

[GitHub] [incubator-pinot] icefury71 commented on a change in pull request #4585: Presence vector

2019-09-06 Thread GitBox
icefury71 commented on a change in pull request #4585: Presence vector
URL: https://github.com/apache/incubator-pinot/pull/4585#discussion_r321864108
 
 

 ##
 File path: 
pinot-core/src/main/java/org/apache/pinot/core/indexsegment/mutable/MutableSegmentImpl.java
 ##
 @@ -479,6 +521,20 @@ public GenericRow getRecord(int docId, GenericRow reuse) {
   .getValue(docId, fieldSpec, _indexReaderWriterMap.get(column), 
_dictionaryMap.get(column),
   _maxNumValuesMap.getOrDefault(column, 0)));
 }
+
+// Explicitly set it to null in case of reuse
+reuse.putField(NULL_FIELDS, null);
+
+// Look up bitmap for this docId and construct a comma separated
+// null column names based on that bitmap
+RoaringBitmap bitmap = _docIdToNullColumnsMap.get(docId);
 
 Review comment:
   I think if we move to a BitSet based approach (as mentioned above) - it'll 
make this part much simpler. We can simply save the Bitset as is per docId and 
use that to recreate the row. 


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 a change in pull request #4585: Presence vector

2019-09-06 Thread GitBox
icefury71 commented on a change in pull request #4585: Presence vector
URL: https://github.com/apache/incubator-pinot/pull/4585#discussion_r321863828
 
 

 ##
 File path: 
pinot-core/src/main/java/org/apache/pinot/core/indexsegment/mutable/MutableSegmentImpl.java
 ##
 @@ -479,6 +521,20 @@ public GenericRow getRecord(int docId, GenericRow reuse) {
   .getValue(docId, fieldSpec, _indexReaderWriterMap.get(column), 
_dictionaryMap.get(column),
   _maxNumValuesMap.getOrDefault(column, 0)));
 }
+
+// Explicitly set it to null in case of reuse
+reuse.putField(NULL_FIELDS, null);
+
+// Look up bitmap for this docId and construct a comma separated
+// null column names based on that bitmap
+RoaringBitmap bitmap = _docIdToNullColumnsMap.get(docId);
 
 Review comment:
   Yes - currently we need to call isPresent to check which columns have 
corresponding null values for this docId.
   
   Yes there's considerable overhead using a BitMap but not as much as using a 
comma separated string / Set


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 a change in pull request #4585: Presence vector

2019-09-06 Thread GitBox
icefury71 commented on a change in pull request #4585: Presence vector
URL: https://github.com/apache/incubator-pinot/pull/4585#discussion_r321862314
 
 

 ##
 File path: 
pinot-core/src/main/java/org/apache/pinot/core/data/recordtransformer/NullValueTransformer.java
 ##
 @@ -33,17 +36,35 @@ public NullValueTransformer(Schema schema) {
 
   @Override
   public GenericRow transform(GenericRow record) {
+Set nullColumnNamesSet = null;
+
+// Clear out the 'null_fields' value in case the Generic row is reused
+record.putField(NULL_FIELDS, null);
+
 for (FieldSpec fieldSpec : _fieldSpecs) {
   String fieldName = fieldSpec.getName();
   // Do not allow default value for time column
   if (record.getValue(fieldName) == null && fieldSpec.getFieldType() != 
FieldSpec.FieldType.TIME) {
+if (nullColumnNamesSet == null) {
+  nullColumnNamesSet = new HashSet<>();
+}
+
+// Only handle null columns for non-virtual columns
+if (!fieldSpec.isVirtualColumn()) {
+  nullColumnNamesSet.add(fieldName);
+}
+
 if (fieldSpec.isSingleValueField()) {
   record.putField(fieldName, fieldSpec.getDefaultNullValue());
 } else {
   record.putField(fieldName, new 
Object[]{fieldSpec.getDefaultNullValue()});
 }
   }
 }
+
+if (nullColumnNamesSet != null) {
+  record.putField(NULL_FIELDS, String.join(",", nullColumnNamesSet));
 
 Review comment:
   I think the concern is the size of a Java object which is potentially bigger 
in this case. 
   
   One optimization here is that if we have a fixed order of column names (in 
the FieldSpec), we can simply use a BitSet to keep track of null columns (which 
will typically be around 64 bytes ish). This can save a lot of effort in other 
parts of the code base as well.


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 realtimetest updated (104825e -> 968b1eb)

2019-09-06 Thread jenniferdai
This is an automated email from the ASF dual-hosted git repository.

jenniferdai pushed a change to branch realtimetest
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git.


omit 104825e  Refactoring
 add 968b1eb  Refactoring

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   (104825e)
\
 N -- N -- N   refs/heads/realtimetest (968b1eb)

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/core/data/manager/realtime/SegmentCommitter.java  | 13 +
 1 file changed, 5 insertions(+), 8 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 #4591: Deleting preprocess output post segment creation

2019-09-06 Thread GitBox
Jackie-Jiang commented on a change in pull request #4591: Deleting preprocess 
output post segment creation
URL: https://github.com/apache/incubator-pinot/pull/4591#discussion_r321852321
 
 

 ##
 File path: 
pinot-hadoop/src/main/java/org/apache/pinot/hadoop/job/SegmentCreationJob.java
 ##
 @@ -90,6 +91,11 @@ public SegmentCreationJob(Properties properties) {
   _pushLocations = null;
 }
 
+String preprocessOutputPath = 
_properties.getProperty(JobConfigConstants.PREPROCESS_PATH_TO_OUTPUT);
 
 Review comment:
   SegmentCreationJob should have no concept on the pre-process path. Also, 
client might want to keep the pre-process output files, so we should not always 
delete them


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 #4590: Disallowing multiple inputs in preprocess

2019-09-06 Thread GitBox
Jackie-Jiang commented on a change in pull request #4590: Disallowing multiple 
inputs in preprocess
URL: https://github.com/apache/incubator-pinot/pull/4590#discussion_r321851374
 
 

 ##
 File path: 
pinot-hadoop/src/main/java/org/apache/pinot/hadoop/job/SegmentPreprocessingJob.java
 ##
 @@ -101,8 +102,20 @@ public SegmentPreprocessingJob(final Properties 
properties) {
 
 _enablePreprocessing = 
Boolean.parseBoolean(_properties.getProperty(JobConfigConstants.ENABLE_PREPROCESSING));
 
-// get input/output paths.
-_inputSegmentDir = 
Preconditions.checkNotNull(getPathFromProperty(JobConfigConstants.PATH_TO_INPUT));
+String inputPath = 
Preconditions.checkNotNull(properties.getProperty(JobConfigConstants.PATH_TO_INPUT));
+
+// We cannot support this because mapreduce takes complete control of the 
output path. In order to support this, we
+// would need control to pipe the exact folders we receive as input to 
multiple outputs. While we can programmatically
+// determine record by record what goes into each output path, this does 
not support our use case. Each folder is a
+// separate "day," and frequently, our customers will have two dates in 
one file, due to timezone of data, so we
+// are not able to distinguish what is "today's" vs. "tomorrow's" data by 
solely looking at the record.
+if (inputPath.split(",").length > 1) {
+  _isMultipleInput = true;
 
 Review comment:
   Let's directly throw exception here with proper error message


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] jenniferdai opened a new pull request #4592: Refactor Realtime Segment Data Manager to remove repeat logic

2019-09-06 Thread GitBox
jenniferdai opened a new pull request #4592: Refactor Realtime Segment Data 
Manager to remove repeat logic 
URL: https://github.com/apache/incubator-pinot/pull/4592
 
 
   * Allows for tests specifically targeting the controller portion of split 
commit


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 realtimetest updated: Refactoring

2019-09-06 Thread jenniferdai
This is an automated email from the ASF dual-hosted git repository.

jenniferdai pushed a commit to branch realtimetest
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git


The following commit(s) were added to refs/heads/realtimetest by this push:
 new 104825e  Refactoring
104825e is described below

commit 104825edcc15c14330b5f140c64ef04b8af939ed
Author: Jennifer Dai 
AuthorDate: Fri Sep 6 11:08:16 2019 -0700

Refactoring
---
 .../realtime/LLRealtimeSegmentDataManager.java | 17 -
 .../data/manager/realtime/SegmentCommitter.java| 84 --
 2 files changed, 43 insertions(+), 58 deletions(-)

diff --git 
a/pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java
 
b/pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java
index 35beae5..eb0c51b 100644
--- 
a/pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java
+++ 
b/pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java
@@ -768,9 +768,20 @@ public class LLRealtimeSegmentDataManager extends 
RealtimeSegmentDataManager {
 }
 SegmentCompletionProtocol.Response returnedResponse;
 boolean isSplitCommit = response.isSplitCommit() && 
_indexLoadingConfig.isEnableSplitCommit();
-SegmentCommitter segmentCommitter = new SegmentCommitter(isSplitCommit, 
_segmentBuildDescriptor, _segmentNameStr,
-_currentOffset, _numRowsConsumed, _instanceId, _isOffHeap, 
_protocolHandler, _memoryManager, _indexLoadingConfig, response);
-returnedResponse = segmentCommitter.commitSegment();
+
+SegmentCompletionProtocol.Request.Params params = new 
SegmentCompletionProtocol.Request.Params();
+
+
params.withSegmentName(_segmentNameStr).withOffset(_currentOffset).withNumRows(_numRowsConsumed)
+
.withInstanceId(_instanceId).withBuildTimeMillis(_segmentBuildDescriptor.getBuildTimeMillis())
+.withSegmentSizeBytes(_segmentBuildDescriptor.getSegmentSizeBytes())
+.withWaitTimeMillis(_segmentBuildDescriptor.getWaitTimeMillis());
+
+if (_isOffHeap) {
+  params.withMemoryUsedBytes(_memoryManager.getTotalAllocatedBytes());
+}
+
+SegmentCommitter segmentCommitter = new SegmentCommitter(isSplitCommit, 
_segmentNameStr, _protocolHandler, _indexLoadingConfig, response, params);
+returnedResponse = segmentCommitter.commitSegment(_segmentBuildDescriptor, 
_currentOffset, _numRowsConsumed);
 
 if 
(!returnedResponse.getStatus().equals(SegmentCompletionProtocol.ControllerResponseStatus.COMMIT_SUCCESS))
 {
   return false;
diff --git 
a/pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/SegmentCommitter.java
 
b/pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/SegmentCommitter.java
index e0320d2..45d5ca2 100644
--- 
a/pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/SegmentCommitter.java
+++ 
b/pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/SegmentCommitter.java
@@ -15,65 +15,45 @@ import org.slf4j.LoggerFactory;
 public class SegmentCommitter {
 
   private boolean _isSplitCommit;
-  private LLRealtimeSegmentDataManager.SegmentBuildDescriptor 
_segmentBuildDescriptor;
   private String _segmentNameStr;
-  private volatile long _currentOffset;
-  private volatile int _numRowsConsumed;
-  private String _instanceId;
-  private boolean _isOffHeap;
   private ServerSegmentCompletionProtocolHandler _protocolHandler;
-  private PinotDataBufferMemoryManager _memoryManager;
   private IndexLoadingConfig _indexLoadingConfig;
   private SegmentCompletionProtocol.Response _prevResponse;
+  private SegmentCompletionProtocol.Request.Params _params;
 
   private final Logger SEGMENT_LOGGER;
 
-  public SegmentCommitter(boolean isSplitCommit, 
LLRealtimeSegmentDataManager.SegmentBuildDescriptor segmentBuildDescriptor,
-  String segmentNameStr, long currentOffset, int numRowsConsumed, String 
instanceId, boolean isOffHeap,
-  ServerSegmentCompletionProtocolHandler protocolHandler, 
PinotDataBufferMemoryManager memoryManager,
-  IndexLoadingConfig indexLoadingConfig, 
SegmentCompletionProtocol.Response prevResponse) {
+  public SegmentCommitter(boolean isSplitCommit, String segmentNameStr, 
ServerSegmentCompletionProtocolHandler protocolHandler,
+  IndexLoadingConfig indexLoadingConfig, 
SegmentCompletionProtocol.Response prevResponse, 
SegmentCompletionProtocol.Request.Params params) {
 _isSplitCommit = isSplitCommit;
-_segmentBuildDescriptor = segmentBuildDescriptor;
 _segmentNameStr = segmentNameStr;
-_currentOffset = currentOffset;
-_numRowsConsumed = numRowsConsumed;
-_instanceId = instanceId;
-_isOffHeap = isOffHeap;
 _protocolHandler = protocolHandler;
-_memoryManager = memoryManager;
 _indexLoadingConfig = indexLoadingConfig;
 _prevResponse = prevRes

[GitHub] [incubator-pinot] fx19880617 commented on issue #4035: Need a tool to display realtime status of a table on a server

2019-09-06 Thread GitBox
fx19880617 commented on issue #4035: Need a tool to display realtime status of 
a table on a server
URL: 
https://github.com/apache/incubator-pinot/issues/4035#issuecomment-528950923
 
 
   @kishoreg ^^
   
   We may also wanna fetch upstream status, e.g. Kafka topic 
information(#partitions, offsets range, etc.)


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] jenniferdai opened a new pull request #4591: Deleting preprocess output post segment creation

2019-09-06 Thread GitBox
jenniferdai opened a new pull request #4591: Deleting preprocess output post 
segment creation
URL: https://github.com/apache/incubator-pinot/pull/4591
 
 
   * If the preprocessed output dir is defined, we will delete it after segment 
creation succeeds.


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 deletepreprocessoutput created (now 24d551c)

2019-09-06 Thread jenniferdai
This is an automated email from the ASF dual-hosted git repository.

jenniferdai pushed a change to branch deletepreprocessoutput
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git.


  at 24d551c  Deleting preprocess output post segment creation

This branch includes the following new commits:

 new 24d551c  Deleting preprocess output post segment creation

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



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



[incubator-pinot] 01/01: Deleting preprocess output post segment creation

2019-09-06 Thread jenniferdai
This is an automated email from the ASF dual-hosted git repository.

jenniferdai pushed a commit to branch deletepreprocessoutput
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git

commit 24d551c890322ecef1aaa75454ffe903c7a519f4
Author: Jennifer Dai 
AuthorDate: Fri Sep 6 10:49:13 2019 -0700

Deleting preprocess output post segment creation
---
 .../java/org/apache/pinot/hadoop/job/SegmentCreationJob.java   | 10 ++
 1 file changed, 10 insertions(+)

diff --git 
a/pinot-hadoop/src/main/java/org/apache/pinot/hadoop/job/SegmentCreationJob.java
 
b/pinot-hadoop/src/main/java/org/apache/pinot/hadoop/job/SegmentCreationJob.java
index aa68f10..5cba226 100644
--- 
a/pinot-hadoop/src/main/java/org/apache/pinot/hadoop/job/SegmentCreationJob.java
+++ 
b/pinot-hadoop/src/main/java/org/apache/pinot/hadoop/job/SegmentCreationJob.java
@@ -63,6 +63,7 @@ public class SegmentCreationJob extends BaseSegmentJob {
   protected final Path _schemaFile;
   protected final String _defaultPermissionsMask;
   protected final List _pushLocations;
+  protected Path _preprocessOutputPath = null;
 
   protected FileSystem _fileSystem;
 
@@ -90,6 +91,11 @@ public class SegmentCreationJob extends BaseSegmentJob {
   _pushLocations = null;
 }
 
+String preprocessOutputPath = 
_properties.getProperty(JobConfigConstants.PREPROCESS_PATH_TO_OUTPUT);
+if (preprocessOutputPath != null) {
+  _preprocessOutputPath = getPathFromProperty(preprocessOutputPath);
+}
+
 
_logger.info("*");
 _logger.info("Input Pattern: {}", _inputPattern);
 _logger.info("Output Directory: {}", _outputDir);
@@ -192,6 +198,10 @@ public class SegmentCreationJob extends BaseSegmentJob {
 // Delete the staging directory
 _logger.info("Deleting the staging directory: {}", _stagingDir);
 _fileSystem.delete(_stagingDir, true);
+
+if (_preprocessOutputPath != null) {
+  _fileSystem.delete(_preprocessOutputPath, true);
+}
   }
 
   @Nullable


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



[GitHub] [incubator-pinot] chenboat commented on issue #4484: Pinot query timeout due to the broker waiting for a single non-responsive server

2019-09-06 Thread GitBox
chenboat commented on issue #4484: Pinot query timeout due to the broker 
waiting for a single non-responsive server
URL: 
https://github.com/apache/incubator-pinot/issues/4484#issuecomment-528948926
 
 
   > @chenboat It may or may not change the EV. If, as a result of the gc 
activity, zookeeper connection is lost, and all reries fail, then yes, the EV 
will change, and the broker will not route to that server anymore. I don't 
think we are discussing that scenario right? In that case we wont have the 
problem of broker waiting for response from a dead server (we wil have it only 
until EV change is detected by the broker).
   
   Thanks @mcvsubbu for the reply. In fact we were suspecting that exact 
scenario. One of our servers got into full GC and the broker still routed query 
to it. I did not have the EV checked at that time. I did not find a test case 
in pinot-broker to cover such case either. It could be a good test to add if 
there is no such one.
   
   BTW, we have external mechanism to detect GC servers as you suggested. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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



[incubator-pinot] branch paths updated (273658f -> fa31be6)

2019-09-06 Thread jenniferdai
This is an automated email from the ASF dual-hosted git repository.

jenniferdai pushed a change to branch paths
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git.


 discard 273658f  Disallowing multiple inputs in preprocess
 add fa31be6  Disallowing multiple inputs in preprocess

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   (273658f)
\
 N -- N -- N   refs/heads/paths (fa31be6)

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:
 .../org/apache/pinot/hadoop/job/SegmentPreprocessingJob.java   | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)


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



[incubator-pinot] branch paths updated (9a39881 -> 273658f)

2019-09-06 Thread jenniferdai
This is an automated email from the ASF dual-hosted git repository.

jenniferdai pushed a change to branch paths
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git.


 discard 9a39881  Disallowing multiple inputs in preprocess
 add 273658f  Disallowing multiple inputs in preprocess

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   (9a39881)
\
 N -- N -- N   refs/heads/paths (273658f)

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:
 .../main/java/org/apache/pinot/hadoop/job/SegmentPreprocessingJob.java   | 1 +
 1 file changed, 1 insertion(+)


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



[GitHub] [incubator-pinot] jenniferdai opened a new pull request #4590: Disallowing multiple inputs in preprocess

2019-09-06 Thread GitBox
jenniferdai opened a new pull request #4590: Disallowing multiple inputs in 
preprocess
URL: https://github.com/apache/incubator-pinot/pull/4590
 
 
   * We cannot support this in preprocess because mapreduce takes complete 
control of the output path(s). In order to support this, we would need control 
to pipe the exact folders we receive as input to multiple outputs. While we can 
programmatically determine record by record what goes into each output path, 
this does not support our use case. Each folder is a separate "day," and 
frequently, our customers will have two dates in one file, due to timezone of 
data, so we are not able to distinguish what is "today's" vs. "tomorrow's" data 
by solely looking at the record.


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] 01/01: Disallowing multiple inputs in preprocess

2019-09-06 Thread jenniferdai
This is an automated email from the ASF dual-hosted git repository.

jenniferdai pushed a commit to branch paths
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git

commit 9a3988198ab9de3fc1bf01f215a6f321a0475fb6
Author: Jennifer Dai 
AuthorDate: Fri Sep 6 10:42:03 2019 -0700

Disallowing multiple inputs in preprocess
---
 .../pinot/hadoop/job/SegmentPreprocessingJob.java   | 21 +++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git 
a/pinot-hadoop/src/main/java/org/apache/pinot/hadoop/job/SegmentPreprocessingJob.java
 
b/pinot-hadoop/src/main/java/org/apache/pinot/hadoop/job/SegmentPreprocessingJob.java
index 942fb6d..3d07245 100644
--- 
a/pinot-hadoop/src/main/java/org/apache/pinot/hadoop/job/SegmentPreprocessingJob.java
+++ 
b/pinot-hadoop/src/main/java/org/apache/pinot/hadoop/job/SegmentPreprocessingJob.java
@@ -83,6 +83,7 @@ public class SegmentPreprocessingJob extends BaseSegmentJob {
   private String _partitionFunction;
   private String _sortedColumn;
   private int _numOutputFiles;
+  private boolean _isMultipleInput = false;
 
   private final Path _inputSegmentDir;
   private final Path _preprocessedOutputDir;
@@ -101,8 +102,19 @@ public class SegmentPreprocessingJob extends 
BaseSegmentJob {
 
 _enablePreprocessing = 
Boolean.parseBoolean(_properties.getProperty(JobConfigConstants.ENABLE_PREPROCESSING));
 
-// get input/output paths.
-_inputSegmentDir = 
Preconditions.checkNotNull(getPathFromProperty(JobConfigConstants.PATH_TO_INPUT));
+String inputPath = 
Preconditions.checkNotNull(properties.getProperty(JobConfigConstants.PATH_TO_INPUT));
+
+// We cannot support this because mapreduce takes complete control of the 
output path. In order to support this, we
+// would need control to pipe the exact folders we receive as input to 
multiple outputs. While we can programmatically
+// determine record by record what goes into each output path, this does 
not support our use case. Each folder is a
+// separate "day," and frequently, our customers will have two dates in 
one file, due to timezone of data, so we
+// are not able to distinguish what is "today's" vs. "tomorrow's" data by 
solely looking at the record.
+if (inputPath.split(",").length > 1) {
+  _isMultipleInput = true;
+}
+
+// get input path/output paths.
+_inputSegmentDir = getPathFromProperty(JobConfigConstants.PATH_TO_INPUT);
 _preprocessedOutputDir = 
getPathFromProperty(JobConfigConstants.PREPROCESS_PATH_TO_OUTPUT);
 _rawTableName = 
Preconditions.checkNotNull(_properties.getProperty(JobConfigConstants.SEGMENT_TABLE_NAME));
 
@@ -138,6 +150,11 @@ public class SegmentPreprocessingJob extends 
BaseSegmentJob {
   _logger.info("Starting {}", getClass().getSimpleName());
 }
 
+if (_isMultipleInput) {
+  _logger.info("Skipping pre-processing, multiple inputs detected. Not 
supported");
+  return;
+}
+
 _fileSystem = FileSystem.get(_conf);
 final List inputDataPaths = getDataFilePaths(_inputSegmentDir);
 Preconditions.checkState(inputDataPaths.size() != 0, "No files in the 
input directory.");


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



[incubator-pinot] branch paths created (now 9a39881)

2019-09-06 Thread jenniferdai
This is an automated email from the ASF dual-hosted git repository.

jenniferdai pushed a change to branch paths
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git.


  at 9a39881  Disallowing multiple inputs in preprocess

This branch includes the following new commits:

 new 9a39881  Disallowing multiple inputs in preprocess

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



-
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 issue #4589: Benchmarking for Order By implementations

2019-09-06 Thread GitBox
npawar opened a new issue #4589: Benchmarking for Order By implementations
URL: https://github.com/apache/incubator-pinot/issues/4589
 
 
   We have introduced a new Table data structure. The structure has methods 
upsert (inserts and aggregates the Record), merge(Table), iterator. We are 
planning to use it in Order By. Eventually, it should be used by all the 
operators and all stages of the query execution end to end. Imagine having just 
a Table data structure that flows from segment to instance to broker to end 
results, and each operator/stage simply upserts/merges/sorts as needed.
   
   The Table has a SimpleIndexedTable and a ConcurrentIndexedTable. 
   1) Using Simple would mean each segment level operator gets its own Table. 
The tables are merged sequentially in the Combine service. The broker combines 
the tables from each server sequentially.
   2) Using Concurrent would mean, we can pass the same Table instance to all 
the segment operators. They can upsert rows into the same Table. The Combine 
service now becomes a noop.
   3) We can mix and match which Table to use at which stage
   
   The Table has a capacity. When the capacity is reached, a sort and trim 
happens. This will help us achieve on-the-fly trimming at each stage of the 
query. We need to benchmark the performance of this for various workloads and 
various algorithms:
   1. Capacity never reached
   2. Capacity reached
   3. Capacity reached very frequently 
   
   


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 #4484: Pinot query timeout due to the broker waiting for a single non-responsive server

2019-09-06 Thread GitBox
mcvsubbu commented on issue #4484: Pinot query timeout due to the broker 
waiting for a single non-responsive server
URL: 
https://github.com/apache/incubator-pinot/issues/4484#issuecomment-528934199
 
 
   @chenboat  It may or may not change the EV. If, as a result of the gc 
activity, zookeeper connection is lost, and all reries fail, then yes, the EV 
will change, and the broker will not route to that server anymore. I don't 
think we are discussing that scenario right? In that case we wont have the 
problem of broker waiting for response from a dead server (we wil have it only 
until EV change is detected by the broker).


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 (76e0c62 -> 68092ab)

2019-09-06 Thread jenniferdai
This is an automated email from the ASF dual-hosted git repository.

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


from 76e0c62  Adding Predicate supports for Bytes Column without dictionary 
(#4586)
 add 68092ab  Adding integration test for Hadoop pre-process job (#4577)

No new revisions were added by this update.

Summary of changes:
 .../name/NormalizedDateSegmentNameGenerator.java   |  4 -
 .../pinot/hadoop/job/SegmentCreationJob.java   | 17 +---
 .../pinot/hadoop/job/SegmentPreprocessingJob.java  | 14 +---
 .../job/mappers/SegmentPreprocessingMapper.java| 17 ++--
 .../pinot/hadoop/utils/JobPreparationHelper.java   | 11 +++
 .../pinot/integration/tests/ClusterTest.java   |  8 +-
 ...mentBuildPushOfflineClusterIntegrationTest.java | 97 +-
 7 files changed, 86 insertions(+), 82 deletions(-)


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



[incubator-pinot] branch cleanup deleted (was 084edf3)

2019-09-06 Thread jenniferdai
This is an automated email from the ASF dual-hosted git repository.

jenniferdai pushed a change to branch cleanup
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git.


 was 084edf3  Fixing error

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] jenniferdai merged pull request #4577: Adding integration test for Hadoop pre-process job

2019-09-06 Thread GitBox
jenniferdai merged pull request #4577: Adding integration test for Hadoop 
pre-process job
URL: https://github.com/apache/incubator-pinot/pull/4577
 
 
   


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 #4545: Add APIs to get leader for all tables or a given table

2019-09-06 Thread GitBox
jackjlli commented on a change in pull request #4545: Add APIs to get leader 
for all tables or a given table
URL: https://github.com/apache/incubator-pinot/pull/4545#discussion_r321821985
 
 

 ##
 File path: 
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/LeadControllerUtils.java
 ##
 @@ -58,4 +65,15 @@ public static String generatePartitionName(int partitionId) 
{
   public static int extractPartitionId(String partitionName) {
 return 
Integer.parseInt(partitionName.substring(partitionName.lastIndexOf('_') + 1));
   }
+
+  /**
+   * Checks from ZK if resource config of leadControllerResource is enabled.
+   */
+  public static boolean isLeadControllerResourceEnabled(HelixManager 
helixManager) {
+HelixDataAccessor helixDataAccessor = helixManager.getHelixDataAccessor();
+PropertyKey propertyKey = 
helixDataAccessor.keyBuilder().resourceConfig(Helix.LEAD_CONTROLLER_RESOURCE_NAME);
+ResourceConfig resourceConfig = helixDataAccessor.getProperty(propertyKey);
+String enableResource = 
resourceConfig.getSimpleConfig(Helix.LEAD_CONTROLLER_RESOURCE_ENABLED_KEY);
 
 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] jackjlli commented on a change in pull request #4545: Add APIs to get leader for all tables or a given table

2019-09-06 Thread GitBox
jackjlli commented on a change in pull request #4545: Add APIs to get leader 
for all tables or a given table
URL: https://github.com/apache/incubator-pinot/pull/4545#discussion_r321821948
 
 

 ##
 File path: 
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotLeadControllerRestletResource.java
 ##
 @@ -0,0 +1,165 @@
+/**
+ * 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.controller.api.resources;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiParam;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.core.MediaType;
+import org.apache.helix.HelixManager;
+import org.apache.helix.model.ExternalView;
+import org.apache.helix.model.MasterSlaveSMD;
+import org.apache.pinot.common.config.TableNameBuilder;
+import org.apache.pinot.common.utils.CommonConstants.Helix;
+import org.apache.pinot.common.utils.helix.LeadControllerUtils;
+import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+@Api(tags = Constants.LEAD_CONTROLLER_TAG)
+@Path("/leader")
+public class PinotLeadControllerRestletResource {
+  public static Logger LOGGER = 
LoggerFactory.getLogger(PinotLeadControllerRestletResource.class);
+
+  @Inject
+  private PinotHelixResourceManager _pinotHelixResourceManager;
+
+  @GET
+  @Produces(MediaType.APPLICATION_JSON)
+  @Path("/tables")
+  @ApiOperation(value = "Gets leaders for all tables", notes = "Gets leaders 
for all tables")
+  public Map getLeadersForAllTables() {
+Map leadControllerResponses = new 
LinkedHashMap<>();
+HelixManager helixManager = _pinotHelixResourceManager.getHelixZkManager();
+// returns an empty map if lead controller resource is disabled.
+if (!LeadControllerUtils.isLeadControllerResourceEnabled(helixManager)) {
+  return leadControllerResponses;
 
 Review comment:
   Refactored the code to return a boolean flag that shows whether lead 
controller resource is enabled or not. If yes, return partition leader. 
Otherwise, returns helix leader.


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 issue #4545: Add APIs to get leader for all tables or a given table

2019-09-06 Thread GitBox
jackjlli commented on issue #4545: Add APIs to get leader for all tables or a 
given table
URL: https://github.com/apache/incubator-pinot/pull/4545#issuecomment-528929151
 
 
   @mcvsubbu design doc 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