[GitHub] [incubator-pinot] codecov-io edited a comment on issue #4533: [Instance Assignment] De-couple assignment strategy from SegmentAssignment

2019-08-16 Thread GitBox
codecov-io edited a comment on issue #4533: [Instance Assignment] De-couple 
assignment strategy from SegmentAssignment
URL: https://github.com/apache/incubator-pinot/pull/4533#issuecomment-521871617
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4533?src=pr=h1) 
Report
   > Merging 
[#4533](https://codecov.io/gh/apache/incubator-pinot/pull/4533?src=pr=desc) 
into 
[master](https://codecov.io/gh/apache/incubator-pinot/commit/4e41bc36b156b8f1d54619eb79fe161eed2c42c9?src=pr=desc)
 will **decrease** coverage by `13.14%`.
   > The diff coverage is `0%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-pinot/pull/4533/graphs/tree.svg?width=650=4ibza2ugkz=150=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4533?src=pr=tree)
   
   ```diff
   @@  Coverage Diff  @@
   ## master#4533   +/-   ##
   =
   - Coverage  56.5%   43.35%   -13.15% 
   =
 Files  1075 1073-2 
 Lines 5594755917   -30 
 Branches   8168 8182   +14 
   =
   - Hits  3161324245 -7368 
   - Misses2183429519 +7685 
   + Partials   2500 2153  -347
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4533?src=pr=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[...ore/assignment/segment/SegmentAssignmentUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL2Fzc2lnbm1lbnQvc2VnbWVudC9TZWdtZW50QXNzaWdubWVudFV0aWxzLmphdmE=)
 | `0% <ø> (-96.64%)` | `0 <0> (ø)` | |
   | 
[...e/assignment/segment/OfflineSegmentAssignment.java](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL2Fzc2lnbm1lbnQvc2VnbWVudC9PZmZsaW5lU2VnbWVudEFzc2lnbm1lbnQuamF2YQ==)
 | `0% <0%> (ø)` | `0 <0> (?)` | |
   | 
[...e/assignment/segment/SegmentAssignmentFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL2Fzc2lnbm1lbnQvc2VnbWVudC9TZWdtZW50QXNzaWdubWVudEZhY3RvcnkuamF2YQ==)
 | `0% <0%> (ø)` | `0 <0> (?)` | |
   | 
[.../assignment/segment/RealtimeSegmentAssignment.java](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL2Fzc2lnbm1lbnQvc2VnbWVudC9SZWFsdGltZVNlZ21lbnRBc3NpZ25tZW50LmphdmE=)
 | `0% <0%> (ø)` | `0 <0> (?)` | |
   | 
[.../org/apache/pinot/common/http/MultiGetRequest.java](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vaHR0cC9NdWx0aUdldFJlcXVlc3QuamF2YQ==)
 | `0% <0%> (-100%)` | `0% <0%> (ø)` | |
   | 
[...che/pinot/common/restlet/resources/TablesList.java](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVzdGxldC9yZXNvdXJjZXMvVGFibGVzTGlzdC5qYXZh)
 | `0% <0%> (-100%)` | `0% <0%> (ø)` | |
   | 
[...che/pinot/pql/parsers/pql2/ast/OptionsAstNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9wcWwyL2FzdC9PcHRpb25zQXN0Tm9kZS5qYXZh)
 | `0% <0%> (-100%)` | `0% <0%> (ø)` | |
   | 
[...ot/core/query/scheduler/TableBasedGroupMapper.java](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9zY2hlZHVsZXIvVGFibGVCYXNlZEdyb3VwTWFwcGVyLmphdmE=)
 | `0% <0%> (-100%)` | `0% <0%> (ø)` | |
   | 
[...t/core/segment/index/readers/OnHeapDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L2luZGV4L3JlYWRlcnMvT25IZWFwRGljdGlvbmFyeS5qYXZh)
 | `0% <0%> (-100%)` | `0% <0%> (ø)` | |
   | 
[...mmon/config/instance/InstanceAssignmentConfig.java](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL2luc3RhbmNlL0luc3RhbmNlQXNzaWdubWVudENvbmZpZy5qYXZh)
 | `0% <0%> (-100%)` | `0% <0%> (ø)` | |
   | ... and [717 
more](https://codecov.io/gh/apache/incubator-pinot/pull/4533/diff?src=pr=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4533?src=pr=continue).
   > **Legend** - [Click here to learn 

[incubator-pinot] branch segment_assignment updated (387bd4e -> 82372a5)

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

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


 discard 387bd4e  [Instance Assignment] De-couple assignment strategy from 
SegmentAssignment
 add 82372a5  [Instance Assignment] De-couple assignment strategy from 
SegmentAssignment

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   (387bd4e)
\
 N -- N -- N   refs/heads/segment_assignment (82372a5)

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:
 .../helix/core/assignment/segment/OfflineSegmentAssignment.java | 2 +-
 .../helix/core/assignment/segment/RealtimeSegmentAssignment.java| 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)


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



[incubator-pinot] branch segment_assignment updated (a20fa2a -> 387bd4e)

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

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


 discard a20fa2a  [Instance Assignment] De-couple assignment strategy from 
SegmentAssignment
 add 387bd4e  [Instance Assignment] De-couple assignment strategy from 
SegmentAssignment

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   (a20fa2a)
\
 N -- N -- N   refs/heads/segment_assignment (387bd4e)

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:
 .../controller/helix/core/assignment/segment/SegmentAssignment.java | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)


-
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 #4533: [Instance Assignment] De-couple assignment strategy from SegmentAssignment

2019-08-16 Thread GitBox
Jackie-Jiang commented on a change in pull request #4533: [Instance Assignment] 
De-couple assignment strategy from SegmentAssignment
URL: https://github.com/apache/incubator-pinot/pull/4533#discussion_r314918982
 
 

 ##
 File path: 
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/SegmentAssignment.java
 ##
 @@ -23,37 +23,42 @@
 import org.apache.commons.configuration.Configuration;
 import org.apache.helix.HelixManager;
 import org.apache.pinot.common.config.TableConfig;
+import org.apache.pinot.common.utils.InstancePartitionsType;
+import org.apache.pinot.controller.helix.core.assignment.InstancePartitions;
 
 
 /**
- * Strategy to assign segment to instances or rebalance all segments in a 
table.
+ * Interface for segment assignment and table rebalancing.
  */
-public interface SegmentAssignmentStrategy {
+public interface SegmentAssignment {
 
 Review comment:
   Good point, added.


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] snleee commented on a change in pull request #4533: [Instance Assignment] De-couple assignment strategy from SegmentAssignment

2019-08-16 Thread GitBox
snleee commented on a change in pull request #4533: [Instance Assignment] 
De-couple assignment strategy from SegmentAssignment
URL: https://github.com/apache/incubator-pinot/pull/4533#discussion_r314898313
 
 

 ##
 File path: 
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/SegmentAssignment.java
 ##
 @@ -23,37 +23,42 @@
 import org.apache.commons.configuration.Configuration;
 import org.apache.helix.HelixManager;
 import org.apache.pinot.common.config.TableConfig;
+import org.apache.pinot.common.utils.InstancePartitionsType;
+import org.apache.pinot.controller.helix.core.assignment.InstancePartitions;
 
 
 /**
- * Strategy to assign segment to instances or rebalance all segments in a 
table.
+ * Interface for segment assignment and table rebalancing.
  */
-public interface SegmentAssignmentStrategy {
+public interface SegmentAssignment {
 
 Review comment:
   Can you add the `TODO` document for adding support for custom segment 
assignment strategy?
   I agree that it's not the right time to add the interface since we need the 
proper design for it. I can see multiple ways to achieve custom segment 
assignment (e.g. cost model - currently the cost is based on # of segments, we 
can extend this to add resources like storage factor into the cost).


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 #4533: [Instance Assignment] De-couple assignment strategy from SegmentAssignment

2019-08-16 Thread GitBox
Jackie-Jiang commented on a change in pull request #4533: [Instance Assignment] 
De-couple assignment strategy from SegmentAssignment
URL: https://github.com/apache/incubator-pinot/pull/4533#discussion_r314889280
 
 

 ##
 File path: 
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/SegmentAssignment.java
 ##
 @@ -23,37 +23,42 @@
 import org.apache.commons.configuration.Configuration;
 import org.apache.helix.HelixManager;
 import org.apache.pinot.common.config.TableConfig;
+import org.apache.pinot.common.utils.InstancePartitionsType;
+import org.apache.pinot.controller.helix.core.assignment.InstancePartitions;
 
 
 /**
- * Strategy to assign segment to instances or rebalance all segments in a 
table.
+ * Interface for segment assignment and table rebalancing.
 
 Review comment:
   This is not the same interface as the one we mentioned in the documentation, 
but the new added one for the resource assignment feature. We will deprecate 
the old interface once the new feature is done, and I will add documentation 
for the new assignment.


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 #4534: Fixing bytes data type merge logic

2019-08-16 Thread GitBox
codecov-io edited a comment on issue #4534: Fixing bytes data type merge logic
URL: https://github.com/apache/incubator-pinot/pull/4534#issuecomment-521919418
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4534?src=pr=h1) 
Report
   > Merging 
[#4534](https://codecov.io/gh/apache/incubator-pinot/pull/4534?src=pr=desc) 
into 
[master](https://codecov.io/gh/apache/incubator-pinot/commit/4e41bc36b156b8f1d54619eb79fe161eed2c42c9?src=pr=desc)
 will **increase** coverage by `8.3%`.
   > The diff coverage is `75%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-pinot/pull/4534/graphs/tree.svg?width=650=4ibza2ugkz=150=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4534?src=pr=tree)
   
   ```diff
   @@Coverage Diff @@
   ## master   #4534 +/-   ##
   ==
   + Coverage  56.5%   64.8%   +8.3% 
 Complexity   20  20 
   ==
 Files  10751075 
 Lines 55947   55949  +2 
 Branches   81688168 
   ==
   + Hits  31613   36258   +4645 
   + Misses21834   17035   -4799 
   - Partials   25002656+156
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4534?src=pr=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[.../operator/transform/TransformBlockDataFetcher.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vVHJhbnNmb3JtQmxvY2tEYXRhRmV0Y2hlci5qYXZh)
 | `70.16% <ø> (+46.35%)` | `0 <0> (ø)` | :arrow_down: |
   | 
[...t/core/query/selection/SelectionOperatorUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9zZWxlY3Rpb24vU2VsZWN0aW9uT3BlcmF0b3JVdGlscy5qYXZh)
 | `88.05% <75%> (+18.36%)` | `0 <0> (ø)` | :arrow_down: |
   | 
[...pache/pinot/core/util/SortedRangeIntersection.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL1NvcnRlZFJhbmdlSW50ZXJzZWN0aW9uLmphdmE=)
 | `83.82% <0%> (-7.36%)` | `0% <0%> (ø)` | |
   | 
[...nsport/netty/PooledNettyClientResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtdHJhbnNwb3J0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC90cmFuc3BvcnQvbmV0dHkvUG9vbGVkTmV0dHlDbGllbnRSZXNvdXJjZU1hbmFnZXIuamF2YQ==)
 | `85.41% <0%> (-4.17%)` | `0% <0%> (ø)` | |
   | 
[.../org/apache/pinot/transport/netty/NettyServer.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtdHJhbnNwb3J0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC90cmFuc3BvcnQvbmV0dHkvTmV0dHlTZXJ2ZXIuamF2YQ==)
 | `80.8% <0%> (-3.04%)` | `0% <0%> (ø)` | |
   | 
[...n/src/main/java/org/apache/pinot/common/Utils.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vVXRpbHMuamF2YQ==)
 | `59.57% <0%> (-2.13%)` | `0% <0%> (ø)` | |
   | 
[.../impl/dictionary/BaseOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvQmFzZU9mZkhlYXBNdXRhYmxlRGljdGlvbmFyeS5qYXZh)
 | `83.92% <0%> (-0.6%)` | `0% <0%> (ø)` | |
   | 
[...he/pinot/controller/util/SegmentIntervalUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL1NlZ21lbnRJbnRlcnZhbFV0aWxzLmphdmE=)
 | `0% <0%> (ø)` | `0% <0%> (ø)` | :arrow_down: |
   | 
[...ator/transform/function/BaseTransformFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vQmFzZVRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=)
 | `29.95% <0%> (+0.42%)` | `0% <0%> (ø)` | :arrow_down: |
   | 
[...ment/creator/impl/SegmentColumnarIndexCreator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L2NyZWF0b3IvaW1wbC9TZWdtZW50Q29sdW1uYXJJbmRleENyZWF0b3IuamF2YQ==)
 | `77.24% <0%> (+0.59%)` | `0% <0%> (ø)` | :arrow_down: |
   | ... and [289 
more](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4534?src=pr=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = 

[incubator-pinot] branch update_selection_query_1 updated (ecac2e0 -> 905ada5)

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

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


 discard ecac2e0  update column data type cast during inter merge
 add 905ada5  update column data type cast during inter merge

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   (ecac2e0)
\
 N -- N -- N   refs/heads/update_selection_query_1 (905ada5)

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:
 .../query/selection/SelectionOperatorUtils.java|  4 ++
 .../selection/SelectionOperatorServiceTest.java| 46 +++---
 2 files changed, 26 insertions(+), 24 deletions(-)


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



[GitHub] [incubator-pinot] fx19880617 commented on issue #4534: Fixing bytes data type merge logic

2019-08-16 Thread GitBox
fx19880617 commented on issue #4534: Fixing bytes data type merge logic
URL: https://github.com/apache/incubator-pinot/pull/4534#issuecomment-522063623
 
 
   Currently we only convert bytes column with dictionary to hex string. 
However for bytes column without dictionary, the value returned is still 
byte[]. this will cause cast 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] mcvsubbu commented on a change in pull request #4533: [Instance Assignment] De-couple assignment strategy from SegmentAssignment

2019-08-16 Thread GitBox
mcvsubbu commented on a change in pull request #4533: [Instance Assignment] 
De-couple assignment strategy from SegmentAssignment
URL: https://github.com/apache/incubator-pinot/pull/4533#discussion_r314769606
 
 

 ##
 File path: 
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/SegmentAssignment.java
 ##
 @@ -23,37 +23,42 @@
 import org.apache.commons.configuration.Configuration;
 import org.apache.helix.HelixManager;
 import org.apache.pinot.common.config.TableConfig;
+import org.apache.pinot.common.utils.InstancePartitionsType;
+import org.apache.pinot.controller.helix.core.assignment.InstancePartitions;
 
 
 /**
- * Strategy to assign segment to instances or rebalance all segments in a 
table.
+ * Interface for segment assignment and table rebalancing.
 
 Review comment:
   Our documentation states that SegmentAssignmentStrategy is public. You 
should keep the existing interface and introduce a new one to suit the needs.
   
https://pinot.readthedocs.io/en/latest/customizations.html#segment-assignment-strategies
   
   Otherwise it is an incompatible change


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 #4534: Fixing bytes data type merge logic

2019-08-16 Thread GitBox
codecov-io commented on issue #4534: Fixing bytes data type merge logic
URL: https://github.com/apache/incubator-pinot/pull/4534#issuecomment-521919418
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4534?src=pr=h1) 
Report
   > Merging 
[#4534](https://codecov.io/gh/apache/incubator-pinot/pull/4534?src=pr=desc) 
into 
[master](https://codecov.io/gh/apache/incubator-pinot/commit/4e41bc36b156b8f1d54619eb79fe161eed2c42c9?src=pr=desc)
 will **increase** coverage by `8.31%`.
   > The diff coverage is `0%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-pinot/pull/4534/graphs/tree.svg?width=650=4ibza2ugkz=150=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4534?src=pr=tree)
   
   ```diff
   @@ Coverage Diff  @@
   ## master#4534  +/-   ##
   
   + Coverage  56.5%   64.82%   +8.31% 
 Complexity   20   20  
   
 Files  1075 1075  
 Lines 5594755947  
 Branches   8168 8167   -1 
   
   + Hits  3161336265+4652 
   + Misses2183417021-4813 
   - Partials   2500 2661 +161
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4534?src=pr=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[.../operator/transform/TransformBlockDataFetcher.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vVHJhbnNmb3JtQmxvY2tEYXRhRmV0Y2hlci5qYXZh)
 | `70.16% <ø> (+46.35%)` | `0 <0> (ø)` | :arrow_down: |
   | 
[...t/core/query/selection/SelectionOperatorUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9zZWxlY3Rpb24vU2VsZWN0aW9uT3BlcmF0b3JVdGlscy5qYXZh)
 | `86.84% <0%> (+17.14%)` | `0 <0> (ø)` | :arrow_down: |
   | 
[...pache/pinot/core/util/SortedRangeIntersection.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL1NvcnRlZFJhbmdlSW50ZXJzZWN0aW9uLmphdmE=)
 | `83.82% <0%> (-7.36%)` | `0% <0%> (ø)` | |
   | 
[.../org/apache/pinot/transport/netty/NettyServer.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtdHJhbnNwb3J0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC90cmFuc3BvcnQvbmV0dHkvTmV0dHlTZXJ2ZXIuamF2YQ==)
 | `80.8% <0%> (-3.04%)` | `0% <0%> (ø)` | |
   | 
[...n/src/main/java/org/apache/pinot/common/Utils.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vVXRpbHMuamF2YQ==)
 | `59.57% <0%> (-2.13%)` | `0% <0%> (ø)` | |
   | 
[...ator/transform/function/BaseTransformFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vQmFzZVRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=)
 | `29.95% <0%> (+0.42%)` | `0% <0%> (ø)` | :arrow_down: |
   | 
[...ment/creator/impl/SegmentColumnarIndexCreator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L2NyZWF0b3IvaW1wbC9TZWdtZW50Q29sdW1uYXJJbmRleENyZWF0b3IuamF2YQ==)
 | `77.24% <0%> (+0.59%)` | `0% <0%> (ø)` | :arrow_down: |
   | 
[...r/transform/function/ValueInTransformFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vVmFsdWVJblRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=)
 | `39.2% <0%> (+0.8%)` | `0% <0%> (ø)` | :arrow_down: |
   | 
[.../helix/core/realtime/SegmentCompletionManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL1NlZ21lbnRDb21wbGV0aW9uTWFuYWdlci5qYXZh)
 | `70.77% <0%> (+0.86%)` | `0% <0%> (ø)` | :arrow_down: |
   | 
[...e/io/writer/impl/MutableOffHeapByteArrayStore.java](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9pby93cml0ZXIvaW1wbC9NdXRhYmxlT2ZmSGVhcEJ5dGVBcnJheVN0b3JlLmphdmE=)
 | `86.45% <0%> (+1.04%)` | `0% <0%> (ø)` | :arrow_down: |
   | ... and [288 
more](https://codecov.io/gh/apache/incubator-pinot/pull/4534/diff?src=pr=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4534?src=pr=continue).
   > **Legend** - [Click here to 

[GitHub] [incubator-pinot] fx19880617 opened a new pull request #4534: Fixing bytes data type merge logic

2019-08-16 Thread GitBox
fx19880617 opened a new pull request #4534: Fixing bytes data type merge logic
URL: https://github.com/apache/incubator-pinot/pull/4534
 
 
   The old implementation convert bytes to string, for dictionary based column, 
but not for non-dictionary based.
   
   Move this logic to merging part.


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 update_selection_query_1 created (now ecac2e0)

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

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


  at ecac2e0  update column data type cast during inter merge

This branch includes the following new commits:

 new ecac2e0  update column data type cast during inter merge

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: update column data type cast during inter merge

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

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

commit ecac2e0b57e3b583ee7827adc5a432562217a2c6
Author: Xiang Fu 
AuthorDate: Thu Aug 15 22:19:08 2019 -0700

update column data type cast during inter merge
---
 .../pinot/core/operator/transform/TransformBlockDataFetcher.java | 5 +
 .../apache/pinot/core/query/selection/SelectionOperatorUtils.java| 5 -
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git 
a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/TransformBlockDataFetcher.java
 
b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/TransformBlockDataFetcher.java
index 54e83be..b5c8d99 100644
--- 
a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/TransformBlockDataFetcher.java
+++ 
b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/TransformBlockDataFetcher.java
@@ -199,9 +199,6 @@ class DictionaryBasedSVValueFetcher implements Fetcher {
   }
 
   public Serializable getValue(int docId) {
-if (_dataType.equals(FieldSpec.DataType.BYTES)) {
-  return 
BytesUtils.toHexString(_dictionary.getBytesValue(_dictionaryIds[docId]));
-}
 return (Serializable) _dictionary.get(_dictionaryIds[docId]);
   }
 }
@@ -337,4 +334,4 @@ class DictionaryBasedMVBytesValueFetcher implements Fetcher 
{
 }
 return values;
   }
-}
\ No newline at end of file
+}
diff --git 
a/pinot-core/src/main/java/org/apache/pinot/core/query/selection/SelectionOperatorUtils.java
 
b/pinot-core/src/main/java/org/apache/pinot/core/query/selection/SelectionOperatorUtils.java
index bbfa42f..0fe09a5 100644
--- 
a/pinot-core/src/main/java/org/apache/pinot/core/query/selection/SelectionOperatorUtils.java
+++ 
b/pinot-core/src/main/java/org/apache/pinot/core/query/selection/SelectionOperatorUtils.java
@@ -39,6 +39,7 @@ import org.apache.pinot.common.request.Selection;
 import org.apache.pinot.common.request.SelectionSort;
 import org.apache.pinot.common.response.ServerInstance;
 import org.apache.pinot.common.response.broker.SelectionResults;
+import org.apache.pinot.common.utils.BytesUtils;
 import org.apache.pinot.common.utils.DataSchema;
 import org.apache.pinot.common.utils.DataTable;
 import org.apache.pinot.core.common.DataSourceMetadata;
@@ -248,9 +249,11 @@ public class SelectionOperatorUtils {
 dataTableBuilder.setColumn(i, ((Number) 
columnValue).doubleValue());
 break;
   case STRING:
-  case BYTES: // BYTES are already converted to String for Selection, 
before reaching this layer.
 dataTableBuilder.setColumn(i, ((String) columnValue));
 break;
+  case BYTES:
+dataTableBuilder.setColumn(i, BytesUtils.toHexString((byte[]) 
columnValue));
+break;
 
   // Multi-value column.
   case INT_ARRAY:


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