[GitHub] [incubator-pinot] codecov-io commented on issue #4112: When refreshing the segment with identical segment, still update creation time and refresh time in ZK metadata

[GitBox]

codecov-io commented on issue #4112: When refreshing the segment with identical 
segment, still update creation time and refresh time in ZK metadata
URL: https://github.com/apache/incubator-pinot/pull/4112#issuecomment-482764024
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4112?src=pr=h1) 
Report
   > Merging 
[#4112](https://codecov.io/gh/apache/incubator-pinot/pull/4112?src=pr=desc) 
into 
[master](https://codecov.io/gh/apache/incubator-pinot/commit/7ef519d16543ffeac6368b68bddda3c419b14a01?src=pr=desc)
 will **increase** coverage by `0.18%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-pinot/pull/4112/graphs/tree.svg?width=650=4ibza2ugkz=150=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4112?src=pr=tree)
   
   ```diff
   @@ Coverage Diff  @@
   ## master#4112  +/-   ##
   
   + Coverage 66.78%   66.96%   +0.18% 
 Complexity   20   20  
   
 Files  1037 1037  
 Lines 5142551427   +2 
 Branches   7192 7192  
   
   + Hits  3434334440  +97 
   + Misses1472614623 -103 
   - Partials   2356 2364   +8
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4112?src=pr=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[...apache/pinot/controller/api/upload/ZKOperator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4112/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvdXBsb2FkL1pLT3BlcmF0b3IuamF2YQ==)
 | `72.82% <100%> (+3.93%)` | `0 <0> (ø)` | :arrow_down: |
   | 
[...pinot/core/operator/docidsets/OrBlockDocIdSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/4112/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZHNldHMvT3JCbG9ja0RvY0lkU2V0LmphdmE=)
 | `84.9% <0%> (-13.21%)` | `0% <0%> (ø)` | |
   | 
[...ation/function/MinMaxRangeAggregationFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4112/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9NaW5NYXhSYW5nZUFnZ3JlZ2F0aW9uRnVuY3Rpb24uamF2YQ==)
 | `61.62% <0%> (-11.63%)` | `0% <0%> (ø)` | |
   | 
[.../impl/dictionary/LongOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4112/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09mZkhlYXBNdXRhYmxlRGljdGlvbmFyeS5qYXZh)
 | `81.81% <0%> (-10.91%)` | `0% <0%> (ø)` | |
   | 
[...e/impl/dictionary/LongOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4112/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09uSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=)
 | `88.88% <0%> (-6.67%)` | `0% <0%> (ø)` | |
   | 
[...mpl/dictionary/DoubleOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4112/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT2ZmSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=)
 | `72.72% <0%> (-5.46%)` | `0% <0%> (ø)` | |
   | 
[...g/apache/pinot/common/metrics/AbstractMetrics.java](https://codecov.io/gh/apache/incubator-pinot/pull/4112/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9BYnN0cmFjdE1ldHJpY3MuamF2YQ==)
 | `75.33% <0%> (-1.34%)` | `0% <0%> (ø)` | |
   | 
[.../pinot/core/common/datatable/DataTableBuilder.java](https://codecov.io/gh/apache/incubator-pinot/pull/4112/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9jb21tb24vZGF0YXRhYmxlL0RhdGFUYWJsZUJ1aWxkZXIuamF2YQ==)
 | `78.94% <0%> (-1.17%)` | `0% <0%> (ø)` | |
   | 
[...a/org/apache/pinot/core/common/DataBlockCache.java](https://codecov.io/gh/apache/incubator-pinot/pull/4112/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9jb21tb24vRGF0YUJsb2NrQ2FjaGUuamF2YQ==)
 | `79.38% <0%> (-0.77%)` | `0% <0%> (ø)` | |
   | 
[...e/operator/dociditerators/MVScanDocIdIterator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4112/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZGl0ZXJhdG9ycy9NVlNjYW5Eb2NJZEl0ZXJhdG9yLmphdmE=)
 | `63.63% <0%> (ø)` | `0% <0%> (ø)` | :arrow_down: |
   | ... and [20 
more](https://codecov.io/gh/apache/incubator-pinot/pull/4112/diff?src=pr=tree-more)
 | |
   
   --
   
   [Continue to review full report at 

[GitHub] [incubator-pinot] tangdian opened a new pull request #4113: [TE] Fix MySQL and H2 timestamp automatic timezone conversion issues

[GitBox]

tangdian opened a new pull request #4113: [TE] Fix MySQL and H2 timestamp 
automatic timezone conversion issues
URL: https://github.com/apache/incubator-pinot/pull/4113
 
 
   Context: For MySQL and H2, the "timestamp" field will be automatically 
converted to connection timezone (thirdeye backend timezone), which may cause 
inconsistency issue in local and prod. So I am giving user the option to set 
server timezone. 


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 segment_metadata_refresh created (now f398d45)

[jackie]

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

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


  at f398d45  When refreshing the segment with identical segment, still 
update creation time and refresh time in ZK metadata

This branch includes the following new commits:

 new f398d45  When refreshing the segment with identical segment, still 
update creation time and refresh time in ZK metadata

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 #4112: When refreshing the segment with identical segment, still update creation time and refresh time in ZK metadata

[GitBox]

Jackie-Jiang opened a new pull request #4112: When refreshing the segment with 
identical segment, still update creation time and refresh time in ZK metadata
URL: https://github.com/apache/incubator-pinot/pull/4112
 
 
   We check crc to identify whether two segments are identical.
   Segment creation time is not included in crc, so the segment
   creation time might be different. This make sense since we might
   generate segment with same data but at different time, and we don't
   need to refresh the segment. In such case we should still update
   creation time and refresh time.


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: Fix description on Query Latency in the doc (#4111)

[jlli]

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

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


The following commit(s) were added to refs/heads/master by this push:
 new 7ef519d  Fix description on Query Latency in the doc (#4111)
7ef519d is described below

commit 7ef519d16543ffeac6368b68bddda3c419b14a01
Author: Jialiang Li 
AuthorDate: Fri Apr 12 15:22:29 2019 -0700

Fix description on Query Latency in the doc (#4111)
---
 docs/in_production.rst | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/docs/in_production.rst b/docs/in_production.rst
index 9201411..a9779ee 100644
--- a/docs/in_production.rst
+++ b/docs/in_production.rst
@@ -72,15 +72,15 @@ Pinot Server
 
 * Query latency - `TOTAL_QUERY_TIME 
`_
 
-  * The number of exception which might have occurred during query execution
+  * Total time to take from receiving to finishing executing the query.
 
 * Query Execution Exceptions - `QUERY_EXECUTION_EXCEPTIONS 
`_
 
-  * The number of exception which might have occurred during query execution
+  * The number of exception which might have occurred during query execution.
 
 * Realtime Consumption Status - `LLC_PARTITION_CONSUMING 
`_
 
-  * This gives a binary value based on whether low-level consumption is 
healthy (1) or unhealthy (0). It's important to ensure at least a single 
replica of each partition is consuming
+  * This gives a binary value based on whether low-level consumption is 
healthy (1) or unhealthy (0). It's important to ensure at least a single 
replica of each partition is consuming.
 
 * Realtime Highest Offset Consumed - `HIGHEST_STREAM_OFFSET_CONSUMED 
`_
 


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

[GitHub] [incubator-pinot] jackjlli merged pull request #4111: Fix description on Query Latency in the doc

[GitBox]

jackjlli merged pull request #4111: Fix description on Query Latency in the doc
URL: https://github.com/apache/incubator-pinot/pull/4111
 
 
   


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 #4111: Fix description on Query Latency in the doc

[GitBox]

codecov-io commented on issue #4111: Fix description on Query Latency in the doc
URL: https://github.com/apache/incubator-pinot/pull/4111#issuecomment-482736854
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4111?src=pr=h1) 
Report
   > Merging 
[#4111](https://codecov.io/gh/apache/incubator-pinot/pull/4111?src=pr=desc) 
into 
[master](https://codecov.io/gh/apache/incubator-pinot/commit/fd61e745db4e91133c75a24b7c3d7e911f0ab6ac?src=pr=desc)
 will **increase** coverage by `0.18%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-pinot/pull/4111/graphs/tree.svg?width=650=4ibza2ugkz=150=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4111?src=pr=tree)
   
   ```diff
   @@ Coverage Diff  @@
   ## master#4111  +/-   ##
   
   + Coverage 66.66%   66.85%   +0.18% 
 Complexity   20   20  
   
 Files  1037 1037  
 Lines 5142551425  
 Branches   7192 7192  
   
   + Hits  3428234378  +96 
   + Misses1478714676 -111 
   - Partials   2356 2371  +15
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4111?src=pr=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[...pinot/core/operator/docidsets/OrBlockDocIdSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/4111/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZHNldHMvT3JCbG9ja0RvY0lkU2V0LmphdmE=)
 | `84.9% <0%> (-13.21%)` | `0% <0%> (ø)` | |
   | 
[...pache/pinot/core/util/SortedRangeIntersection.java](https://codecov.io/gh/apache/incubator-pinot/pull/4111/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL1NvcnRlZFJhbmdlSW50ZXJzZWN0aW9uLmphdmE=)
 | `83.82% <0%> (-7.36%)` | `0% <0%> (ø)` | |
   | 
[.../impl/dictionary/FloatOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4111/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPbkhlYXBNdXRhYmxlRGljdGlvbmFyeS5qYXZh)
 | `86.66% <0%> (-6.67%)` | `0% <0%> (ø)` | |
   | 
[...e/operator/dociditerators/BitmapDocIdIterator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4111/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZGl0ZXJhdG9ycy9CaXRtYXBEb2NJZEl0ZXJhdG9yLmphdmE=)
 | `60.71% <0%> (-3.58%)` | `0% <0%> (ø)` | |
   | 
[...g/apache/pinot/common/metrics/AbstractMetrics.java](https://codecov.io/gh/apache/incubator-pinot/pull/4111/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9BYnN0cmFjdE1ldHJpY3MuamF2YQ==)
 | `74% <0%> (-2.67%)` | `0% <0%> (ø)` | |
   | 
[...ot/core/query/pruner/ColumnValueSegmentPruner.java](https://codecov.io/gh/apache/incubator-pinot/pull/4111/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9wcnVuZXIvQ29sdW1uVmFsdWVTZWdtZW50UHJ1bmVyLmphdmE=)
 | `85.91% <0%> (-1.41%)` | `0% <0%> (ø)` | |
   | 
[...ache/pinot/common/metadata/ZKMetadataProvider.java](https://codecov.io/gh/apache/incubator-pinot/pull/4111/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0YWRhdGEvWktNZXRhZGF0YVByb3ZpZGVyLmphdmE=)
 | `72.45% <0%> (-1.2%)` | `0% <0%> (ø)` | |
   | 
[...lix/core/realtime/PinotRealtimeSegmentManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4111/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL1Bpbm90UmVhbHRpbWVTZWdtZW50TWFuYWdlci5qYXZh)
 | `73% <0%> (-1%)` | `0% <0%> (ø)` | |
   | 
[...ot/common/protocols/SegmentCompletionProtocol.java](https://codecov.io/gh/apache/incubator-pinot/pull/4111/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcHJvdG9jb2xzL1NlZ21lbnRDb21wbGV0aW9uUHJvdG9jb2wuamF2YQ==)
 | `92.48% <0%> (-0.58%)` | `0% <0%> (ø)` | |
   | 
[...regation/function/customobject/QuantileDigest.java](https://codecov.io/gh/apache/incubator-pinot/pull/4111/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9jdXN0b21vYmplY3QvUXVhbnRpbGVEaWdlc3QuamF2YQ==)
 | `57.74% <0%> (-0.45%)` | `0% <0%> (ø)` | |
   | ... and [20 
more](https://codecov.io/gh/apache/incubator-pinot/pull/4111/diff?src=pr=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4111?src=pr=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = 

[GitHub] [incubator-pinot] codecov-io edited a comment on issue #4103: Add validations to table config before updating a table config

[GitBox]

codecov-io edited a comment on issue #4103: Add validations to table config 
before updating a table config
URL: https://github.com/apache/incubator-pinot/pull/4103#issuecomment-481935319
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4103?src=pr=h1) 
Report
   > Merging 
[#4103](https://codecov.io/gh/apache/incubator-pinot/pull/4103?src=pr=desc) 
into 
[master](https://codecov.io/gh/apache/incubator-pinot/commit/5be8431d6a4984b7efa3a04cd5d9076ed48ea9d8?src=pr=desc)
 will **increase** coverage by `21.03%`.
   > The diff coverage is `93.47%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-pinot/pull/4103/graphs/tree.svg?width=650=4ibza2ugkz=150=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4103?src=pr=tree)
   
   ```diff
   @@  Coverage Diff  @@
   ## master#4103   +/-   ##
   =
   + Coverage 45.74%   66.77%   +21.03% 
   - Complexity0   20   +20 
   =
 Files  1037 1037   
 Lines 5142651434+8 
 Branches   7192 7192   
   =
   + Hits  2352534346+10821 
   + Misses2589114727-11164 
   - Partials   2010 2361  +351
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4103?src=pr=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[...oller/api/resources/PinotTableRestletResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/4103/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFibGVSZXN0bGV0UmVzb3VyY2UuamF2YQ==)
 | `49.05% <100%> (+17.45%)` | `0 <0> (ø)` | :arrow_down: |
   | 
[...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4103/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==)
 | `63.7% <93.33%> (+24.66%)` | `0 <0> (ø)` | :arrow_down: |
   | 
[...he/pinot/core/query/pruner/ValidSegmentPruner.java](https://codecov.io/gh/apache/incubator-pinot/pull/4103/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9wcnVuZXIvVmFsaWRTZWdtZW50UHJ1bmVyLmphdmE=)
 | `57.14% <0%> (-28.58%)` | `0% <0%> (ø)` | |
   | 
[...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4103/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==)
 | `75% <0%> (-25%)` | `0% <0%> (ø)` | |
   | 
[...apache/pinot/common/metrics/ValidationMetrics.java](https://codecov.io/gh/apache/incubator-pinot/pull/4103/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9WYWxpZGF0aW9uTWV0cmljcy5qYXZh)
 | `20.28% <0%> (-23.19%)` | `0% <0%> (ø)` | |
   | 
[...r/validation/RealtimeSegmentValidationManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4103/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci92YWxpZGF0aW9uL1JlYWx0aW1lU2VnbWVudFZhbGlkYXRpb25NYW5hZ2VyLmphdmE=)
 | `58.33% <0%> (-16.67%)` | `0% <0%> (ø)` | |
   | 
[...impl/dictionary/FloatOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4103/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPZmZIZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==)
 | `81.81% <0%> (-10.91%)` | `0% <0%> (ø)` | |
   | 
[...e/impl/dictionary/LongOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4103/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09uSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=)
 | `84.44% <0%> (-8.89%)` | `0% <0%> (ø)` | |
   | 
[...mpl/dictionary/DoubleOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4103/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT2ZmSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=)
 | `65.45% <0%> (-5.46%)` | `0% <0%> (ø)` | |
   | 
[...pinot/core/operator/docidsets/OrBlockDocIdSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/4103/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZHNldHMvT3JCbG9ja0RvY0lkU2V0LmphdmE=)
 | `84.9% <0%> (-3.78%)` | `0% <0%> (ø)` | |
   | ... and [522 
more](https://codecov.io/gh/apache/incubator-pinot/pull/4103/diff?src=pr=tree-more)
 | |
   
   --
   
   [Continue to review full report at 

[GitHub] [incubator-pinot] jackjlli opened a new pull request #4111: Fix description on Query Latency in the doc

[GitBox]

jackjlli opened a new pull request #4111: Fix description on Query Latency in 
the doc
URL: https://github.com/apache/incubator-pinot/pull/4111
 
 
   This PR fixes description on Query Latency in the doc.
   
   @jgutmann 


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] sunithabeeram edited a comment on issue #4087: Upgrade RoaringBitmap to 0.8.0

[GitBox]

sunithabeeram edited a comment on issue #4087: Upgrade RoaringBitmap to 0.8.0
URL: https://github.com/apache/incubator-pinot/pull/4087#issuecomment-482725417
 
 
   I didn't check the backward compatibility part (I am assuming we have 
generated indexes in atleast one of our tests), however, I was recently looking 
into the performance of a query against one of our tables; It had a huge IN 
clause and also had a couple of NOT IN clauses following this format:
   
   SELECT count(col) from table where col1 IN "<100+ values>" and col2 NOT IN 
"<2 values>" and col3 NOT IN "<10+values>" 
   
   (All columns in the filter clause had inverted-indexes).
   
   The base latency of this query was ~450ms. With only updating to 
roaringbitmap 0.8.0, I see that the latency drops to ~70ms. Thats pretty neat.
   
   The mileage across other queries might vary.


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] sunithabeeram commented on issue #4087: Upgrade RoaringBitmap to 0.8.0

[GitBox]

sunithabeeram commented on issue #4087: Upgrade RoaringBitmap to 0.8.0
URL: https://github.com/apache/incubator-pinot/pull/4087#issuecomment-482725417
 
 
   I didn't check the backward compatibility part (I am assuming we have 
generated indexes in atleast one of our tests), however, I was recently looking 
into the performance of a query against one of our tables; It had a huge IN 
clause and also had a couple of NOT IN clauses following this format:
   
   SELECT count(col) from table where col1 IN "<100+ values>" and col2 NOT IN 
"<2 values>" and col3 NOT IN "<10+values>" 
   
   The base latency of this query was ~450ms. With only updating to 
roaringbitmap 0.8.0, I see that the latency drops to ~70ms. Thats pretty neat.
   
   The mileage across other queries might vary.


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


With regards,
Apache Git Services

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

[GitHub] [incubator-pinot] Jackie-Jiang merged pull request #4087: Upgrade RoaringBitmap to 0.8.0

[GitBox]

Jackie-Jiang merged pull request #4087: Upgrade RoaringBitmap to 0.8.0
URL: https://github.com/apache/incubator-pinot/pull/4087
 
 
   


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: upgrade roaringbitmap to 0.8.0 (#4087)

[jackie]

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

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


The following commit(s) were added to refs/heads/master by this push:
 new fd61e74  upgrade roaringbitmap to 0.8.0 (#4087)
fd61e74 is described below

commit fd61e745db4e91133c75a24b7c3d7e911f0ab6ac
Author: Xue Yu <278006...@qq.com>
AuthorDate: Sat Apr 13 05:10:59 2019 +0800

upgrade roaringbitmap to 0.8.0 (#4087)
---
 .../apache/pinot/core/operator/docidsets/BitmapDocIdSet.java |  5 +
 .../pinot/core/common/docidsets/BitmapDocIdSetTest.java  | 12 +---
 pom.xml  |  2 +-
 3 files changed, 7 insertions(+), 12 deletions(-)

diff --git 
a/pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/BitmapDocIdSet.java
 
b/pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/BitmapDocIdSet.java
index ae469ff..b1936a1 100644
--- 
a/pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/BitmapDocIdSet.java
+++ 
b/pinot-core/src/main/java/org/apache/pinot/core/operator/docidsets/BitmapDocIdSet.java
@@ -40,10 +40,7 @@ public class BitmapDocIdSet implements FilterBlockDocIdSet {
   _bitmap = orBitmap;
 } else if (numBitmaps == 1) {
   if (exclusive) {
-// NOTE: cannot use ImmutableRoaringBitmap.flip() because the library 
has a bug in that method
-// TODO: the bug has been fixed in the latest version of 
ImmutableRoaringBitmap, update the version
-MutableRoaringBitmap bitmap = bitmaps[0].toMutableRoaringBitmap();
-bitmap.flip(startDocId, endDocId + 1);
+MutableRoaringBitmap bitmap = ImmutableRoaringBitmap.flip(bitmaps[0], 
startDocId, endDocId + 1);
 _bitmap = bitmap;
   } else {
 _bitmap = bitmaps[0];
diff --git 
a/pinot-core/src/test/java/org/apache/pinot/core/common/docidsets/BitmapDocIdSetTest.java
 
b/pinot-core/src/test/java/org/apache/pinot/core/common/docidsets/BitmapDocIdSetTest.java
index 2926b86..5b8bd5d 100644
--- 
a/pinot-core/src/test/java/org/apache/pinot/core/common/docidsets/BitmapDocIdSetTest.java
+++ 
b/pinot-core/src/test/java/org/apache/pinot/core/common/docidsets/BitmapDocIdSetTest.java
@@ -54,14 +54,12 @@ public class BitmapDocIdSetTest {
 originalSet.add(docId);
 mutableRoaringBitmap.add(docId);
   }
-  ByteArrayOutputStream bos = new ByteArrayOutputStream();
-  DataOutputStream dos = new DataOutputStream(bos);
-  // could call "rr1.runOptimize()" and "rr2.runOptimize" if there
+  // could call "rr1.runOptimize()" and "rr2.runOptimize" if
   // there were runs to compress
-  mutableRoaringBitmap.serialize(dos);
-  dos.close();
-  ByteBuffer bb = ByteBuffer.wrap(bos.toByteArray());
-  ImmutableRoaringBitmap immutableRoaringBitmap = new 
ImmutableRoaringBitmap(bb);
+  final ByteBuffer buffer = 
ByteBuffer.allocate(mutableRoaringBitmap.serializedSizeInBytes());
+  mutableRoaringBitmap.serialize(buffer);
+  buffer.flip();
+  ImmutableRoaringBitmap immutableRoaringBitmap = new 
ImmutableRoaringBitmap(buffer);
   list.add(immutableRoaringBitmap);
 }
 ImmutableRoaringBitmap[] bitmaps = new ImmutableRoaringBitmap[list.size()];
diff --git a/pom.xml b/pom.xml
index 7d4c1ab..52abd08 100644
--- a/pom.xml
+++ b/pom.xml
@@ -327,7 +327,7 @@
   
 org.roaringbitmap
 RoaringBitmap
-0.5.10
+0.8.0
   
   
 com.101tec


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

[incubator-pinot] branch master updated: Fixing PerfBenchmarkDriver (#4109)

[snlee]

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

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


The following commit(s) were added to refs/heads/master by this push:
 new 905688f  Fixing PerfBenchmarkDriver (#4109)
905688f is described below

commit 905688f9214f69770b80b2dbe5b73e8d6b6a0ea7
Author: Seunghyun Lee 
AuthorDate: Fri Apr 12 13:53:40 2019 -0700

Fixing PerfBenchmarkDriver (#4109)

Due to the change in #3864, controller now registers as a helix
participant. This causes the issue with PerfBenchmarkDriver
because it separately create helixZkManager and it tries to
register controller participant with the same host and port.
This pr resolves the issue.
---
 .../pinot/tools/perf/PerfBenchmarkDriver.java  | 47 +++---
 .../pinot/tools/perf/PerfBenchmarkDriverConf.java  | 10 +
 2 files changed, 34 insertions(+), 23 deletions(-)

diff --git 
a/pinot-tools/src/main/java/org/apache/pinot/tools/perf/PerfBenchmarkDriver.java
 
b/pinot-tools/src/main/java/org/apache/pinot/tools/perf/PerfBenchmarkDriver.java
index c8a6005..77e1edc 100644
--- 
a/pinot-tools/src/main/java/org/apache/pinot/tools/perf/PerfBenchmarkDriver.java
+++ 
b/pinot-tools/src/main/java/org/apache/pinot/tools/perf/PerfBenchmarkDriver.java
@@ -71,6 +71,7 @@ public class PerfBenchmarkDriver {
   private final String _segmentFormatVersion;
   private final boolean _verbose;
 
+  private ControllerStarter _controllerStarter;
   private String _controllerHost;
   private int _controllerPort;
   private String _controllerAddress;
@@ -88,6 +89,10 @@ public class PerfBenchmarkDriver {
   private final String _brokerTenantName = "DefaultTenant";
   private final String _serverTenantName = "DefaultTenant";
 
+  // Used for creating tables and tenants, and uploading segments. Since 
uncompressed segments are already available
+  // for PerfBenchmarkDriver, servers can directly load the segments. 
PinotHelixResourceManager.addNewSegment(), which
+  // updates ZKSegmentMetadata only, is not exposed from controller API so we 
need to update segments directly via
+  // PinotHelixResourceManager.
   private PinotHelixResourceManager _helixResourceManager;
 
   public PerfBenchmarkDriver(PerfBenchmarkDriverConf conf) {
@@ -187,7 +192,8 @@ public class PerfBenchmarkDriver {
 }
 ControllerConf conf = getControllerConf();
 LOGGER.info("Starting controller at {}", _controllerAddress);
-new ControllerStarter(conf).start();
+_controllerStarter = new ControllerStarter(conf);
+_controllerStarter.start();
   }
 
   private ControllerConf getControllerConf() {
@@ -205,7 +211,7 @@ public class PerfBenchmarkDriver {
 
   private void startBroker()
   throws Exception {
-if (!_conf.isStartBroker()) {
+if (!_conf.shouldStartBroker()) {
   LOGGER.info("Skipping start broker step. Assumes broker is already 
started.");
   return;
 }
@@ -237,18 +243,31 @@ public class PerfBenchmarkDriver {
 
   private void startHelixResourceManager()
   throws Exception {
-_helixResourceManager = new PinotHelixResourceManager(getControllerConf());
-_helixResourceManager.start();
-
-// Create broker tenant.
-Tenant brokerTenant = new 
TenantBuilder(_brokerTenantName).setRole(TenantRole.BROKER).setTotalInstances(1).build();
-_helixResourceManager.createBrokerTenant(brokerTenant);
-
-// Create server tenant.
-Tenant serverTenant =
-new 
TenantBuilder(_serverTenantName).setRole(TenantRole.SERVER).setTotalInstances(1).setOfflineInstances(1)
-.build();
-_helixResourceManager.createServerTenant(serverTenant);
+if (_conf.shouldStartController()) {
+  // helix resource manager is already available at this time if 
controller is started
+  _helixResourceManager = _controllerStarter.getHelixResourceManager();
+} else {
+  // When starting server only, we need to change the controller port to 
avoid registering controller helix
+  // participant with the same host and port.
+  ControllerConf controllerConf = getControllerConf();
+  
controllerConf.setControllerPort(Integer.toString(_conf.getControllerPort() + 
1));
+  _helixResourceManager = new PinotHelixResourceManager(controllerConf);
+  _helixResourceManager.start();
+}
+
+// Create server tenants if required
+if (_conf.shouldStartServer()) {
+  Tenant serverTenant =
+  new 
TenantBuilder(_serverTenantName).setRole(TenantRole.SERVER).setTotalInstances(1).setOfflineInstances(1)
+  .build();
+  _helixResourceManager.createServerTenant(serverTenant);
+}
+
+// Create broker tenant if required
+if (_conf.shouldStartBroker()) {
+  Tenant brokerTenant = new 
TenantBuilder(_brokerTenantName).setRole(TenantRole.BROKER).setTotalInstances(1).build();
+  _helixResourceManager.createBrokerTenant(brokerTenant);
+}
   }
 
 

[GitHub] [incubator-pinot] codecov-io edited a comment on issue #4109: Fixing PerfBenchmarkDriver

[GitBox]

codecov-io edited a comment on issue #4109: Fixing PerfBenchmarkDriver
URL: https://github.com/apache/incubator-pinot/pull/4109#issuecomment-482649098
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4109?src=pr=h1) 
Report
   > Merging 
[#4109](https://codecov.io/gh/apache/incubator-pinot/pull/4109?src=pr=desc) 
into 
[master](https://codecov.io/gh/apache/incubator-pinot/commit/a5777cd1a108f9f7bf52feedcb6570cdb2ac6f0d?src=pr=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/4109/graphs/tree.svg?width=650=4ibza2ugkz=150=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4109?src=pr=tree)
   
   ```diff
   @@ Coverage Diff  @@
   ## master#4109  +/-   ##
   
   + Coverage 66.71%   66.94%   +0.22% 
 Complexity   20   20  
   
 Files  1037 1037  
 Lines 5142651426  
 Branches   7192 7192  
   
   + Hits  3431034425 +115 
   + Misses1475814625 -133 
   - Partials   2358 2376  +18
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4109?src=pr=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4109/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==)
 | `50% <0%> (-25%)` | `0% <0%> (ø)` | |
   | 
[...mpl/dictionary/DoubleOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4109/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT2ZmSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=)
 | `65.45% <0%> (-7.28%)` | `0% <0%> (ø)` | |
   | 
[...impl/dictionary/DoubleOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4109/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT25IZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==)
 | `68.88% <0%> (-6.67%)` | `0% <0%> (ø)` | |
   | 
[...e/operator/dociditerators/SortedDocIdIterator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4109/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZGl0ZXJhdG9ycy9Tb3J0ZWREb2NJZEl0ZXJhdG9yLmphdmE=)
 | `58.33% <0%> (-5.56%)` | `0% <0%> (ø)` | |
   | 
[.../impl/dictionary/LongOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4109/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09mZkhlYXBNdXRhYmxlRGljdGlvbmFyeS5qYXZh)
 | `87.27% <0%> (-5.46%)` | `0% <0%> (ø)` | |
   | 
[.../controller/helix/core/SegmentDeletionManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4109/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1NlZ21lbnREZWxldGlvbk1hbmFnZXIuamF2YQ==)
 | `76% <0%> (-2.41%)` | `0% <0%> (ø)` | |
   | 
[...pinot/server/starter/helix/HelixServerStarter.java](https://codecov.io/gh/apache/incubator-pinot/pull/4109/diff?src=pr=tree#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9IZWxpeFNlcnZlclN0YXJ0ZXIuamF2YQ==)
 | `52.94% <0%> (-2.21%)` | `0% <0%> (ø)` | |
   | 
[...lix/core/realtime/PinotRealtimeSegmentManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4109/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL1Bpbm90UmVhbHRpbWVTZWdtZW50TWFuYWdlci5qYXZh)
 | `73% <0%> (-2%)` | `0% <0%> (ø)` | |
   | 
[...ata/manager/realtime/RealtimeTableDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4109/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVUYWJsZURhdGFNYW5hZ2VyLmphdmE=)
 | `41.17% <0%> (-1.48%)` | `0% <0%> (ø)` | |
   | 
[...ache/pinot/common/metadata/ZKMetadataProvider.java](https://codecov.io/gh/apache/incubator-pinot/pull/4109/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0YWRhdGEvWktNZXRhZGF0YVByb3ZpZGVyLmphdmE=)
 | `72.45% <0%> (-1.2%)` | `0% <0%> (ø)` | |
   | ... and [20 
more](https://codecov.io/gh/apache/incubator-pinot/pull/4109/diff?src=pr=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4109?src=pr=continue).
   > **Legend** - [Click here 

[GitHub] [incubator-pinot] snleee commented on a change in pull request #4109: Fixing PerfBenchmarkDriver

[GitBox]

snleee commented on a change in pull request #4109: Fixing PerfBenchmarkDriver
URL: https://github.com/apache/incubator-pinot/pull/4109#discussion_r275049103
 
 

 ##
 File path: 
pinot-tools/src/main/java/org/apache/pinot/tools/perf/PerfBenchmarkDriver.java
 ##
 @@ -237,18 +239,31 @@ private void startServer()
 
   private void startHelixResourceManager()
   throws Exception {
-_helixResourceManager = new PinotHelixResourceManager(getControllerConf());
-_helixResourceManager.start();
-
-// Create broker tenant.
-Tenant brokerTenant = new 
TenantBuilder(_brokerTenantName).setRole(TenantRole.BROKER).setTotalInstances(1).build();
-_helixResourceManager.createBrokerTenant(brokerTenant);
-
-// Create server tenant.
-Tenant serverTenant =
-new 
TenantBuilder(_serverTenantName).setRole(TenantRole.SERVER).setTotalInstances(1).setOfflineInstances(1)
-.build();
-_helixResourceManager.createServerTenant(serverTenant);
+if (_conf.shouldStartController()) {
+  // helix resource manager is already available at this time if 
controller is started
+  _helixResourceManager = _controllerStarter.getHelixResourceManager();
+} else {
+  // When starting server only, we need to change the controller port to 
avoid registering controller helix
+  // participant with the same host and port.
+  ControllerConf controllerConf = getControllerConf();
+  
controllerConf.setControllerPort(Integer.toString(_conf.getControllerPort() + 
1));
 
 Review comment:
   I added some comments. Can you check again?


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 #4109: Fixing PerfBenchmarkDriver

[GitBox]

jackjlli commented on a change in pull request #4109: Fixing PerfBenchmarkDriver
URL: https://github.com/apache/incubator-pinot/pull/4109#discussion_r275038898
 
 

 ##
 File path: 
pinot-tools/src/main/java/org/apache/pinot/tools/perf/PerfBenchmarkDriver.java
 ##
 @@ -237,18 +239,31 @@ private void startServer()
 
   private void startHelixResourceManager()
   throws Exception {
-_helixResourceManager = new PinotHelixResourceManager(getControllerConf());
-_helixResourceManager.start();
-
-// Create broker tenant.
-Tenant brokerTenant = new 
TenantBuilder(_brokerTenantName).setRole(TenantRole.BROKER).setTotalInstances(1).build();
-_helixResourceManager.createBrokerTenant(brokerTenant);
-
-// Create server tenant.
-Tenant serverTenant =
-new 
TenantBuilder(_serverTenantName).setRole(TenantRole.SERVER).setTotalInstances(1).setOfflineInstances(1)
-.build();
-_helixResourceManager.createServerTenant(serverTenant);
+if (_conf.shouldStartController()) {
+  // helix resource manager is already available at this time if 
controller is started
+  _helixResourceManager = _controllerStarter.getHelixResourceManager();
+} else {
+  // When starting server only, we need to change the controller port to 
avoid registering controller helix
+  // participant with the same host and port.
+  ControllerConf controllerConf = getControllerConf();
+  
controllerConf.setControllerPort(Integer.toString(_conf.getControllerPort() + 
1));
 
 Review comment:
   I see. Thanks for the explanation! Can you add some comments on 
`_helixResourceManager` to explain a bit why it's needed here? I'm sure we're 
not the last one who had question on it.


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


With regards,
Apache Git Services

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

[GitHub] [incubator-pinot] codecov-io commented on issue #4110: Default PinotAdmin Commands to not use System.exit(...).

[GitBox]

codecov-io commented on issue #4110: Default PinotAdmin Commands to not use 
System.exit(...).
URL: https://github.com/apache/incubator-pinot/pull/4110#issuecomment-482678182
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4110?src=pr=h1) 
Report
   > Merging 
[#4110](https://codecov.io/gh/apache/incubator-pinot/pull/4110?src=pr=desc) 
into 
[master](https://codecov.io/gh/apache/incubator-pinot/commit/26330f3a2c3309e7cf574e1fff86a1de9fb934ff?src=pr=desc)
 will **increase** coverage by `20.91%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-pinot/pull/4110/graphs/tree.svg?width=650=4ibza2ugkz=150=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4110?src=pr=tree)
   
   ```diff
   @@  Coverage Diff  @@
   ## master#4110   +/-   ##
   =
   + Coverage 45.74%   66.66%   +20.91% 
   - Complexity0   20   +20 
   =
 Files  1037 1037   
 Lines 5142651426   
 Branches   7192 7192   
   =
   + Hits  2352534282+10757 
   + Misses2589114780-1 
   - Partials   2010 2364  +354
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4110?src=pr=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[...er/validation/BrokerResourceValidationManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4110/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci92YWxpZGF0aW9uL0Jyb2tlclJlc291cmNlVmFsaWRhdGlvbk1hbmFnZXIuamF2YQ==)
 | `25% <0%> (-56.25%)` | `0% <0%> (ø)` | |
   | 
[...he/pinot/core/query/pruner/ValidSegmentPruner.java](https://codecov.io/gh/apache/incubator-pinot/pull/4110/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9wcnVuZXIvVmFsaWRTZWdtZW50UHJ1bmVyLmphdmE=)
 | `57.14% <0%> (-28.58%)` | `0% <0%> (ø)` | |
   | 
[...apache/pinot/common/metrics/ValidationMetrics.java](https://codecov.io/gh/apache/incubator-pinot/pull/4110/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9WYWxpZGF0aW9uTWV0cmljcy5qYXZh)
 | `20.28% <0%> (-23.19%)` | `0% <0%> (ø)` | |
   | 
[...r/validation/RealtimeSegmentValidationManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4110/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci92YWxpZGF0aW9uL1JlYWx0aW1lU2VnbWVudFZhbGlkYXRpb25NYW5hZ2VyLmphdmE=)
 | `55% <0%> (-20%)` | `0% <0%> (ø)` | |
   | 
[...e/impl/dictionary/LongOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4110/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09uSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=)
 | `82.22% <0%> (-11.12%)` | `0% <0%> (ø)` | |
   | 
[...impl/dictionary/FloatOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4110/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPZmZIZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==)
 | `83.63% <0%> (-9.1%)` | `0% <0%> (ø)` | |
   | 
[.../impl/dictionary/FloatOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4110/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPbkhlYXBNdXRhYmxlRGljdGlvbmFyeS5qYXZh)
 | `86.66% <0%> (-6.67%)` | `0% <0%> (ø)` | |
   | 
[...mpl/dictionary/DoubleOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4110/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT2ZmSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=)
 | `65.45% <0%> (-5.46%)` | `0% <0%> (ø)` | |
   | 
[...not/broker/broker/helix/ClusterChangeMediator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4110/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0NsdXN0ZXJDaGFuZ2VNZWRpYXRvci5qYXZh)
 | `66.66% <0%> (-3.45%)` | `0% <0%> (ø)` | |
   | 
[...e/operator/dociditerators/MVScanDocIdIterator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4110/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZGl0ZXJhdG9ycy9NVlNjYW5Eb2NJZEl0ZXJhdG9yLmphdmE=)
 | `60.6% <0%> (-3.04%)` | `0% <0%> (ø)` | |
   | ... and [519 
more](https://codecov.io/gh/apache/incubator-pinot/pull/4110/diff?src=pr=tree-more)
 | |
   
   --
   
   [Continue to review full report at 

[GitHub] [incubator-pinot] snleee commented on a change in pull request #4109: Fixing PerfBenchmarkDriver

[GitBox]

snleee commented on a change in pull request #4109: Fixing PerfBenchmarkDriver
URL: https://github.com/apache/incubator-pinot/pull/4109#discussion_r275016684
 
 

 ##
 File path: 
pinot-tools/src/main/java/org/apache/pinot/tools/perf/PerfBenchmarkDriver.java
 ##
 @@ -237,18 +239,31 @@ private void startServer()
 
   private void startHelixResourceManager()
   throws Exception {
-_helixResourceManager = new PinotHelixResourceManager(getControllerConf());
-_helixResourceManager.start();
-
-// Create broker tenant.
-Tenant brokerTenant = new 
TenantBuilder(_brokerTenantName).setRole(TenantRole.BROKER).setTotalInstances(1).build();
-_helixResourceManager.createBrokerTenant(brokerTenant);
-
-// Create server tenant.
-Tenant serverTenant =
-new 
TenantBuilder(_serverTenantName).setRole(TenantRole.SERVER).setTotalInstances(1).setOfflineInstances(1)
-.build();
-_helixResourceManager.createServerTenant(serverTenant);
+if (_conf.shouldStartController()) {
+  // helix resource manager is already available at this time if 
controller is started
+  _helixResourceManager = _controllerStarter.getHelixResourceManager();
+} else {
+  // When starting server only, we need to change the controller port to 
avoid registering controller helix
+  // participant with the same host and port.
+  ControllerConf controllerConf = getControllerConf();
+  
controllerConf.setControllerPort(Integer.toString(_conf.getControllerPort() + 
1));
 
 Review comment:
   Good point. I already attempted that approach but we have some limits. We 
can easily change it for tenants and table creation. However, one issue is that 
we are taking different code path for segment upload. `addNewSegment()` is used 
for tests mostly and the function only updates `segmentZkMetadata`. We use this 
approach because extracted segment files are already available when starting a 
server in a perfBenchmarkRunner. If we want to use the controller API, we will 
have to tar segment files and upload to controller.


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 #4109: Fixing PerfBenchmarkDriver

[GitBox]

snleee commented on a change in pull request #4109: Fixing PerfBenchmarkDriver
URL: https://github.com/apache/incubator-pinot/pull/4109#discussion_r275016684
 
 

 ##
 File path: 
pinot-tools/src/main/java/org/apache/pinot/tools/perf/PerfBenchmarkDriver.java
 ##
 @@ -237,18 +239,31 @@ private void startServer()
 
   private void startHelixResourceManager()
   throws Exception {
-_helixResourceManager = new PinotHelixResourceManager(getControllerConf());
-_helixResourceManager.start();
-
-// Create broker tenant.
-Tenant brokerTenant = new 
TenantBuilder(_brokerTenantName).setRole(TenantRole.BROKER).setTotalInstances(1).build();
-_helixResourceManager.createBrokerTenant(brokerTenant);
-
-// Create server tenant.
-Tenant serverTenant =
-new 
TenantBuilder(_serverTenantName).setRole(TenantRole.SERVER).setTotalInstances(1).setOfflineInstances(1)
-.build();
-_helixResourceManager.createServerTenant(serverTenant);
+if (_conf.shouldStartController()) {
+  // helix resource manager is already available at this time if 
controller is started
+  _helixResourceManager = _controllerStarter.getHelixResourceManager();
+} else {
+  // When starting server only, we need to change the controller port to 
avoid registering controller helix
+  // participant with the same host and port.
+  ControllerConf controllerConf = getControllerConf();
+  
controllerConf.setControllerPort(Integer.toString(_conf.getControllerPort() + 
1));
 
 Review comment:
   Good point. I already attempted that approach but we have some limits. We 
can easily change it for tenants and table creation. However, one issue is that 
we are taking different code path for segment upload. `addNewSegment()` is used 
for tests mostly and the function only updates `segmentZkMetadata`. We use this 
approach because extracted segment files are already available when starting a 
server in a perfBenchmarkRunner. If we want to use the controller API, we will 
have to tar segment files and upload to controller, which is not wanted in this 
case.


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 #4109: Fixing PerfBenchmarkDriver

[GitBox]

snleee commented on a change in pull request #4109: Fixing PerfBenchmarkDriver
URL: https://github.com/apache/incubator-pinot/pull/4109#discussion_r275016684
 
 

 ##
 File path: 
pinot-tools/src/main/java/org/apache/pinot/tools/perf/PerfBenchmarkDriver.java
 ##
 @@ -237,18 +239,31 @@ private void startServer()
 
   private void startHelixResourceManager()
   throws Exception {
-_helixResourceManager = new PinotHelixResourceManager(getControllerConf());
-_helixResourceManager.start();
-
-// Create broker tenant.
-Tenant brokerTenant = new 
TenantBuilder(_brokerTenantName).setRole(TenantRole.BROKER).setTotalInstances(1).build();
-_helixResourceManager.createBrokerTenant(brokerTenant);
-
-// Create server tenant.
-Tenant serverTenant =
-new 
TenantBuilder(_serverTenantName).setRole(TenantRole.SERVER).setTotalInstances(1).setOfflineInstances(1)
-.build();
-_helixResourceManager.createServerTenant(serverTenant);
+if (_conf.shouldStartController()) {
+  // helix resource manager is already available at this time if 
controller is started
+  _helixResourceManager = _controllerStarter.getHelixResourceManager();
+} else {
+  // When starting server only, we need to change the controller port to 
avoid registering controller helix
+  // participant with the same host and port.
+  ControllerConf controllerConf = getControllerConf();
+  
controllerConf.setControllerPort(Integer.toString(_conf.getControllerPort() + 
1));
 
 Review comment:
   Good point. I already attempted that approach but we have some limits. We 
can easily change it for tenants and table creation. However, one issue is that 
we are taking different code path for segment upload. `addNewSegment()` is used 
for mostly tests and the function only updates `segmentZkMetadata`. We use this 
approach because extracted segment files are already available when starting a 
server in a perfBenchmarkRunner. If we want to use the controller API, we will 
have to tar segment files and upload to controller.


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 #4109: Fixing PerfBenchmarkDriver

[GitBox]

snleee commented on a change in pull request #4109: Fixing PerfBenchmarkDriver
URL: https://github.com/apache/incubator-pinot/pull/4109#discussion_r275016684
 
 

 ##
 File path: 
pinot-tools/src/main/java/org/apache/pinot/tools/perf/PerfBenchmarkDriver.java
 ##
 @@ -237,18 +239,31 @@ private void startServer()
 
   private void startHelixResourceManager()
   throws Exception {
-_helixResourceManager = new PinotHelixResourceManager(getControllerConf());
-_helixResourceManager.start();
-
-// Create broker tenant.
-Tenant brokerTenant = new 
TenantBuilder(_brokerTenantName).setRole(TenantRole.BROKER).setTotalInstances(1).build();
-_helixResourceManager.createBrokerTenant(brokerTenant);
-
-// Create server tenant.
-Tenant serverTenant =
-new 
TenantBuilder(_serverTenantName).setRole(TenantRole.SERVER).setTotalInstances(1).setOfflineInstances(1)
-.build();
-_helixResourceManager.createServerTenant(serverTenant);
+if (_conf.shouldStartController()) {
+  // helix resource manager is already available at this time if 
controller is started
+  _helixResourceManager = _controllerStarter.getHelixResourceManager();
+} else {
+  // When starting server only, we need to change the controller port to 
avoid registering controller helix
+  // participant with the same host and port.
+  ControllerConf controllerConf = getControllerConf();
+  
controllerConf.setControllerPort(Integer.toString(_conf.getControllerPort() + 
1));
 
 Review comment:
   Good point. I already attempted that approach but we have some limits. We 
can easily change it for tenants and table creation. However, one issue is that 
we are taking a different code path for segment upload. `addNewSegment()` is 
used for mostly tests and the function only updates `segmentZkMetadata`. We use 
this approach because extracted segment files are already available when 
starting a server in a perfBenchmarkRunner. If we want to use the controller 
API, we will have to tar segment files and upload to controller.


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 #4109: Fixing PerfBenchmarkDriver

[GitBox]

snleee commented on a change in pull request #4109: Fixing PerfBenchmarkDriver
URL: https://github.com/apache/incubator-pinot/pull/4109#discussion_r275016684
 
 

 ##
 File path: 
pinot-tools/src/main/java/org/apache/pinot/tools/perf/PerfBenchmarkDriver.java
 ##
 @@ -237,18 +239,31 @@ private void startServer()
 
   private void startHelixResourceManager()
   throws Exception {
-_helixResourceManager = new PinotHelixResourceManager(getControllerConf());
-_helixResourceManager.start();
-
-// Create broker tenant.
-Tenant brokerTenant = new 
TenantBuilder(_brokerTenantName).setRole(TenantRole.BROKER).setTotalInstances(1).build();
-_helixResourceManager.createBrokerTenant(brokerTenant);
-
-// Create server tenant.
-Tenant serverTenant =
-new 
TenantBuilder(_serverTenantName).setRole(TenantRole.SERVER).setTotalInstances(1).setOfflineInstances(1)
-.build();
-_helixResourceManager.createServerTenant(serverTenant);
+if (_conf.shouldStartController()) {
+  // helix resource manager is already available at this time if 
controller is started
+  _helixResourceManager = _controllerStarter.getHelixResourceManager();
+} else {
+  // When starting server only, we need to change the controller port to 
avoid registering controller helix
+  // participant with the same host and port.
+  ControllerConf controllerConf = getControllerConf();
+  
controllerConf.setControllerPort(Integer.toString(_conf.getControllerPort() + 
1));
 
 Review comment:
   Good point. I already attempted that approach but we have some limits. We 
can easily change it for tenants and table creation. However, one issue is that 
we are taking different code path for segment upload. `addNewSegment()` is used 
for mostly tests and the function only updates `segmentZkMetadata`. We use this 
approach because extracted segment files are already available when starting a 
server in a perfBenchmarkRunner. If we want to use the controller API, we will 
have to tar segment files and upload to controller.


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 closed issue #4107: quick-start-hybrid.sh fails due to 400 error on tenant creation

[GitBox]

snleee closed issue #4107: quick-start-hybrid.sh fails due to 400 error on 
tenant creation
URL: https://github.com/apache/incubator-pinot/issues/4107
 
 
   


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: Fix AddTenantCommand bug for posting tenant config (#4108)

[snlee]

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

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


The following commit(s) were added to refs/heads/master by this push:
 new a5777cd  Fix AddTenantCommand bug for posting tenant config (#4108)
a5777cd is described below

commit a5777cd1a108f9f7bf52feedcb6570cdb2ac6f0d
Author: Seunghyun Lee 
AuthorDate: Fri Apr 12 11:17:29 2019 -0700

Fix AddTenantCommand bug for posting tenant config (#4108)
---
 .../java/org/apache/pinot/tools/admin/command/AddTenantCommand.java   | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git 
a/pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/AddTenantCommand.java
 
b/pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/AddTenantCommand.java
index 0c32f9c..ba1232e 100644
--- 
a/pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/AddTenantCommand.java
+++ 
b/pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/AddTenantCommand.java
@@ -19,6 +19,7 @@
 package org.apache.pinot.tools.admin.command;
 
 import org.apache.pinot.common.config.Tenant;
+import org.apache.pinot.common.utils.JsonUtils;
 import org.apache.pinot.common.utils.NetUtil;
 import org.apache.pinot.common.utils.TenantRole;
 import org.apache.pinot.controller.helix.ControllerRequestURLBuilder;
@@ -116,7 +117,8 @@ public class AddTenantCommand extends 
AbstractBaseAdminCommand implements Comman
 
.setOfflineInstances(_offlineInstanceCount).setRealtimeInstances(_realtimeInstanceCount).build();
 
 String res = AbstractBaseAdminCommand
-
.sendPostRequest(ControllerRequestURLBuilder.baseUrl(_controllerAddress).forTenantCreate(),
 t.toString());
+
.sendPostRequest(ControllerRequestURLBuilder.baseUrl(_controllerAddress).forTenantCreate(),
+JsonUtils.objectToString(t));
 
 LOGGER.info(res);
 System.out.print(res);


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

[GitHub] [incubator-pinot] snleee merged pull request #4108: Fix AddTenantCommand bug for posting tenant config

[GitBox]

snleee merged pull request #4108: Fix AddTenantCommand bug for posting tenant 
config
URL: https://github.com/apache/incubator-pinot/pull/4108
 
 
   


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 fixing_admin_cmd_exit_code created (now ec3e820)

[xiangfu]

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

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


  at ec3e820  Default PinotAdmin Commands to not use System.exit(...).

This branch includes the following new commits:

 new ec3e820  Default PinotAdmin Commands to not use System.exit(...).

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: Default PinotAdmin Commands to not use System.exit(...).

[xiangfu]

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

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

commit ec3e8201157e4dcd44ce154d32a35097c90cd32a
Author: Xiang Fu 
AuthorDate: Fri Apr 12 10:43:06 2019 -0700

Default PinotAdmin Commands to not use System.exit(...).
---
 .../src/main/java/org/apache/pinot/tools/admin/PinotAdministrator.java  | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/pinot-tools/src/main/java/org/apache/pinot/tools/admin/PinotAdministrator.java
 
b/pinot-tools/src/main/java/org/apache/pinot/tools/admin/PinotAdministrator.java
index f490e2c..df4ab08 100644
--- 
a/pinot-tools/src/main/java/org/apache/pinot/tools/admin/PinotAdministrator.java
+++ 
b/pinot-tools/src/main/java/org/apache/pinot/tools/admin/PinotAdministrator.java
@@ -104,7 +104,7 @@ public class PinotAdministrator {
   public static void main(String[] args) {
 PinotAdministrator pinotAdministrator = new PinotAdministrator();
 pinotAdministrator.execute(args);
-if (!System.getProperties().getProperty("pinot.admin.system.exit", 
"true").equalsIgnoreCase("false")) {
+if (System.getProperties().getProperty("pinot.admin.system.exit", 
"false").equalsIgnoreCase("true")) {
   System.exit(pinotAdministrator.getStatus() ? 0 : 1);
 }
   }


-
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 #4109: Fixing PerfBenchmarkDriver

[GitBox]

codecov-io commented on issue #4109: Fixing PerfBenchmarkDriver
URL: https://github.com/apache/incubator-pinot/pull/4109#issuecomment-482649098
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4109?src=pr=h1) 
Report
   > Merging 
[#4109](https://codecov.io/gh/apache/incubator-pinot/pull/4109?src=pr=desc) 
into 
[master](https://codecov.io/gh/apache/incubator-pinot/commit/26330f3a2c3309e7cf574e1fff86a1de9fb934ff?src=pr=desc)
 will **increase** coverage by `21.1%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-pinot/pull/4109/graphs/tree.svg?width=650=4ibza2ugkz=150=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4109?src=pr=tree)
   
   ```diff
   @@ Coverage Diff  @@
   ## master#4109  +/-   ##
   
   + Coverage 45.74%   66.85%   +21.1% 
   - Complexity0   20  +20 
   
 Files  1037 1037  
 Lines 5142651426  
 Branches   7192 7192  
   
   + Hits  2352534379   +10854 
   + Misses2589114683   -11208 
   - Partials   2010 2364 +354
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4109?src=pr=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[...er/validation/BrokerResourceValidationManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4109/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci92YWxpZGF0aW9uL0Jyb2tlclJlc291cmNlVmFsaWRhdGlvbk1hbmFnZXIuamF2YQ==)
 | `25% <0%> (-56.25%)` | `0% <0%> (ø)` | |
   | 
[...he/pinot/core/query/pruner/ValidSegmentPruner.java](https://codecov.io/gh/apache/incubator-pinot/pull/4109/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9wcnVuZXIvVmFsaWRTZWdtZW50UHJ1bmVyLmphdmE=)
 | `57.14% <0%> (-28.58%)` | `0% <0%> (ø)` | |
   | 
[...egation/function/customobject/MinMaxRangePair.java](https://codecov.io/gh/apache/incubator-pinot/pull/4109/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9jdXN0b21vYmplY3QvTWluTWF4UmFuZ2VQYWlyLmphdmE=)
 | `75.86% <0%> (-24.14%)` | `0% <0%> (ø)` | |
   | 
[...r/validation/RealtimeSegmentValidationManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4109/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci92YWxpZGF0aW9uL1JlYWx0aW1lU2VnbWVudFZhbGlkYXRpb25NYW5hZ2VyLmphdmE=)
 | `55% <0%> (-20%)` | `0% <0%> (ø)` | |
   | 
[.../impl/dictionary/FloatOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4109/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPbkhlYXBNdXRhYmxlRGljdGlvbmFyeS5qYXZh)
 | `82.22% <0%> (-11.12%)` | `0% <0%> (ø)` | |
   | 
[...e/impl/dictionary/LongOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4109/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09uSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=)
 | `82.22% <0%> (-11.12%)` | `0% <0%> (ø)` | |
   | 
[...impl/dictionary/FloatOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4109/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPZmZIZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==)
 | `87.27% <0%> (-5.46%)` | `0% <0%> (ø)` | |
   | 
[...pinot/core/operator/docidsets/OrBlockDocIdSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/4109/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZHNldHMvT3JCbG9ja0RvY0lkU2V0LmphdmE=)
 | `84.9% <0%> (-3.78%)` | `0% <0%> (ø)` | |
   | 
[...not/broker/broker/helix/ClusterChangeMediator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4109/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0NsdXN0ZXJDaGFuZ2VNZWRpYXRvci5qYXZh)
 | `66.66% <0%> (-3.45%)` | `0% <0%> (ø)` | |
   | 
[...e/operator/dociditerators/MVScanDocIdIterator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4109/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZGl0ZXJhdG9ycy9NVlNjYW5Eb2NJZEl0ZXJhdG9yLmphdmE=)
 | `60.6% <0%> (-3.04%)` | `0% <0%> (ø)` | |
   | ... and [521 
more](https://codecov.io/gh/apache/incubator-pinot/pull/4109/diff?src=pr=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4109?src=pr=continue).
   > **Legend** - [Click here 

[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #4109: Fixing PerfBenchmarkDriver

[GitBox]

jackjlli commented on a change in pull request #4109: Fixing PerfBenchmarkDriver
URL: https://github.com/apache/incubator-pinot/pull/4109#discussion_r274981827
 
 

 ##
 File path: 
pinot-tools/src/main/java/org/apache/pinot/tools/perf/PerfBenchmarkDriver.java
 ##
 @@ -237,18 +239,31 @@ private void startServer()
 
   private void startHelixResourceManager()
   throws Exception {
-_helixResourceManager = new PinotHelixResourceManager(getControllerConf());
-_helixResourceManager.start();
-
-// Create broker tenant.
-Tenant brokerTenant = new 
TenantBuilder(_brokerTenantName).setRole(TenantRole.BROKER).setTotalInstances(1).build();
-_helixResourceManager.createBrokerTenant(brokerTenant);
-
-// Create server tenant.
-Tenant serverTenant =
-new 
TenantBuilder(_serverTenantName).setRole(TenantRole.SERVER).setTotalInstances(1).setOfflineInstances(1)
-.build();
-_helixResourceManager.createServerTenant(serverTenant);
+if (_conf.shouldStartController()) {
+  // helix resource manager is already available at this time if 
controller is started
+  _helixResourceManager = _controllerStarter.getHelixResourceManager();
+} else {
+  // When starting server only, we need to change the controller port to 
avoid registering controller helix
+  // participant with the same host and port.
+  ControllerConf controllerConf = getControllerConf();
+  
controllerConf.setControllerPort(Integer.toString(_conf.getControllerPort() + 
1));
 
 Review comment:
   In my opinion `_helixResourceManager` shouldn't even be instantiated here. 
Looking through the code, it only does the following steps:
   * create tenants
   * add table
   * add segment
   
   All these behaviors should be done by sending requests to controller. 


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 opened a new pull request #4109: Fixing PerfBenchmarkDriver

[GitBox]

snleee opened a new pull request #4109: Fixing PerfBenchmarkDriver
URL: https://github.com/apache/incubator-pinot/pull/4109
 
 
   Due to the change in #3864, controller now registers as a helix
   participant. This causes the issue with PerfBenchmarkDriver
   because it separately create helixZkManager and it tries to
   register controller participant with the same host and port.
   This pr resolves the issue.


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 issue #4108: Fix AddTenantCommand bug for posting tenant config

[GitBox]

snleee commented on issue #4108: Fix AddTenantCommand bug for posting tenant 
config
URL: https://github.com/apache/incubator-pinot/pull/4108#issuecomment-482452831
 
 
   #4107


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 #4108: Fix AddTenantCommand bug for posting tenant config

[GitBox]

codecov-io commented on issue #4108: Fix AddTenantCommand bug for posting 
tenant config
URL: https://github.com/apache/incubator-pinot/pull/4108#issuecomment-482451474
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4108?src=pr=h1) 
Report
   > Merging 
[#4108](https://codecov.io/gh/apache/incubator-pinot/pull/4108?src=pr=desc) 
into 
[master](https://codecov.io/gh/apache/incubator-pinot/commit/c22a4aed36e7520fcae37e7da53b45ccbac8c0c2?src=pr=desc)
 will **increase** coverage by `21.1%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-pinot/pull/4108/graphs/tree.svg?width=650=4ibza2ugkz=150=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4108?src=pr=tree)
   
   ```diff
   @@ Coverage Diff  @@
   ## master#4108  +/-   ##
   
   + Coverage 45.74%   66.84%   +21.1% 
   - Complexity0   20  +20 
   
 Files  1037 1037  
 Lines 5142651426  
 Branches   7192 7192  
   
   + Hits  2352534376   +10851 
   + Misses2589114686   -11205 
   - Partials   2010 2364 +354
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4108?src=pr=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[...he/pinot/core/query/pruner/ValidSegmentPruner.java](https://codecov.io/gh/apache/incubator-pinot/pull/4108/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9wcnVuZXIvVmFsaWRTZWdtZW50UHJ1bmVyLmphdmE=)
 | `57.14% <0%> (-28.58%)` | `0% <0%> (ø)` | |
   | 
[...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4108/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==)
 | `75% <0%> (-25%)` | `0% <0%> (ø)` | |
   | 
[...egation/function/customobject/MinMaxRangePair.java](https://codecov.io/gh/apache/incubator-pinot/pull/4108/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9jdXN0b21vYmplY3QvTWluTWF4UmFuZ2VQYWlyLmphdmE=)
 | `75.86% <0%> (-24.14%)` | `0% <0%> (ø)` | |
   | 
[...r/validation/RealtimeSegmentValidationManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4108/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci92YWxpZGF0aW9uL1JlYWx0aW1lU2VnbWVudFZhbGlkYXRpb25NYW5hZ2VyLmphdmE=)
 | `55% <0%> (-20%)` | `0% <0%> (ø)` | |
   | 
[.../impl/dictionary/FloatOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4108/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPbkhlYXBNdXRhYmxlRGljdGlvbmFyeS5qYXZh)
 | `86.66% <0%> (-6.67%)` | `0% <0%> (ø)` | |
   | 
[...impl/dictionary/FloatOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4108/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPZmZIZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==)
 | `87.27% <0%> (-5.46%)` | `0% <0%> (ø)` | |
   | 
[...e/impl/dictionary/LongOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4108/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09uSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=)
 | `88.88% <0%> (-4.45%)` | `0% <0%> (ø)` | |
   | 
[...pinot/core/operator/docidsets/OrBlockDocIdSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/4108/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZHNldHMvT3JCbG9ja0RvY0lkU2V0LmphdmE=)
 | `84.9% <0%> (-3.78%)` | `0% <0%> (ø)` | |
   | 
[...not/broker/broker/helix/ClusterChangeMediator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4108/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0NsdXN0ZXJDaGFuZ2VNZWRpYXRvci5qYXZh)
 | `66.66% <0%> (-3.45%)` | `0% <0%> (ø)` | |
   | 
[...e/operator/dociditerators/MVScanDocIdIterator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4108/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZGl0ZXJhdG9ycy9NVlNjYW5Eb2NJZEl0ZXJhdG9yLmphdmE=)
 | `60.6% <0%> (-3.04%)` | `0% <0%> (ø)` | |
   | ... and [522 
more](https://codecov.io/gh/apache/incubator-pinot/pull/4108/diff?src=pr=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4108?src=pr=continue).
   > **Legend** - 

[incubator-pinot] branch master updated: [TE] frontend - harleyjj/alerts - filter alerts by single subscription group despite having multiple subscription groups (#4093)

[jihao]

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

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


The following commit(s) were added to refs/heads/master by this push:
 new 26330f3  [TE] frontend - harleyjj/alerts - filter alerts by single 
subscription group despite having multiple subscription groups (#4093)
26330f3 is described below

commit 26330f3a2c3309e7cf574e1fff86a1de9fb934ff
Author: Harley Jackson 
AuthorDate: Thu Apr 11 23:14:53 2019 -0700

[TE] frontend - harleyjj/alerts - filter alerts by single subscription 
group despite having multiple subscription groups (#4093)
---
 .../app/pods/manage/alerts/index/controller.js | 30 +++--
 .../app/pods/manage/alerts/index/route.js  | 50 ++
 .../app/pods/manage/explore/route.js   | 36 +---
 3 files changed, 42 insertions(+), 74 deletions(-)

diff --git 
a/thirdeye/thirdeye-frontend/app/pods/manage/alerts/index/controller.js 
b/thirdeye/thirdeye-frontend/app/pods/manage/alerts/index/controller.js
index 9cec7cd..c658533 100644
--- a/thirdeye/thirdeye-frontend/app/pods/manage/alerts/index/controller.js
+++ b/thirdeye/thirdeye-frontend/app/pods/manage/alerts/index/controller.js
@@ -281,7 +281,20 @@ export default Controller.extend({
   _recalculateFilterKeys(alertsCollection, blockItem) {
 const filterToPropertyMap = get(this, 'filterToPropertyMap');
 // Aggregate all existing values for our target properties in the current 
array collection
-const alertPropsAsKeys = alertsCollection.map(alert => 
alert[filterToPropertyMap[blockItem.name]]);
+let alertPropsAsKeys = [];
+// Make sure subscription groups are not bundled for filter parameters
+if (blockItem.name === 'subscription') {
+  alertsCollection.forEach(alert => {
+let groups = alert[filterToPropertyMap[blockItem.name]];
+if (groups) {
+  groups.split(", ").forEach(g => {
+alertPropsAsKeys.push(g);
+  });
+}
+  });
+} else {
+  alertPropsAsKeys = alertsCollection.map(alert => 
alert[filterToPropertyMap[blockItem.name]]);
+}
 // Add 'none' select option if allowed
 const canInsertNullOption = alertPropsAsKeys.includes(undefined) && 
blockItem.hasNullOption;
 if (canInsertNullOption) { alertPropsAsKeys.push('none'); }
@@ -312,7 +325,6 @@ export default Controller.extend({
 }
 // Pick up cached alert array for the secondary filters
 let filteredAlerts = get(this, 'filteredAlerts');
-
 // If there is a secondary filter present, filter by it, using the keys 
we've set up in our filter map
 Object.keys(filterToPropertyMap).forEach((filterKey) => {
   let filterValueArray = filters[filterKey];
@@ -320,7 +332,19 @@ export default Controller.extend({
 let newAlerts = filteredAlerts.filter(alert => {
   // See 'filterToPropertyMap' in route. For filterKey = 'owner' this 
would map alerts by alert['createdBy'] = x
   const targetAlertPropertyValue = 
alert[filterToPropertyMap[filterKey]];
-  const alertMeetsCriteria = targetAlertPropertyValue && 
filterValueArray.includes(targetAlertPropertyValue);
+  let alertMeetsCriteria = false;
+  // In the case for subscription, there can be multiple groups.  We 
just need to match on one
+  if (filterKey === "subscription") {
+if (targetAlertPropertyValue) {
+  filterValueArray.forEach(val => {
+if (targetAlertPropertyValue.includes(val)) {
+  alertMeetsCriteria = true;
+}
+  });
+}
+  } else {
+alertMeetsCriteria = targetAlertPropertyValue && 
filterValueArray.includes(targetAlertPropertyValue);
+  }
   const isMatchForNone = 
!alert.hasOwnProperty(filterToPropertyMap[filterKey]) && 
filterValueArray.includes('none');
   return alertMeetsCriteria || isMatchForNone;
 });
diff --git a/thirdeye/thirdeye-frontend/app/pods/manage/alerts/index/route.js 
b/thirdeye/thirdeye-frontend/app/pods/manage/alerts/index/route.js
index a21c43f..f22ecf3 100644
--- a/thirdeye/thirdeye-frontend/app/pods/manage/alerts/index/route.js
+++ b/thirdeye/thirdeye-frontend/app/pods/manage/alerts/index/route.js
@@ -66,7 +66,6 @@ export default Route.extend({
 }
   }
 }
-
 // Perform initial filters for our 'primary' filter types and add counts
 const user = getWithDefault(get(this, 'session'), 
'data.authenticated.name', null);
 const myAlertIds = user ? this._findAlertIdsByUserGroup(user, 
model.detectionAlertConfig) : [];
@@ -138,7 +137,20 @@ export default Route.extend({
 
 // Fill in select options for these filters ('filterKeys') based on alert 
properties from model.alerts
 filterBlocksLocal.filter(block => block.type === 
'select').forEach((filter) => {
-  const 

[GitHub] [incubator-pinot] jihaozh merged pull request #4093: [TE] frontend - harleyjj/alerts - filter alerts by single subscriptio…

[GitBox]

jihaozh merged pull request #4093: [TE] frontend - harleyjj/alerts - filter 
alerts by single subscriptio…
URL: https://github.com/apache/incubator-pinot/pull/4093
 
 
   


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


With regards,
Apache Git Services

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

[GitHub] [incubator-pinot] jihaozh commented on a change in pull request #4067: [TE] Holt Winters detector

[GitBox]

jihaozh commented on a change in pull request #4067: [TE] Holt Winters detector
URL: https://github.com/apache/incubator-pinot/pull/4067#discussion_r274768078
 
 

 ##
 File path: 
thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/components/HoltWintersDetector.java
 ##
 @@ -0,0 +1,541 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.thirdeye.detection.components;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+import org.apache.commons.math3.analysis.MultivariateFunction;
+import org.apache.commons.math3.optim.PointValuePair;
+import org.apache.pinot.thirdeye.common.time.TimeGranularity;
+import org.apache.pinot.thirdeye.dataframe.BooleanSeries;
+import org.apache.pinot.thirdeye.dataframe.DataFrame;
+import org.apache.pinot.thirdeye.dataframe.DoubleSeries;
+import org.apache.pinot.thirdeye.dataframe.LongSeries;
+import org.apache.pinot.thirdeye.dataframe.Series;
+import org.apache.pinot.thirdeye.dataframe.util.MetricSlice;
+import org.apache.pinot.thirdeye.datalayer.dto.DatasetConfigDTO;
+import org.apache.pinot.thirdeye.datalayer.dto.MergedAnomalyResultDTO;
+import org.apache.pinot.thirdeye.detection.ConfigUtils;
+import org.apache.pinot.thirdeye.detection.DetectionUtils;
+import org.apache.pinot.thirdeye.detection.InputDataFetcher;
+import org.apache.pinot.thirdeye.detection.Pattern;
+import org.apache.pinot.thirdeye.detection.algorithm.AlgorithmUtils;
+import org.apache.pinot.thirdeye.detection.annotation.Components;
+import org.apache.pinot.thirdeye.detection.annotation.DetectionTag;
+import org.apache.pinot.thirdeye.detection.annotation.Param;
+import org.apache.pinot.thirdeye.detection.spec.HoltWintersDetectorSpec;
+import org.apache.pinot.thirdeye.detection.spi.components.AnomalyDetector;
+import org.apache.pinot.thirdeye.detection.spi.components.BaselineProvider;
+import org.apache.pinot.thirdeye.detection.spi.model.InputData;
+import org.apache.pinot.thirdeye.detection.spi.model.InputDataSpec;
+import org.apache.pinot.thirdeye.detection.spi.model.TimeSeries;
+import org.apache.pinot.thirdeye.rootcause.impl.MetricEntity;
+import org.joda.time.DateTime;
+import org.joda.time.Interval;
+import org.joda.time.Period;
+import org.apache.commons.math3.optim.MaxIter;
+import org.apache.commons.math3.optim.nonlinear.scalar.ObjectiveFunction;
+import org.apache.commons.math3.optim.MaxEval;
+import org.apache.commons.math3.optim.SimpleBounds;
+import org.apache.commons.math3.optim.nonlinear.scalar.noderiv.BOBYQAOptimizer;
+import org.apache.commons.math3.optim.InitialGuess;
+import org.apache.commons.math3.optim.nonlinear.scalar.GoalType;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import static org.apache.pinot.thirdeye.dataframe.util.DataFrameUtils.*;
+
+/**
+ * Holt-Winters forecasting algorithm with multiplicative method
+ * Supports seasonality and trend detection
+ * https://otexts.com/fpp2/holt-winters.html
+ */
+@Components(title = "Holt Winters triple exponential smoothing forecasting and 
detection",
+type = "HOLT_WINTERS_RULE",
+tags = {DetectionTag.RULE_DETECTION},
+description = "Forecast with holt winters triple exponential smoothing and 
generate anomalies",
+params = {
+@Param(name = "alpha"),
+@Param(name = "beta"),
+@Param(name = "gamma"),
+@Param(name = "period"),
+@Param(name = "pattern"),
+@Param(name = "sensitivity"),
+@Param(name = "kernelSmoothing")})
+public class HoltWintersDetector implements 
BaselineProvider,
+
AnomalyDetector {
+  private static final Logger LOG = 
LoggerFactory.getLogger(HoltWintersDetector.class);
+  private InputDataFetcher dataFetcher;
+  private static final String COL_CURR = "current";
+  private static final String COL_BASE = "baseline";
+  private static final String COL_ANOMALY = "anomaly";
+  private static final String COL_PATTERN = "pattern";
+  private static final String COL_DIFF = "diff";
+  private static final String COL_DIFF_VIOLATION = "diff_violation";
+  private static final String COL_ERROR = "error";
+ 

[GitHub] [incubator-pinot] jihaozh commented on a change in pull request #4067: [TE] Holt Winters detector

[GitBox]

jihaozh commented on a change in pull request #4067: [TE] Holt Winters detector
URL: https://github.com/apache/incubator-pinot/pull/4067#discussion_r274766066
 
 

 ##
 File path: 
thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/components/HoltWintersDetector.java
 ##
 @@ -0,0 +1,541 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.thirdeye.detection.components;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+import org.apache.commons.math3.analysis.MultivariateFunction;
+import org.apache.commons.math3.optim.PointValuePair;
+import org.apache.pinot.thirdeye.common.time.TimeGranularity;
+import org.apache.pinot.thirdeye.dataframe.BooleanSeries;
+import org.apache.pinot.thirdeye.dataframe.DataFrame;
+import org.apache.pinot.thirdeye.dataframe.DoubleSeries;
+import org.apache.pinot.thirdeye.dataframe.LongSeries;
+import org.apache.pinot.thirdeye.dataframe.Series;
+import org.apache.pinot.thirdeye.dataframe.util.MetricSlice;
+import org.apache.pinot.thirdeye.datalayer.dto.DatasetConfigDTO;
+import org.apache.pinot.thirdeye.datalayer.dto.MergedAnomalyResultDTO;
+import org.apache.pinot.thirdeye.detection.ConfigUtils;
+import org.apache.pinot.thirdeye.detection.DetectionUtils;
+import org.apache.pinot.thirdeye.detection.InputDataFetcher;
+import org.apache.pinot.thirdeye.detection.Pattern;
+import org.apache.pinot.thirdeye.detection.algorithm.AlgorithmUtils;
+import org.apache.pinot.thirdeye.detection.annotation.Components;
+import org.apache.pinot.thirdeye.detection.annotation.DetectionTag;
+import org.apache.pinot.thirdeye.detection.annotation.Param;
+import org.apache.pinot.thirdeye.detection.spec.HoltWintersDetectorSpec;
+import org.apache.pinot.thirdeye.detection.spi.components.AnomalyDetector;
+import org.apache.pinot.thirdeye.detection.spi.components.BaselineProvider;
+import org.apache.pinot.thirdeye.detection.spi.model.InputData;
+import org.apache.pinot.thirdeye.detection.spi.model.InputDataSpec;
+import org.apache.pinot.thirdeye.detection.spi.model.TimeSeries;
+import org.apache.pinot.thirdeye.rootcause.impl.MetricEntity;
+import org.joda.time.DateTime;
+import org.joda.time.Interval;
+import org.joda.time.Period;
+import org.apache.commons.math3.optim.MaxIter;
+import org.apache.commons.math3.optim.nonlinear.scalar.ObjectiveFunction;
+import org.apache.commons.math3.optim.MaxEval;
+import org.apache.commons.math3.optim.SimpleBounds;
+import org.apache.commons.math3.optim.nonlinear.scalar.noderiv.BOBYQAOptimizer;
+import org.apache.commons.math3.optim.InitialGuess;
+import org.apache.commons.math3.optim.nonlinear.scalar.GoalType;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import static org.apache.pinot.thirdeye.dataframe.util.DataFrameUtils.*;
+
+/**
+ * Holt-Winters forecasting algorithm with multiplicative method
+ * Supports seasonality and trend detection
+ * https://otexts.com/fpp2/holt-winters.html
+ */
+@Components(title = "Holt Winters triple exponential smoothing forecasting and 
detection",
+type = "HOLT_WINTERS_RULE",
+tags = {DetectionTag.RULE_DETECTION},
+description = "Forecast with holt winters triple exponential smoothing and 
generate anomalies",
+params = {
+@Param(name = "alpha"),
+@Param(name = "beta"),
+@Param(name = "gamma"),
+@Param(name = "period"),
+@Param(name = "pattern"),
+@Param(name = "sensitivity"),
+@Param(name = "kernelSmoothing")})
+public class HoltWintersDetector implements 
BaselineProvider,
+
AnomalyDetector {
+  private static final Logger LOG = 
LoggerFactory.getLogger(HoltWintersDetector.class);
+  private InputDataFetcher dataFetcher;
+  private static final String COL_CURR = "current";
+  private static final String COL_BASE = "baseline";
+  private static final String COL_ANOMALY = "anomaly";
+  private static final String COL_PATTERN = "pattern";
+  private static final String COL_DIFF = "diff";
+  private static final String COL_DIFF_VIOLATION = "diff_violation";
+  private static final String COL_ERROR = "error";
+ 

[GitHub] [incubator-pinot] jihaozh commented on a change in pull request #4067: [TE] Holt Winters detector

[GitBox]

jihaozh commented on a change in pull request #4067: [TE] Holt Winters detector
URL: https://github.com/apache/incubator-pinot/pull/4067#discussion_r274769321
 
 

 ##
 File path: 
thirdeye/thirdeye-pinot/src/test/resources/org/apache/pinot/thirdeye/detection/algorithm/daily.csv
 ##
 @@ -0,0 +1,732 @@
+timestamp,value
 
 Review comment:
   Thank you for adding the mock data sets for testing.


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


With regards,
Apache Git Services

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

[GitHub] [incubator-pinot] jihaozh commented on a change in pull request #4067: [TE] Holt Winters detector

[GitBox]

jihaozh commented on a change in pull request #4067: [TE] Holt Winters detector
URL: https://github.com/apache/incubator-pinot/pull/4067#discussion_r274768750
 
 

 ##
 File path: 
thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/yaml/CompositePipelineConfigTranslator.java
 ##
 @@ -179,7 +179,8 @@
   private static final Set TUNING_OFF_COMPONENTS =
   ImmutableSet.of("MIGRATED_ALGORITHM_FILTER", "MIGRATED_ALGORITHM", 
"MIGRATED_ALGORITHM_BASELINE");
   private static final Map DETECTOR_TO_BASELINE =
-  ImmutableMap.of("ALGORITHM", "ALGORITHM_BASELINE", "MIGRATED_ALGORITHM", 
"MIGRATED_ALGORITHM_BASELINE");
+  ImmutableMap.of("ALGORITHM", "ALGORITHM_BASELINE", "MIGRATED_ALGORITHM", 
"MIGRATED_ALGORITHM_BASELINE",
 
 Review comment:
   I'm thinking to generalize the multiple inheritance approach (baseline 
provider and detector implemented by the same class) in all detector 
implementations, as you did in this Hot Winters implementation. After that, 
hard code this shouldn't be necessary anymore. So this hack should go away soon 
^^. Thank you for making a great example!


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