[GitHub] [incubator-pinot] codecov-io edited a comment on issue #4252: pinot_kafka_0.9.0.1 module

2019-05-30 Thread GitBox
codecov-io edited a comment on issue #4252: pinot_kafka_0.9.0.1 module
URL: https://github.com/apache/incubator-pinot/pull/4252#issuecomment-496732583
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4252?src=pr=h1) 
Report
   > Merging 
[#4252](https://codecov.io/gh/apache/incubator-pinot/pull/4252?src=pr=desc) 
into 
[master](https://codecov.io/gh/apache/incubator-pinot/commit/d540d95d24833216254ba2d1980955015e667c6f?src=pr=desc)
 will **decrease** coverage by `0.07%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-pinot/pull/4252/graphs/tree.svg?width=650=4ibza2ugkz=150=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4252?src=pr=tree)
   
   ```diff
   @@ Coverage Diff  @@
   ## master#4252  +/-   ##
   
   - Coverage 67.24%   67.16%   -0.08% 
 Complexity   20   20  
   
 Files  1041 1039   -2 
 Lines 5156451535  -29 
 Branches   7229 7228   -1 
   
   - Hits  3467534615  -60 
   - Misses1453114558  +27 
   - Partials   2358 2362   +4
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4252?src=pr=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[...altime/impl/kafka/KafkaStreamMetadataProvider.java](https://codecov.io/gh/apache/incubator-pinot/pull/4252/diff?src=pr=tree#diff-cGlub3Qta2Fma2EtMC45Lngvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2NvcmUvcmVhbHRpbWUvaW1wbC9rYWZrYS9LYWZrYVN0cmVhbU1ldGFkYXRhUHJvdmlkZXIuamF2YQ==)
 | `44.7% <ø> (ø)` | `0 <0> (?)` | |
   | 
[...re/realtime/impl/kafka/KafkaConnectionHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/4252/diff?src=pr=tree#diff-cGlub3Qta2Fma2EtMC45Lngvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2NvcmUvcmVhbHRpbWUvaW1wbC9rYWZrYS9LYWZrYUNvbm5lY3Rpb25IYW5kbGVyLmphdmE=)
 | `76.04% <ø> (ø)` | `0 <0> (?)` | |
   | 
[...core/realtime/impl/kafka/KafkaConsumerManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4252/diff?src=pr=tree#diff-cGlub3Qta2Fma2EtMC45Lngvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2NvcmUvcmVhbHRpbWUvaW1wbC9rYWZrYS9LYWZrYUNvbnN1bWVyTWFuYWdlci5qYXZh)
 | `58.82% <ø> (ø)` | `0 <0> (?)` | |
   | 
[...ime/impl/kafka/KafkaSimpleConsumerFactoryImpl.java](https://codecov.io/gh/apache/incubator-pinot/pull/4252/diff?src=pr=tree#diff-cGlub3Qta2Fma2EtMC45Lngvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2NvcmUvcmVhbHRpbWUvaW1wbC9rYWZrYS9LYWZrYVNpbXBsZUNvbnN1bWVyRmFjdG9yeUltcGwuamF2YQ==)
 | `100% <ø> (ø)` | `0 <0> (?)` | |
   | 
[.../core/realtime/impl/kafka/ConsumerAndIterator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4252/diff?src=pr=tree#diff-cGlub3Qta2Fma2EtMC45Lngvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2NvcmUvcmVhbHRpbWUvaW1wbC9rYWZrYS9Db25zdW1lckFuZEl0ZXJhdG9yLmphdmE=)
 | `90% <ø> (ø)` | `0 <0> (?)` | |
   | 
[...altime/impl/kafka/KafkaPartitionLevelConsumer.java](https://codecov.io/gh/apache/incubator-pinot/pull/4252/diff?src=pr=tree#diff-cGlub3Qta2Fma2EtMC45Lngvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2NvcmUvcmVhbHRpbWUvaW1wbC9rYWZrYS9LYWZrYVBhcnRpdGlvbkxldmVsQ29uc3VtZXIuamF2YQ==)
 | `68.57% <ø> (ø)` | `0 <0> (?)` | |
   | 
[...e/realtime/impl/kafka/KafkaJSONMessageDecoder.java](https://codecov.io/gh/apache/incubator-pinot/pull/4252/diff?src=pr=tree#diff-cGlub3Qta2Fma2EtMC45Lngvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2NvcmUvcmVhbHRpbWUvaW1wbC9rYWZrYS9LYWZrYUpTT05NZXNzYWdlRGVjb2Rlci5qYXZh)
 | `0% <ø> (ø)` | `0 <0> (?)` | |
   | 
[...ot/core/realtime/impl/kafka/KafkaStarterUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/4252/diff?src=pr=tree#diff-cGlub3Qta2Fma2EtMC45Lngvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2NvcmUvcmVhbHRpbWUvaW1wbC9rYWZrYS9LYWZrYVN0YXJ0ZXJVdGlscy5qYXZh)
 | `82.53% <ø> (ø)` | `0 <0> (?)` | |
   | 
[...e/realtime/impl/kafka/KafkaAvroMessageDecoder.java](https://codecov.io/gh/apache/incubator-pinot/pull/4252/diff?src=pr=tree#diff-cGlub3Qta2Fma2EtMC45Lngvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2NvcmUvcmVhbHRpbWUvaW1wbC9rYWZrYS9LYWZrYUF2cm9NZXNzYWdlRGVjb2Rlci5qYXZh)
 | `0% <ø> (ø)` | `0 <0> (?)` | |
   | 
[...altime/impl/kafka/KafkaStreamConfigProperties.java](https://codecov.io/gh/apache/incubator-pinot/pull/4252/diff?src=pr=tree#diff-cGlub3Qta2Fma2EtMC45Lngvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L2NvcmUvcmVhbHRpbWUvaW1wbC9rYWZrYS9LYWZrYVN0cmVhbUNvbmZpZ1Byb3BlcnRpZXMuamF2YQ==)
 | `25% <ø> (ø)` | `0 <0> (?)` | |
   | ... and [35 
more](https://codecov.io/gh/apache/incubator-pinot/pull/4252/diff?src=pr=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4252?src=pr=continue).
   > **Legend** - [Click here to learn 

[GitHub] [incubator-pinot] snleee edited a comment on issue #4252: pinot_kafka_0.9.0.1 module

2019-05-30 Thread GitBox
snleee edited a comment on issue #4252: pinot_kafka_0.9.0.1 module
URL: https://github.com/apache/incubator-pinot/pull/4252#issuecomment-497541544
 
 
   @mcvsubbu Instead of copying code, we can still document that `1.0` module 
is compatible with `2.0` if we `verified` that they are compatible. I didn't 
want to guarantee 1.0 would work with 2.0 because we might not want to test and 
track compatibility for every single kafka version. We can add module on demand 
when we need the new one after API change.
   
   My suggestion was more about naming since I didn't like to put specific 
version because it seemed not to be the convention.


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 edited a comment on issue #4252: pinot_kafka_0.9.0.1 module

2019-05-30 Thread GitBox
snleee edited a comment on issue #4252: pinot_kafka_0.9.0.1 module
URL: https://github.com/apache/incubator-pinot/pull/4252#issuecomment-497541544
 
 
   @mcvsubbu We can still document that `1.0` module is compatible with `2.0` 
if we `verified` that they are instead of copying code. I didn't want to 
guarantee 1.0 would work with 2.0 because we might not want to test 
compatibility for every single kafka version. We can add module on demand when 
we need the new one after API change.
   
   My suggestion was more about naming since I didn't like to put specific 
version because it seemed not to be the convention.


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 edited a comment on issue #4252: pinot_kafka_0.9.0.1 module

2019-05-30 Thread GitBox
snleee edited a comment on issue #4252: pinot_kafka_0.9.0.1 module
URL: https://github.com/apache/incubator-pinot/pull/4252#issuecomment-497541544
 
 
   @mcvsubbu Instead of copying code, we can still document that `1.0` module 
is compatible with `2.0` if we `verified` that they are compatible. I didn't 
want to guarantee 1.0 would work with 2.0 because we might not want to test 
compatibility for every single kafka version. We can add module on demand when 
we need the new one after API change.
   
   My suggestion was more about naming since I didn't like to put specific 
version because it seemed not to be the convention.


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 edited a comment on issue #4252: pinot_kafka_0.9.0.1 module

2019-05-30 Thread GitBox
snleee edited a comment on issue #4252: pinot_kafka_0.9.0.1 module
URL: https://github.com/apache/incubator-pinot/pull/4252#issuecomment-497541544
 
 
   @mcvsubbu We can still document that `1.0` module is compatible with `2.0` 
if we *verified* that they are instead of copying code. I didn't want to 
guarantee 1.0 would work with 2.0 because we might not want to test 
compatibility for every single kafka version. We can add module on demand when 
we need the new one after API change.
   
   My suggestion was more about naming since I didn't like to put specific 
version because it seemed not to be the convention.


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 #4252: pinot_kafka_0.9.0.1 module

2019-05-30 Thread GitBox
snleee commented on issue #4252: pinot_kafka_0.9.0.1 module
URL: https://github.com/apache/incubator-pinot/pull/4252#issuecomment-497541544
 
 
   @mcvsubbu We can still document that `1.0` module is compatible with `2.0` 
if we verified that they are instead of copying code. We can add module on 
demand when we need the new one after API change.
   
   My suggestion was more about naming since I didn't like to put specific 
version because it seemed not to be the convention.


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


With regards,
Apache Git Services

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



[GitHub] [incubator-pinot] mcvsubbu commented on issue #4252: pinot_kafka_0.9.0.1 module

2019-05-30 Thread GitBox
mcvsubbu commented on issue #4252: pinot_kafka_0.9.0.1 module
URL: https://github.com/apache/incubator-pinot/pull/4252#issuecomment-497539277
 
 
   > @mcvsubbu `kafka-1.0` module will import `1.0.x` library. It may work with 
`kafka-2.0` if there is no API change but we don't guarantee it. For explicit 
2.0 support, we can create another module `kafka-2.0` that imports `2.0.x` and 
we can guarantee that it will work with kafka 2.0.
   > 
   > @npawar I vote +1 for following flink's approach to organize our modules. 
It's very clean. I also vote for `pinot-kafka-0.9` instead of 
`pinot-kafka-0.9.x`.
   
   In other words, you are suggesting that we copy over all files even if 
nothing changes, and keep doing this for every (major) kafka release. 
   


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 edited a comment on issue #4252: pinot_kafka_0.9.0.1 module

2019-05-30 Thread GitBox
snleee edited a comment on issue #4252: pinot_kafka_0.9.0.1 module
URL: https://github.com/apache/incubator-pinot/pull/4252#issuecomment-497537882
 
 
   @mcvsubbu `kafka-1.0` module will import `1.0.x`  library. It may work with 
`kafka-2.0` if there is no API change but we don't guarantee it. For explicit 
2.0 support,  we can create another module `kafka-2.0` that imports `2.0.x` and 
we can guarantee that it will work with kafka 2.0. 
   
   @npawar I vote +1 for following flink's approach to organize our modules. 
It's very clean. I also vote for `kafka-0.9` instead of `kafka-0.9.x`.


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 edited a comment on issue #4252: pinot_kafka_0.9.0.1 module

2019-05-30 Thread GitBox
snleee edited a comment on issue #4252: pinot_kafka_0.9.0.1 module
URL: https://github.com/apache/incubator-pinot/pull/4252#issuecomment-497537882
 
 
   @mcvsubbu `kafka-1.0` module will import `1.0.x`  library. It may work with 
`kafka-2.0` if there is no API change but we don't guarantee it. For explicit 
2.0 support,  we can create another module `kafka-2.0` that imports `2.0.x` and 
we can guarantee that it will work with kafka 2.0. 
   
   @npawar I vote +1 for following flink's approach to organize our modules. 
It's very clean. I also vote for `pinot-kafka-0.9` instead of 
`pinot-kafka-0.9.x`.


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 edited a comment on issue #4252: pinot_kafka_0.9.0.1 module

2019-05-30 Thread GitBox
snleee edited a comment on issue #4252: pinot_kafka_0.9.0.1 module
URL: https://github.com/apache/incubator-pinot/pull/4252#issuecomment-497537882
 
 
   @mcvsubbu `kafka-1.0` module will import `1.0.x`  library. It may work with 
`kafka-2.0` if there is no API change but we don't guarantee it. For explicit 
2.0 support,  we can create another module `kafka-2.0` that imports `2.0.x` and 
we can guarantee that it will work with kafka 2.0. 
   
   @npawar I vote +1 for following flink approach to organize our modules. It's 
very clean. I also vote for `kafka-0.9` instead of `kafka-0.9.x`.


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 edited a comment on issue #4252: pinot_kafka_0.9.0.1 module

2019-05-30 Thread GitBox
snleee edited a comment on issue #4252: pinot_kafka_0.9.0.1 module
URL: https://github.com/apache/incubator-pinot/pull/4252#issuecomment-497537882
 
 
   @mcvsubbu `kafka-1.0` module will import `1.0.x`  library. It may work with 
`kafka-2.0` if there was no API change but we don't guarantee it. For explicit 
2.0 support,  we can create another module `kafka-2.0` that imports `2.0.x` and 
we can guarantee that it will work with kafka 2.0. 
   
   @npawar I vote +1 for following flink approach to organize our modules. It's 
very clean. I also vote for `kafka-0.9` instead of `kafka-0.9.x`.


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 edited a comment on issue #4252: pinot_kafka_0.9.0.1 module

2019-05-30 Thread GitBox
snleee edited a comment on issue #4252: pinot_kafka_0.9.0.1 module
URL: https://github.com/apache/incubator-pinot/pull/4252#issuecomment-497537882
 
 
   @mcvsubbu `kafka-1.0` module will import `1.0.x`  library. It may work with 
`kafka-2.0` if there was no API change but we don't guarantee it. For explicit 
2.0 support,  we can create another module `kafka-2.0` that import `2.0.x` and 
we can guarantee that it will work with kafka 2.0. 
   
   @npawar I vote +1 for following flink approach to organize our modules. It's 
very clean. I also vote for `kafka-0.9` instead of `kafka-0.9.x`.


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 #4252: pinot_kafka_0.9.0.1 module

2019-05-30 Thread GitBox
snleee commented on issue #4252: pinot_kafka_0.9.0.1 module
URL: https://github.com/apache/incubator-pinot/pull/4252#issuecomment-497537882
 
 
   @mcvsubbu `kafka-1.0` module will import `1.0.x`  library. It may work with 
`kafka-2.0` if there was no API change but we don't guarantee it. We will 
create another module `kafka-2.0` that will import `2.0.x` and we can guarantee 
that it will work with kafka 2.0. 
   
   @npawar I vote +1 for following flink approach to organize our modules. It's 
very clean. I also vote for `kafka-0.9` instead of `kafka-0.9.x`.


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


With regards,
Apache Git Services

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



[GitHub] [incubator-pinot] mcvsubbu commented on issue #4252: pinot_kafka_0.9.0.1 module

2019-05-30 Thread GitBox
mcvsubbu commented on issue #4252: pinot_kafka_0.9.0.1 module
URL: https://github.com/apache/incubator-pinot/pull/4252#issuecomment-497534338
 
 
   > Thanks for the update, we have some plans to upgrade to later version of 
Kafka (> 1.0). I think we should open a new module named like pinot-kafka-1.0 
when we work on the new version?
   > 
   > We also have plans to migrate away from the scala-based kafka simple 
consumer to the [new consumer 
model](https://www.confluent.io/blog/tutorial-getting-started-with-the-new-apache-kafka-0-9-consumer-client/)
 as it is deprecated since 
[0.10](https://cwiki.apache.org/confluence/display/KAFKA/KIP-109%3A+Old+Consumer+Deprecation)
 and the new consumer has compatible API as well more features. Do you guys 
have any work planned for it or we can investigate more into this effort? 
@chenboat
   
   +1 to what @npawar said, but the naming of the folders are still up in the 
air.
   
For example, if we name the directory 1.0.10 and it so happens that kafka 
does not change any APIs, and it is also valid for 2.0 then what happens?


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


With regards,
Apache Git Services

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



[GitHub] [incubator-pinot] kishoreg commented on issue #4252: pinot_kafka_0.9.0.1 module

2019-05-30 Thread GitBox
kishoreg commented on issue #4252: pinot_kafka_0.9.0.1 module
URL: https://github.com/apache/incubator-pinot/pull/4252#issuecomment-497531293
 
 
   > > I missed the comment from @snleee of naming. I agree with him and prefer 
0.9.x and I think Kafka guarantees that API's won't change in minor versions. 
The problem with naming it 0.9.0.1 is that its not a common convention and 
users might think that it works only for that version of Kafka which is not the 
case. See Flink naming convention for example 
https://github.com/apache/flink/tree/master/flink-connectors.
   > > Another suggestion is to create a subfolder for all implementations if 
possible. We have too many modules at the root pinot-orc, pinot-parquet, 
pinot-hadoop-filesytem. If it's not too hard, we can move kafka module into a 
subfolder.
   > 
   > Sure, we can consider keeping it just 0.9, 1.0 etc, will check regarding 
the kafka guarantee about no changes in minor versions.
   > 
   > Regarding sub folder for all implementations, are you suggesting we put 
all implementations in 1 module (pinot-streams?)
   
   @npawar Thanks. I am suggesting one module per implementation but just group 
them under one directory. Similar to the way Flink has organized its 
connectors. https://github.com/apache/flink/tree/master/flink-connectors. 
pinot-connectors is actually a pretty good name for the subfolder and we can 
have pinot-connector-kafka-0.9 module. 
   
   


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: Change ControllerTest default prot to 18998 (#4259)

2019-05-30 Thread 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 d540d95  Change ControllerTest default prot to 18998 (#4259)
d540d95 is described below

commit d540d95d24833216254ba2d1980955015e667c6f
Author: Xiaotian (Jackie) Jiang <1751+jackie-ji...@users.noreply.github.com>
AuthorDate: Thu May 30 17:22:05 2019 -0700

Change ControllerTest default prot to 18998 (#4259)

Port 8998 is commonly used, which might cause address already in use 
problem.
Change it to 18998 to avoid this.
Also remove redundant entries in gitignore.
---
 .gitignore  | 6 --
 .../test/java/org/apache/pinot/controller/helix/ControllerTest.java | 2 +-
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/.gitignore b/.gitignore
index be407e9..78661c2e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -14,12 +14,6 @@ test-output
 pinot-transport/test-output/
 pinot-broker/test-output/
 pinot-core/test-output/
-pinot-dashboard/.env
-pinot-dashboard/activate
-pinot-dashboard/config.yml
-pinot-dashboard/logs/*
-pinot-dashboard/dist
-pinot-dashboard/pinotui.egg-info
 *.log
 .doppelganger
 docs/_build
diff --git 
a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
 
b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
index 20bdcfb..9893377 100644
--- 
a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
+++ 
b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
@@ -57,7 +57,7 @@ import org.testng.Assert;
 public abstract class ControllerTest {
   public static final String LOCAL_HOST = "localhost";
 
-  private static final int DEFAULT_CONTROLLER_PORT = 8998;
+  private static final int DEFAULT_CONTROLLER_PORT = 18998;
   private static final String DEFAULT_DATA_DIR =
   new File(FileUtils.getTempDirectoryPath(), "test-controller-" + 
System.currentTimeMillis()).getAbsolutePath();
 


-
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 #4259: Change ControllerTest default prot to 18998

2019-05-30 Thread GitBox
Jackie-Jiang merged pull request #4259: Change ControllerTest default prot to 
18998
URL: https://github.com/apache/incubator-pinot/pull/4259
 
 
   


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 change_controller_test_port deleted (was 9918487)

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

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


 was 9918487  Change ControllerTest default prot to 18998

The revisions that were on this branch are still contained in
other references; therefore, this change does not discard any commits
from the repository.


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



[GitHub] [incubator-pinot] codecov-io edited a comment on issue #4253: Add pre-compute Hadoop job

2019-05-30 Thread GitBox
codecov-io edited a comment on issue #4253: Add pre-compute Hadoop job
URL: https://github.com/apache/incubator-pinot/pull/4253#issuecomment-497205230
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4253?src=pr=h1) 
Report
   > Merging 
[#4253](https://codecov.io/gh/apache/incubator-pinot/pull/4253?src=pr=desc) 
into 
[master](https://codecov.io/gh/apache/incubator-pinot/commit/3b9ebea18ca18e1dbd18c6c12a414162c6c9ceff?src=pr=desc)
 will **increase** coverage by `6.99%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-pinot/pull/4253/graphs/tree.svg?width=650=4ibza2ugkz=150=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4253?src=pr=tree)
   
   ```diff
   @@ Coverage Diff  @@
   ## master#4253  +/-   ##
   
   + Coverage 60.38%   67.38%   +6.99% 
 Complexity   20   20  
   
 Files  1165 1041 -124 
 Lines 5856551567-6998 
 Branches   8153 7229 -924 
   
   - Hits  3536734750 -617 
   + Misses2074714447-6300 
   + Partials   2451 2370  -81
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4253?src=pr=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[...va/org/apache/pinot/common/config/TableConfig.java](https://codecov.io/gh/apache/incubator-pinot/pull/4253/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL1RhYmxlQ29uZmlnLmphdmE=)
 | `81.81% <100%> (+0.2%)` | `0 <0> (ø)` | :arrow_down: |
   | 
[.../BrokerResourceOnlineOfflineStateModelFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/4253/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclJlc291cmNlT25saW5lT2ZmbGluZVN0YXRlTW9kZWxGYWN0b3J5LmphdmE=)
 | `46.42% <0%> (-10.72%)` | `0% <0%> (ø)` | |
   | 
[...e/operator/dociditerators/MVScanDocIdIterator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4253/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZGl0ZXJhdG9ycy9NVlNjYW5Eb2NJZEl0ZXJhdG9yLmphdmE=)
 | `60.6% <0%> (-6.07%)` | `0% <0%> (ø)` | |
   | 
[...ot/core/query/pruner/ColumnValueSegmentPruner.java](https://codecov.io/gh/apache/incubator-pinot/pull/4253/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9wcnVuZXIvQ29sdW1uVmFsdWVTZWdtZW50UHJ1bmVyLmphdmE=)
 | `85.91% <0%> (-4.23%)` | `0% <0%> (ø)` | |
   | 
[...n/java/org/apache/pinot/common/utils/DataSize.java](https://codecov.io/gh/apache/incubator-pinot/pull/4253/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvRGF0YVNpemUuamF2YQ==)
 | `81.25% <0%> (-3.13%)` | `0% <0%> (ø)` | |
   | 
[...pby/NoDictionarySingleColumnGroupKeyGenerator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4253/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9ncm91cGJ5L05vRGljdGlvbmFyeVNpbmdsZUNvbHVtbkdyb3VwS2V5R2VuZXJhdG9yLmphdmE=)
 | `86.53% <0%> (-1.93%)` | `0% <0%> (ø)` | |
   | 
[...ata/manager/realtime/RealtimeTableDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4253/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVUYWJsZURhdGFNYW5hZ2VyLmphdmE=)
 | `41.17% <0%> (-1.48%)` | `0% <0%> (ø)` | |
   | 
[...g/apache/pinot/common/metrics/AbstractMetrics.java](https://codecov.io/gh/apache/incubator-pinot/pull/4253/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9BYnN0cmFjdE1ldHJpY3MuamF2YQ==)
 | `76% <0%> (-1.34%)` | `0% <0%> (ø)` | |
   | 
[...a/org/apache/pinot/core/common/DataBlockCache.java](https://codecov.io/gh/apache/incubator-pinot/pull/4253/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9jb21tb24vRGF0YUJsb2NrQ2FjaGUuamF2YQ==)
 | `78.62% <0%> (-0.77%)` | `0% <0%> (ø)` | |
   | 
[...ot/common/protocols/SegmentCompletionProtocol.java](https://codecov.io/gh/apache/incubator-pinot/pull/4253/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcHJvdG9jb2xzL1NlZ21lbnRDb21wbGV0aW9uUHJvdG9jb2wuamF2YQ==)
 | `92.48% <0%> (-0.58%)` | `0% <0%> (ø)` | |
   | ... and [144 
more](https://codecov.io/gh/apache/incubator-pinot/pull/4253/diff?src=pr=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4253?src=pr=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, 

[GitHub] [incubator-pinot] jihaozh merged pull request #4263: add padding to graphs in email and Anomalies route

2019-05-30 Thread GitBox
jihaozh merged pull request #4263: add padding to graphs in email and Anomalies 
route
URL: https://github.com/apache/incubator-pinot/pull/4263
 
 
   


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


With regards,
Apache Git Services

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



[incubator-pinot] branch master updated: [TE] add padding to graphs in email and Anomalies route (#4263)

2019-05-30 Thread 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 27b61a1  [TE] add padding to graphs in email and Anomalies route 
(#4263)
27b61a1 is described below

commit 27b61a1bf23fcc358ade6bf8ea073415fd21aa56
Author: Harley Jackson 
AuthorDate: Thu May 30 17:00:34 2019 -0700

[TE] add padding to graphs in email and Anomalies route (#4263)
---
 .../pods/components/anomaly-summary/component.js   | 28 ++
 .../app/pods/screenshot/controller.js  | 21 
 .../thirdeye-frontend/app/pods/screenshot/route.js |  7 ++
 3 files changed, 36 insertions(+), 20 deletions(-)

diff --git 
a/thirdeye/thirdeye-frontend/app/pods/components/anomaly-summary/component.js 
b/thirdeye/thirdeye-frontend/app/pods/components/anomaly-summary/component.js
index 2d1e7eb..d4d65e9 100644
--- 
a/thirdeye/thirdeye-frontend/app/pods/components/anomaly-summary/component.js
+++ 
b/thirdeye/thirdeye-frontend/app/pods/components/anomaly-summary/component.js
@@ -81,8 +81,19 @@ export default Component.extend({
 
   axis: computed(
 'anomalyData',
+'series',
 function () {
-  const anomalyData = get(this, 'anomalyData');
+  const {
+anomalyData,
+series
+  } = this.getProperties('anomalyData', 'series');
+
+  let start = anomalyData.startTime;
+  let end = anomalyData.endTime;
+  if (series.current && series.current.timestamps && 
Array.isArray(series.current.timestamps)) {
+start = series.current.timestamps[0];
+end = series.current.timestamps[series.current.timestamps.length - 1];
+  }
 
   return {
 y: {
@@ -99,8 +110,8 @@ export default Component.extend({
 x: {
   type: 'timeseries',
   show: true,
-  min: anomalyData.startTime,
-  max: anomalyData.endTime,
+  min: start,
+  max: end,
   tick: {
 fit: false,
 format: (d) => {
@@ -137,10 +148,10 @@ export default Component.extend({
 };
   }
 
-  if (current && !_.isEmpty(current.value)) {
+  if (current && !_.isEmpty(current.current)) {
 series['current'] = {
   timestamps: current.timestamp,
-  values: current.value,
+  values: current.current,
   type: 'line',
   color: 'blue'
 };
@@ -206,17 +217,14 @@ export default Component.extend({
   .then(checkStatus)
   .then(res => {
 set(this, 'anomalyData', res);
-const timeZone = 'America/Los_Angeles';
-const currentUrl = 
`/rootcause/metric/timeseries?urn=${res.metricUrn}=${res.startTime}=${res.endTime}=current=${timeZone}`;
-const predictedUrl = 
`/detection/predicted-baseline/${anomalyId}?start=${res.startTime}=${res.endTime}`;
+const predictedUrl = 
`/detection/predicted-baseline/${anomalyId}?start=${res.startTime}=${res.endTime}=true`;
 const timeseriesHash = {
-  current: fetch(currentUrl).then(res => checkStatus(res, 'get', 
true)),
   predicted: fetch(predictedUrl).then(res => checkStatus(res, 'get', 
true))
 };
 return RSVP.hash(timeseriesHash);
   })
   .then((res) => {
-set(this, 'current', res.current);
+set(this, 'current', res.predicted);
 set(this, 'predicted', res.predicted);
 set(this, 'isLoading', false);
   })
diff --git a/thirdeye/thirdeye-frontend/app/pods/screenshot/controller.js 
b/thirdeye/thirdeye-frontend/app/pods/screenshot/controller.js
index 0072ad4..245fb7a 100644
--- a/thirdeye/thirdeye-frontend/app/pods/screenshot/controller.js
+++ b/thirdeye/thirdeye-frontend/app/pods/screenshot/controller.js
@@ -72,10 +72,10 @@ export default Controller.extend({
 };
   }
 
-  if (current && !_.isEmpty(current.value)) {
+  if (current && !_.isEmpty(current.current)) {
 series['current'] = {
   timestamps: current.timestamp,
-  values: current.value,
+  values: current.current,
   type: 'line',
   color: 'blue'
 };
@@ -95,8 +95,19 @@ export default Controller.extend({
 
   axis: computed(
 'anomalyData',
+'series',
 function () {
-  const anomalyData = get(this, 'anomalyData');
+  const {
+anomalyData,
+series
+  } = this.getProperties('anomalyData', 'series');
+
+  let start = anomalyData.startTime;
+  let end = anomalyData.endTime;
+  if (series.current && series.current.timestamps && 
Array.isArray(series.current.timestamps)) {
+start = series.current.timestamps[0];
+end = series.current.timestamps[series.current.timestamps.length - 1];
+  }
 
   return {
 y: {
@@ -113,8 +124,8 @@ export default Controller.extend({
 x: {
   type: 

[GitHub] [incubator-pinot] jihaozh merged pull request #4262: [TE] add current time series if it's unavailable in predicted baseline endpoint

2019-05-30 Thread GitBox
jihaozh merged pull request #4262: [TE] add current time series if it's 
unavailable in predicted baseline endpoint
URL: https://github.com/apache/incubator-pinot/pull/4262
 
 
   


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


With regards,
Apache Git Services

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



[incubator-pinot] branch master updated: [TE] add current time series if it's unavailable in predicted baseline endpoint (#4262)

2019-05-30 Thread 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 e768195  [TE] add current time series if it's unavailable  in 
predicted baseline endpoint (#4262)
e768195 is described below

commit e7681954731349be98318b4f52177e79ec638b11
Author: Jihao Zhang 
AuthorDate: Thu May 30 16:59:36 2019 -0700

[TE] add current time series if it's unavailable  in predicted baseline 
endpoint (#4262)

In predicted baseline endpoint, add the current time series only when it's 
unavailable.
---
 .../apache/pinot/thirdeye/detection/DetectionResource.java | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git 
a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/DetectionResource.java
 
b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/DetectionResource.java
index 9104593..9779fed 100644
--- 
a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/DetectionResource.java
+++ 
b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/DetectionResource.java
@@ -551,11 +551,15 @@ public class DetectionResource {
   end = new DateTime(end, 
dataTimeZone).plus(offsets.getPostOffsetPeriod()).getMillis();
 }
 MetricEntity me = MetricEntity.fromURN(anomaly.getMetricUrn());
-TimeSeries baselineTimeseries = 
DetectionUtils.getBaselineTimeseries(anomaly, me.getFilters(), me.getId(), 
configDAO.findById(anomaly.getDetectionConfigId()), start, end, loader, 
provider);
-// add current time series
-MetricSlice currentSlice = MetricSlice.from(me.getId(), start, end, 
me.getFilters());
-DataFrame dfCurrent = 
this.provider.fetchTimeseries(Collections.singleton(currentSlice)).get(currentSlice).renameSeries(COL_VALUE,
 COL_CURRENT);
-return 
Response.ok(dfCurrent.joinOuter(baselineTimeseries.getDataFrame())).build();
+DataFrame baselineTimeseries = 
DetectionUtils.getBaselineTimeseries(anomaly, me.getFilters(), me.getId(),
+configDAO.findById(anomaly.getDetectionConfigId()), start, end, 
loader, provider).getDataFrame();
+if (!baselineTimeseries.contains(COL_CURRENT)) {
+  // add current time series if not exists
+  MetricSlice currentSlice = MetricSlice.from(me.getId(), start, end, 
me.getFilters());
+  DataFrame dfCurrent = 
this.provider.fetchTimeseries(Collections.singleton(currentSlice)).get(currentSlice).renameSeries(COL_VALUE,
 COL_CURRENT);
+  baselineTimeseries = dfCurrent.joinOuter(baselineTimeseries);
+}
+return Response.ok(baselineTimeseries).build();
   }
 
 }


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



[GitHub] [incubator-pinot] jihaozh edited a comment on issue #4262: [TE] add current time series if it's unavailable in predicted baseline endpoint

2019-05-30 Thread GitBox
jihaozh edited a comment on issue #4262: [TE] add current time series if it's 
unavailable in predicted baseline endpoint
URL: https://github.com/apache/incubator-pinot/pull/4262#issuecomment-497520534
 
 
   > @jihaozh Can you give more details why what will leads to this scenario?
   
   The previous PR added padding so that the graphs will have paddings. 
https://github.com/apache/incubator-pinot/pull/4260
   
   This PR fixes an edge case that if the current time series is already 
returned from the baseline provider, it will skip adding 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] harleyjj opened a new pull request #4263: add padding to graphs in email and Anomalies route

2019-05-30 Thread GitBox
harleyjj opened a new pull request #4263: add padding to graphs in email and 
Anomalies route
URL: https://github.com/apache/incubator-pinot/pull/4263
 
 
   


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 issue #4262: [TE] add current time series if it's unavailable in predicted baseline endpoint

2019-05-30 Thread GitBox
jihaozh commented on issue #4262: [TE] add current time series if it's 
unavailable in predicted baseline endpoint
URL: https://github.com/apache/incubator-pinot/pull/4262#issuecomment-497520534
 
 
   > @jihaozh Can you give more details why what will leads to this scenario?
   The previous PR added padding so that the graphs will have paddings. 
https://github.com/apache/incubator-pinot/pull/4260
   
   This PR fixes an edge case that if the current time series is already 
returned from the baseline provider, it will skip adding 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] sunithabeeram commented on a change in pull request #4243: Add support for passing headers in pinot client

2019-05-30 Thread GitBox
sunithabeeram commented on a change in pull request #4243: Add support for 
passing headers in pinot client
URL: https://github.com/apache/incubator-pinot/pull/4243#discussion_r289208037
 
 

 ##
 File path: 
pinot-api/src/main/java/org/apache/pinot/client/PinotClientTransportFactory.java
 ##
 @@ -18,9 +18,11 @@
  */
 package org.apache.pinot.client;
 
+import java.util.Map;
+
 /**
  * Factory for client transports.
  */
 interface PinotClientTransportFactory {
-  PinotClientTransport buildTransport();
+  PinotClientTransport buildTransport(Map headers);
 
 Review comment:
   @kishoreg it looks like the current ConnectionFactory is tied to JsonHttp 
transport. How would things work if Https has to be supported? It appears to me 
that the transport factory itself needs to be injected to ConnectionFactory 
class in that case. 
   
   The class has been marked as evolving. Is it because you see new methods 
getting added in the future or do you have other cleanup/changes in mind?


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


With regards,
Apache Git Services

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



[GitHub] [incubator-pinot] xiaohui-sun commented on issue #4262: [TE] add current time series if it's unavailable in predicted baseline endpoint

2019-05-30 Thread GitBox
xiaohui-sun commented on issue #4262: [TE] add current time series if it's 
unavailable in predicted baseline endpoint
URL: https://github.com/apache/incubator-pinot/pull/4262#issuecomment-497518264
 
 
   @jihaozh Can you give more details why what will leads to this scenario?


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 a change in pull request #4243: Add support for passing headers in pinot client

2019-05-30 Thread GitBox
sunithabeeram commented on a change in pull request #4243: Add support for 
passing headers in pinot client
URL: https://github.com/apache/incubator-pinot/pull/4243#discussion_r289204095
 
 

 ##
 File path: 
pinot-api/src/main/java/org/apache/pinot/client/ConnectionFactory.java
 ##
 @@ -18,13 +18,20 @@
  */
 package org.apache.pinot.client;
 
+import org.apache.yetus.audience.InterfaceAudience;
 
 Review comment:
   Please use annotations from pinot-common.


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


With regards,
Apache Git Services

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



[GitHub] [incubator-pinot] jihaozh opened a new pull request #4262: [TE] add current time series if it's unavailable in predicted baseline endpoint

2019-05-30 Thread GitBox
jihaozh opened a new pull request #4262: [TE] add current time series if it's 
unavailable in predicted baseline endpoint
URL: https://github.com/apache/incubator-pinot/pull/4262
 
 
   In predicted baseline endpoint, add the current time series only when it's 
unavailable. 


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


With regards,
Apache Git Services

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



[incubator-pinot] branch master updated: [TE] add padding to baseline endpoints (#4260)

2019-05-30 Thread 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 3b9ebea  [TE] add padding to baseline endpoints (#4260)
3b9ebea is described below

commit 3b9ebea18ca18e1dbd18c6c12a414162c6c9ceff
Author: Jihao Zhang 
AuthorDate: Thu May 30 15:28:23 2019 -0700

[TE] add padding to baseline endpoints (#4260)

- add padding option to the predicted baseline endpoints.
- refactor rule-based baseline providers to return predicted baselines and 
boundaries.
---
 .../thirdeye/detection/DetectionResource.java  | 37 +-
 .../components/AbsoluteChangeRuleDetector.java |  5 ++-
 .../components/PercentageChangeRuleDetector.java   |  5 ++-
 .../detection/components/RuleBaselineProvider.java | 13 ++--
 .../components/ThresholdRuleDetector.java  |  1 -
 .../components/RuleBaselineProviderTest.java   |  6 ++--
 6 files changed, 46 insertions(+), 21 deletions(-)

diff --git 
a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/DetectionResource.java
 
b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/DetectionResource.java
index cb7e761..9104593 100644
--- 
a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/DetectionResource.java
+++ 
b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/DetectionResource.java
@@ -52,6 +52,8 @@ import org.apache.pinot.thirdeye.constant.AnomalyFeedbackType;
 import org.apache.pinot.thirdeye.constant.AnomalyResultSource;
 import org.apache.pinot.thirdeye.dashboard.resources.v2.ResourceUtils;
 import 
org.apache.pinot.thirdeye.dashboard.resources.v2.rootcause.AnomalyEventFormatter;
+import org.apache.pinot.thirdeye.dataframe.DataFrame;
+import org.apache.pinot.thirdeye.dataframe.util.MetricSlice;
 import org.apache.pinot.thirdeye.datalayer.bao.DatasetConfigManager;
 import org.apache.pinot.thirdeye.datalayer.bao.DetectionAlertConfigManager;
 import org.apache.pinot.thirdeye.datalayer.bao.DetectionConfigManager;
@@ -60,6 +62,7 @@ import org.apache.pinot.thirdeye.datalayer.bao.EventManager;
 import org.apache.pinot.thirdeye.datalayer.bao.MergedAnomalyResultManager;
 import org.apache.pinot.thirdeye.datalayer.bao.MetricConfigManager;
 import org.apache.pinot.thirdeye.datalayer.dto.AnomalyFeedbackDTO;
+import org.apache.pinot.thirdeye.datalayer.dto.DatasetConfigDTO;
 import org.apache.pinot.thirdeye.datalayer.dto.DetectionAlertConfigDTO;
 import org.apache.pinot.thirdeye.datalayer.dto.DetectionConfigDTO;
 import org.apache.pinot.thirdeye.datalayer.dto.MergedAnomalyResultDTO;
@@ -75,12 +78,18 @@ import 
org.apache.pinot.thirdeye.detection.finetune.GridSearchTuningAlgorithm;
 import org.apache.pinot.thirdeye.detection.finetune.TuningAlgorithm;
 import org.apache.pinot.thirdeye.detection.spi.model.AnomalySlice;
 import org.apache.pinot.thirdeye.detection.spi.model.TimeSeries;
+import org.apache.pinot.thirdeye.detector.function.BaseAnomalyFunction;
+import org.apache.pinot.thirdeye.rootcause.impl.MetricEntity;
+import org.apache.pinot.thirdeye.util.AnomalyOffset;
 import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
 import org.joda.time.Interval;
 import org.quartz.CronExpression;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static org.apache.pinot.thirdeye.dataframe.util.DataFrameUtils.*;
+
 
 @Path("/detection")
 @Produces(MediaType.APPLICATION_JSON)
@@ -518,23 +527,35 @@ public class DetectionResource {
   }
 
   @GET
-  @ApiOperation("get the predicted baseline for an anomaly within a time 
range")
+  @ApiOperation("get the current time series and predicted baselines for an 
anomaly within a time range")
   @Path(value = "/predicted-baseline/{anomalyId}")
   public Response getPredictedBaseline(
 @PathParam("anomalyId") @ApiParam("anomalyId") long anomalyId,
-  @QueryParam("start") long start,
-  @QueryParam("end") long end
+  @ApiParam("Start time for the predicted baselines") @QueryParam("start") 
long start,
+  @ApiParam("End time for the predicted baselines") @QueryParam("end") 
long end,
+  @ApiParam("Add padding to the window based on metric granularity") 
@QueryParam("padding") @DefaultValue("false") boolean padding
   ) throws Exception {
 MergedAnomalyResultDTO anomaly = anomalyDAO.findById(anomalyId);
 if (anomaly == null) {
   throw new IllegalArgumentException(String.format("Could not resolve 
anomaly id %d", anomalyId));
 }
-MetricConfigDTO metric = 
this.metricDAO.findByMetricAndDataset(anomaly.getMetric(), 
anomaly.getCollection());
-if (metric == null) {
-  throw new IllegalArgumentException(String.format("Could not resolve 
metric '%s' in dataset '%s' for anomaly id %d", anomaly.getMetric(), 
anomaly.getCollection(), anomaly.getId()));
+if 

[GitHub] [incubator-pinot] jihaozh merged pull request #4260: [TE] add padding to baseline endpoints

2019-05-30 Thread GitBox
jihaozh merged pull request #4260: [TE] add padding to baseline endpoints
URL: https://github.com/apache/incubator-pinot/pull/4260
 
 
   


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


With regards,
Apache Git Services

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



[GitHub] [incubator-pinot] npawar commented on issue #4252: pinot_kafka_0.9.0.1 module

2019-05-30 Thread GitBox
npawar commented on issue #4252: pinot_kafka_0.9.0.1 module
URL: https://github.com/apache/incubator-pinot/pull/4252#issuecomment-497505892
 
 
   > Thanks for the update, we have some plans to upgrade to later version of 
Kafka (> 1.0). I think we should open a new module named like pinot-kafka-1.0 
when we work on the new version?
   > 
   > We also have plans to migrate away from the scala-based kafka simple 
consumer to the [new consumer 
model](https://www.confluent.io/blog/tutorial-getting-started-with-the-new-apache-kafka-0-9-consumer-client/)
 as it is deprecated since 
[0.10](https://cwiki.apache.org/confluence/display/KAFKA/KIP-109%3A+Old+Consumer+Deprecation)
 and the new consumer has compatible API as well more features. Do you guys 
have any work planned for it or we can investigate more into this effort? 
@chenboat
   
   Yes, you can create a new pinot-kafka-1.0 when you decide to work with newer 
version. We don't have any immediate plans of investigating it. It would be 
great if you can help out with that


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


With regards,
Apache Git Services

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



[GitHub] [incubator-pinot] npawar commented on issue #4252: pinot_kafka_0.9.0.1 module

2019-05-30 Thread GitBox
npawar commented on issue #4252: pinot_kafka_0.9.0.1 module
URL: https://github.com/apache/incubator-pinot/pull/4252#issuecomment-497505493
 
 
   > I missed the comment from @snleee of naming. I agree with him and prefer 
0.9.x and I think Kafka guarantees that API's won't change in minor versions. 
The problem with naming it 0.9.0.1 is that its not a common convention and 
users might think that it works only for that version of Kafka which is not the 
case. See Flink naming convention for example 
https://github.com/apache/flink/tree/master/flink-connectors.
   > 
   > Another suggestion is to create a subfolder for all implementations if 
possible. We have too many modules at the root pinot-orc, pinot-parquet, 
pinot-hadoop-filesytem. If it's not too hard, we can move kafka module into a 
subfolder.
   
   Sure, we can consider keeping it just 0.9, 1.0 etc, will check regarding the 
kafka guarantee about no changes in minor versions.
   
   Regarding sub folder for all implementations, are you suggesting we put all 
implementations in 1 module (pinot-streams?)


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


With regards,
Apache Git Services

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



[GitHub] [incubator-pinot] jamesyfshao commented on issue #4261: implement upsert support on pinot

2019-05-30 Thread GitBox
jamesyfshao commented on issue #4261: implement upsert support on pinot
URL: 
https://github.com/apache/incubator-pinot/issues/4261#issuecomment-497497405
 
 
   summary on 1st discussion of upsert design (May 15th):
   
   1. @Jackie-Jiang points out that current design of rewriting queries for 
upsert table is problematic as this cause too much overhead in pinot query 
process with so many or-conditions. We should look into method to reduce the 
query overhead in upsert table
   
   2. We should look into message delivery from coordinator service to pinot 
server. @Jackie-Jiang proposed methods on re-using existing download API on 
coordinator service to deliver messages from coordinator to server for unified 
API.
   
   3. How to handle kafka topic partition change. @mcvsubbu suggested that we 
should look into how to handle accidental expand of kafka topic if that happens.


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 #4259: Change ControllerTest default prot to 18998

2019-05-30 Thread GitBox
codecov-io commented on issue #4259: Change ControllerTest default prot to 18998
URL: https://github.com/apache/incubator-pinot/pull/4259#issuecomment-497492925
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4259?src=pr=h1) 
Report
   > Merging 
[#4259](https://codecov.io/gh/apache/incubator-pinot/pull/4259?src=pr=desc) 
into 
[master](https://codecov.io/gh/apache/incubator-pinot/commit/7abfb12e1ad96614c1811f1276eedd3af16af051?src=pr=desc)
 will **increase** coverage by `6.87%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-pinot/pull/4259/graphs/tree.svg?width=650=4ibza2ugkz=150=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4259?src=pr=tree)
   
   ```diff
   @@ Coverage Diff  @@
   ## master#4259  +/-   ##
   
   + Coverage 60.35%   67.23%   +6.87% 
 Complexity   20   20  
   
 Files  1165 1041 -124 
 Lines 5853551534-7001 
 Branches   8144 7220 -924 
   
   - Hits  3532934648 -681 
   + Misses2076414529-6235 
   + Partials   2442 2357  -85
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4259?src=pr=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4259/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==)
 | `50% <0%> (-50%)` | `0% <0%> (ø)` | |
   | 
[...pache/pinot/core/util/SortedRangeIntersection.java](https://codecov.io/gh/apache/incubator-pinot/pull/4259/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL1NvcnRlZFJhbmdlSW50ZXJzZWN0aW9uLmphdmE=)
 | `83.82% <0%> (-7.36%)` | `0% <0%> (ø)` | |
   | 
[...e/operator/dociditerators/SortedDocIdIterator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4259/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZGl0ZXJhdG9ycy9Tb3J0ZWREb2NJZEl0ZXJhdG9yLmphdmE=)
 | `55.55% <0%> (-2.78%)` | `0% <0%> (ø)` | |
   | 
[...g/apache/pinot/common/metrics/AbstractMetrics.java](https://codecov.io/gh/apache/incubator-pinot/pull/4259/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9BYnN0cmFjdE1ldHJpY3MuamF2YQ==)
 | `76% <0%> (-1.34%)` | `0% <0%> (ø)` | |
   | 
[.../pinot/core/data/manager/BaseTableDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4259/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvQmFzZVRhYmxlRGF0YU1hbmFnZXIuamF2YQ==)
 | `92.68% <0%> (-1.22%)` | `0% <0%> (ø)` | |
   | 
[.../pinot/core/common/datatable/DataTableBuilder.java](https://codecov.io/gh/apache/incubator-pinot/pull/4259/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9jb21tb24vZGF0YXRhYmxlL0RhdGFUYWJsZUJ1aWxkZXIuamF2YQ==)
 | `78.94% <0%> (-1.17%)` | `0% <0%> (ø)` | |
   | 
[...core/query/executor/ServerQueryExecutorV1Impl.java](https://codecov.io/gh/apache/incubator-pinot/pull/4259/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9leGVjdXRvci9TZXJ2ZXJRdWVyeUV4ZWN1dG9yVjFJbXBsLmphdmE=)
 | `81.37% <0%> (-0.69%)` | `0% <0%> (ø)` | |
   | 
[...n/java/org/apache/pinot/tools/SegmentDumpTool.java](https://codecov.io/gh/apache/incubator-pinot/pull/4259/diff?src=pr=tree#diff-cGlub3QtdG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3Rvb2xzL1NlZ21lbnREdW1wVG9vbC5qYXZh)
 | | | |
   | 
[...segment/converter/PinotSegmentToJsonConverter.java](https://codecov.io/gh/apache/incubator-pinot/pull/4259/diff?src=pr=tree#diff-cGlub3QtdG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3Rvb2xzL3NlZ21lbnQvY29udmVydGVyL1Bpbm90U2VnbWVudFRvSnNvbkNvbnZlcnRlci5qYXZh)
 | | | |
   | 
[...ot/tools/query/comparison/SegmentInfoProvider.java](https://codecov.io/gh/apache/incubator-pinot/pull/4259/diff?src=pr=tree#diff-cGlub3QtdG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3Rvb2xzL3F1ZXJ5L2NvbXBhcmlzb24vU2VnbWVudEluZm9Qcm92aWRlci5qYXZh)
 | | | |
   | ... and [132 
more](https://codecov.io/gh/apache/incubator-pinot/pull/4259/diff?src=pr=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4259?src=pr=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4259?src=pr=footer).
 Last update 

[GitHub] [incubator-pinot] jamesyfshao opened a new issue #4261: implement upsert support on pinot

2019-05-30 Thread GitBox
jamesyfshao opened a new issue #4261: implement upsert support on pinot
URL: https://github.com/apache/incubator-pinot/issues/4261
 
 
   Pinot is a distributed real-time OLAP engine that can provide second-level 
data freshness by ingesting kafka events and capacity to manage months of 
historical data load from various data sources such as HDFS, schemaless, etc. 
However, Pinot right now mostly functions as an append-only storage system. It 
doesn’t allow modify/delete of existing records with the exception of 
overriding all data within a time range with offline tables. This limits the 
applicability of pinot system due to a lot of use-cases requiring updates to 
their data due to the nature of their events or needs for data 
correction/backfill. In order to extend the capacity of pinot and serve more 
use-cases, we are going to implement the upsert features in Pinot which allows 
users to update existing records in Pinot tables with its kafka data input 
stream.
   
   Some initial requirements for the upsert projects:
   
   1. Only support for full update to pinot event
   
   2. Only support for Kafka-compatible queue ingestion model
   
   3. Single pinot server/table can handle 10k/sec ingestion message rate
   
   4. Each pinot server can handle 1 Billion records or 1TB storage
   
   5. Ingestion latency overhead compared to non-upsert model < 1min
   
   6. Query latency overhead compared to non-upsert model < 50%
   
   7. Data retention < 2 weeks
   


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


With regards,
Apache Git Services

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



[GitHub] [incubator-pinot] jihaozh opened a new pull request #4260: [TE] add padding to baseline endpoints

2019-05-30 Thread GitBox
jihaozh opened a new pull request #4260: [TE] add padding to baseline endpoints
URL: https://github.com/apache/incubator-pinot/pull/4260
 
 
   - add padding option to the predicted baseline endpoints. 
   - refactor rule-based baseline providers to return current, predicted 
baselines and boundaries. 


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


With regards,
Apache Git Services

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



[GitHub] [incubator-pinot] codecov-io edited a comment on issue #4253: Add pre-compute Hadoop job

2019-05-30 Thread GitBox
codecov-io edited a comment on issue #4253: Add pre-compute Hadoop job
URL: https://github.com/apache/incubator-pinot/pull/4253#issuecomment-497205230
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4253?src=pr=h1) 
Report
   > Merging 
[#4253](https://codecov.io/gh/apache/incubator-pinot/pull/4253?src=pr=desc) 
into 
[master](https://codecov.io/gh/apache/incubator-pinot/commit/7abfb12e1ad96614c1811f1276eedd3af16af051?src=pr=desc)
 will **increase** coverage by `6.89%`.
   > The diff coverage is `100%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-pinot/pull/4253/graphs/tree.svg?width=650=4ibza2ugkz=150=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4253?src=pr=tree)
   
   ```diff
   @@ Coverage Diff  @@
   ## master#4253  +/-   ##
   
   + Coverage 60.35%   67.25%   +6.89% 
 Complexity   20   20  
   
 Files  1165 1041 -124 
 Lines 5853551537-6998 
 Branches   8144 7220 -924 
   
   - Hits  3532934661 -668 
   + Misses2076414511-6253 
   + Partials   2442 2365  -77
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4253?src=pr=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[...va/org/apache/pinot/common/config/TableConfig.java](https://codecov.io/gh/apache/incubator-pinot/pull/4253/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL1RhYmxlQ29uZmlnLmphdmE=)
 | `81.81% <100%> (+0.2%)` | `0 <0> (ø)` | :arrow_down: |
   | 
[...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4253/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==)
 | `50% <0%> (-50%)` | `0% <0%> (ø)` | |
   | 
[...he/pinot/core/query/pruner/ValidSegmentPruner.java](https://codecov.io/gh/apache/incubator-pinot/pull/4253/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9wcnVuZXIvVmFsaWRTZWdtZW50UHJ1bmVyLmphdmE=)
 | `57.14% <0%> (-28.58%)` | `0% <0%> (ø)` | |
   | 
[.../apache/pinot/core/transport/DataTableHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/4253/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS90cmFuc3BvcnQvRGF0YVRhYmxlSGFuZGxlci5qYXZh)
 | `88% <0%> (-12%)` | `0% <0%> (ø)` | |
   | 
[...pache/pinot/core/util/SortedRangeIntersection.java](https://codecov.io/gh/apache/incubator-pinot/pull/4253/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL1NvcnRlZFJhbmdlSW50ZXJzZWN0aW9uLmphdmE=)
 | `83.82% <0%> (-7.36%)` | `0% <0%> (ø)` | |
   | 
[...mpl/dictionary/DoubleOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4253/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT2ZmSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=)
 | `73.21% <0%> (-5.36%)` | `0% <0%> (ø)` | |
   | 
[...n/java/org/apache/pinot/common/utils/DataSize.java](https://codecov.io/gh/apache/incubator-pinot/pull/4253/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvRGF0YVNpemUuamF2YQ==)
 | `81.25% <0%> (-3.13%)` | `0% <0%> (ø)` | |
   | 
[...e/operator/dociditerators/MVScanDocIdIterator.java](https://codecov.io/gh/apache/incubator-pinot/pull/4253/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZGl0ZXJhdG9ycy9NVlNjYW5Eb2NJZEl0ZXJhdG9yLmphdmE=)
 | `60.6% <0%> (-3.04%)` | `0% <0%> (ø)` | |
   | 
[...core/query/executor/ServerQueryExecutorV1Impl.java](https://codecov.io/gh/apache/incubator-pinot/pull/4253/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9leGVjdXRvci9TZXJ2ZXJRdWVyeUV4ZWN1dG9yVjFJbXBsLmphdmE=)
 | `80.68% <0%> (-1.38%)` | `0% <0%> (ø)` | |
   | 
[.../pinot/core/data/manager/BaseTableDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4253/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvQmFzZVRhYmxlRGF0YU1hbmFnZXIuamF2YQ==)
 | `92.68% <0%> (-1.22%)` | `0% <0%> (ø)` | |
   | ... and [137 
more](https://codecov.io/gh/apache/incubator-pinot/pull/4253/diff?src=pr=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4253?src=pr=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø = not affected`, `? = missing data`
   > Powered by 

[incubator-pinot] branch master updated: Add RealtimeConsumptionCatchupServiceCallback (#4218)

2019-05-30 Thread nehapawar
This is an automated email from the ASF dual-hosted git repository.

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


The following commit(s) were added to refs/heads/master by this push:
 new 62ce57f  Add RealtimeConsumptionCatchupServiceCallback (#4218)
62ce57f is described below

commit 62ce57f4886e36daaeee438c9af7c6dc279b4844
Author: Neha Pawar 
AuthorDate: Thu May 30 13:06:41 2019 -0700

Add RealtimeConsumptionCatchupServiceCallback (#4218)

Creating a ServiceCallback within ServiceStatus, to check that realtime 
consuming segments have caught up. This will help with latency issues which can 
happen during server startup, because the server has to catchup on a lot of 
consumption.
For the initial version, this only adds a static wait time (configurable). 
More smarts can be added later (monitor stability or idleness of stream 
consumption rate)
---
 .../broker/broker/helix/HelixBrokerStarter.java|  34 +--
 .../pinot/common/config/TableNameBuilder.java  |   7 ++
 .../apache/pinot/common/utils/CommonConstants.java |   3 +
 .../apache/pinot/common/utils/ServiceStatus.java   | 107 -
 .../tests/OfflineClusterIntegrationTest.java   |  38 ++--
 .../server/starter/helix/HelixServerStarter.java   |  73 --
 6 files changed, 190 insertions(+), 72 deletions(-)

diff --git 
a/pinot-broker/src/main/java/org/apache/pinot/broker/broker/helix/HelixBrokerStarter.java
 
b/pinot-broker/src/main/java/org/apache/pinot/broker/broker/helix/HelixBrokerStarter.java
index 10d34fb..57beed3 100644
--- 
a/pinot-broker/src/main/java/org/apache/pinot/broker/broker/helix/HelixBrokerStarter.java
+++ 
b/pinot-broker/src/main/java/org/apache/pinot/broker/broker/helix/HelixBrokerStarter.java
@@ -35,6 +35,7 @@ import org.apache.helix.HelixManagerFactory;
 import org.apache.helix.InstanceType;
 import org.apache.helix.SystemPropertyKeys;
 import org.apache.helix.ZNRecord;
+import org.apache.helix.model.IdealState;
 import org.apache.helix.model.InstanceConfig;
 import org.apache.helix.model.Message;
 import org.apache.helix.participant.StateMachineEngine;
@@ -237,16 +238,35 @@ public class HelixBrokerStarter {
 .addPreConnectCallback(() -> 
brokerMetrics.addMeteredGlobalValue(BrokerMeter.HELIX_ZOOKEEPER_RECONNECTS, 
1L));
 
 // Register the service status handler
-LOGGER.info("Registering service status handler");
+registerServiceStatusHandler();
+
+LOGGER.info("Finish starting Pinot broker");
+  }
+
+  /**
+   * Fetches the resources to monitor and registers the {@link 
org.apache.pinot.common.utils.ServiceStatus.ServiceStatusCallback}s
+   */
+  private void registerServiceStatusHandler() {
+List resourcesToMonitor = new ArrayList<>(1);
+IdealState brokerResourceIdealState = 
_helixAdmin.getResourceIdealState(_clusterName, Helix.BROKER_RESOURCE_INSTANCE);
+if (brokerResourceIdealState != null && 
brokerResourceIdealState.isEnabled()) {
+  for (String partitionName : brokerResourceIdealState.getPartitionSet()) {
+if 
(brokerResourceIdealState.getInstanceSet(partitionName).contains(_brokerId)) {
+  resourcesToMonitor.add(Helix.BROKER_RESOURCE_INSTANCE);
+  break;
+}
+  }
+}
+
 double minResourcePercentForStartup = 
_brokerConf.getDouble(Broker.CONFIG_OF_BROKER_MIN_RESOURCE_PERCENT_FOR_START,
 Broker.DEFAULT_BROKER_MIN_RESOURCE_PERCENT_FOR_START);
-ServiceStatus.setServiceStatusCallback(new 
ServiceStatus.MultipleCallbackServiceStatusCallback(ImmutableList
-.of(new 
ServiceStatus.IdealStateAndCurrentStateMatchServiceStatusCallback(_participantHelixManager,
-_clusterName, _brokerId, minResourcePercentForStartup),
-new 
ServiceStatus.IdealStateAndExternalViewMatchServiceStatusCallback(_participantHelixManager,
-_clusterName, _brokerId, minResourcePercentForStartup;
 
-LOGGER.info("Finish starting Pinot broker");
+LOGGER.info("Registering service status handler");
+ServiceStatus.setServiceStatusCallback(new 
ServiceStatus.MultipleCallbackServiceStatusCallback(ImmutableList.of(
+new 
ServiceStatus.IdealStateAndCurrentStateMatchServiceStatusCallback(_participantHelixManager,
 _clusterName,
+_brokerId, resourcesToMonitor, minResourcePercentForStartup),
+new 
ServiceStatus.IdealStateAndExternalViewMatchServiceStatusCallback(_participantHelixManager,
 _clusterName,
+_brokerId, resourcesToMonitor, minResourcePercentForStartup;
   }
 
   private void addInstanceTagIfNeeded() {
diff --git 
a/pinot-common/src/main/java/org/apache/pinot/common/config/TableNameBuilder.java
 
b/pinot-common/src/main/java/org/apache/pinot/common/config/TableNameBuilder.java
index 5ecac01..8ec01ff 100644
--- 
a/pinot-common/src/main/java/org/apache/pinot/common/config/TableNameBuilder.java
+++ 

[GitHub] [incubator-pinot] npawar merged pull request #4218: Add RealtimeConsumptionCatchupServiceCallback

2019-05-30 Thread GitBox
npawar merged pull request #4218: Add RealtimeConsumptionCatchupServiceCallback
URL: https://github.com/apache/incubator-pinot/pull/4218
 
 
   


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


With regards,
Apache Git Services

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



[GitHub] [incubator-pinot] codecov-io edited a comment on issue #4218: Add RealtimeConsumptionCatchupServiceCallback

2019-05-30 Thread GitBox
codecov-io edited a comment on issue #4218: Add 
RealtimeConsumptionCatchupServiceCallback
URL: https://github.com/apache/incubator-pinot/pull/4218#issuecomment-493596443
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4218?src=pr=h1) 
Report
   > Merging 
[#4218](https://codecov.io/gh/apache/incubator-pinot/pull/4218?src=pr=desc) 
into 
[master](https://codecov.io/gh/apache/incubator-pinot/commit/45a8430bc3d981b44e2d8e242c75480d7dc593e6?src=pr=desc)
 will **increase** coverage by `0.05%`.
   > The diff coverage is `33.84%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-pinot/pull/4218/graphs/tree.svg?width=650=4ibza2ugkz=150=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4218?src=pr=tree)
   
   ```diff
   @@ Coverage Diff  @@
   ## master#4218  +/-   ##
   
   + Coverage 67.24%   67.29%   +0.05% 
 Complexity   20   20  
   
 Files  1041 1041  
 Lines 5151451564  +50 
 Branches   7216 7229  +13 
   
   + Hits  3464034700  +60 
   + Misses1450614480  -26 
   - Partials   2368 2384  +16
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4218?src=pr=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[...org/apache/pinot/common/utils/CommonConstants.java](https://codecov.io/gh/apache/incubator-pinot/pull/4218/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=)
 | `45.65% <ø> (ø)` | `0 <0> (ø)` | :arrow_down: |
   | 
[...g/apache/pinot/common/config/TableNameBuilder.java](https://codecov.io/gh/apache/incubator-pinot/pull/4218/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL1RhYmxlTmFtZUJ1aWxkZXIuamF2YQ==)
 | `92.3% <0%> (-3.7%)` | `0 <0> (ø)` | |
   | 
[...a/org/apache/pinot/common/utils/ServiceStatus.java](https://codecov.io/gh/apache/incubator-pinot/pull/4218/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvU2VydmljZVN0YXR1cy5qYXZh)
 | `75% <0%> (-8.96%)` | `0 <0> (ø)` | |
   | 
[...pinot/server/starter/helix/HelixServerStarter.java](https://codecov.io/gh/apache/incubator-pinot/pull/4218/diff?src=pr=tree#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvc3RhcnRlci9oZWxpeC9IZWxpeFNlcnZlclN0YXJ0ZXIuamF2YQ==)
 | `44.91% <44.11%> (-0.98%)` | `0 <0> (ø)` | |
   | 
[.../pinot/broker/broker/helix/HelixBrokerStarter.java](https://codecov.io/gh/apache/incubator-pinot/pull/4218/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0hlbGl4QnJva2VyU3RhcnRlci5qYXZh)
 | `73.15% <53.84%> (-2.56%)` | `0 <0> (ø)` | |
   | 
[...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4218/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==)
 | `75% <0%> (-25%)` | `0% <0%> (ø)` | |
   | 
[...ot/core/realtime/stream/PartitionCountFetcher.java](https://codecov.io/gh/apache/incubator-pinot/pull/4218/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9zdHJlYW0vUGFydGl0aW9uQ291bnRGZXRjaGVyLmphdmE=)
 | `54.16% <0%> (-20.84%)` | `0% <0%> (ø)` | |
   | 
[.../pinot/controller/ControllerLeadershipManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4218/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9Db250cm9sbGVyTGVhZGVyc2hpcE1hbmFnZXIuamF2YQ==)
 | `74.41% <0%> (-15.59%)` | `0% <0%> (ø)` | |
   | 
[...e/impl/dictionary/LongOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4218/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09uSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=)
 | `82.6% <0%> (-13.05%)` | `0% <0%> (ø)` | |
   | 
[.../impl/dictionary/LongOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4218/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09mZkhlYXBNdXRhYmxlRGljdGlvbmFyeS5qYXZh)
 | `89.28% <0%> (-5.36%)` | `0% <0%> (ø)` | |
   | ... and [38 
more](https://codecov.io/gh/apache/incubator-pinot/pull/4218/diff?src=pr=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4218?src=pr=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  (impact)`, `ø 

[GitHub] [incubator-pinot] jamesyfshao commented on issue #4252: pinot_kafka_0.9.0.1 module

2019-05-30 Thread GitBox
jamesyfshao commented on issue #4252: pinot_kafka_0.9.0.1 module
URL: https://github.com/apache/incubator-pinot/pull/4252#issuecomment-497452153
 
 
   Thanks for the update, we have some plans to upgrade to later version of 
Kafka (> 1.0). I think we should open a new module named like pinot-kafka-1.0 
when we work on the new version? 
   
   We also have plans to migrate away from the scala-based kafka simple 
consumer to the [new consumer 
model](https://www.confluent.io/blog/tutorial-getting-started-with-the-new-apache-kafka-0-9-consumer-client/)
 as it is deprecated since 
[0.10](https://cwiki.apache.org/confluence/display/KAFKA/KIP-109%3A+Old+Consumer+Deprecation)
 and the new consumer has compatible API as well more features. Do you guys 
have any work planned for it or we can investigate more into this effort? 
@chenboat


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 change_controller_test_port created (now 9918487)

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

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


  at 9918487  Change ControllerTest default prot to 18998

This branch includes the following new commits:

 new 9918487  Change ControllerTest default prot to 18998

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: Change ControllerTest default prot to 18998

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

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

commit 991848790c0c1567a8ee4d82431728331c1a0e03
Author: Jackie (Xiaotian) Jiang 
AuthorDate: Thu May 30 11:44:43 2019 -0700

Change ControllerTest default prot to 18998

Port 8998 is commonly used, which might cause address already in use 
problem.
Change it to 18998 to avoid this.
Also remove redundant entries in gitignore.
---
 .gitignore  | 6 --
 .../test/java/org/apache/pinot/controller/helix/ControllerTest.java | 2 +-
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/.gitignore b/.gitignore
index be407e9..78661c2e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -14,12 +14,6 @@ test-output
 pinot-transport/test-output/
 pinot-broker/test-output/
 pinot-core/test-output/
-pinot-dashboard/.env
-pinot-dashboard/activate
-pinot-dashboard/config.yml
-pinot-dashboard/logs/*
-pinot-dashboard/dist
-pinot-dashboard/pinotui.egg-info
 *.log
 .doppelganger
 docs/_build
diff --git 
a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
 
b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
index 20bdcfb..9893377 100644
--- 
a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
+++ 
b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
@@ -57,7 +57,7 @@ import org.testng.Assert;
 public abstract class ControllerTest {
   public static final String LOCAL_HOST = "localhost";
 
-  private static final int DEFAULT_CONTROLLER_PORT = 8998;
+  private static final int DEFAULT_CONTROLLER_PORT = 18998;
   private static final String DEFAULT_DATA_DIR =
   new File(FileUtils.getTempDirectoryPath(), "test-controller-" + 
System.currentTimeMillis()).getAbsolutePath();
 


-
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 #4259: Change ControllerTest default prot to 18998

2019-05-30 Thread GitBox
Jackie-Jiang opened a new pull request #4259: Change ControllerTest default 
prot to 18998
URL: https://github.com/apache/incubator-pinot/pull/4259
 
 
   Port 8998 is commonly used, which might cause address already in use problem.
   Change it to 18998 to avoid this.
   Also remove redundant entries in gitignore.


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


With regards,
Apache Git Services

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



[GitHub] [incubator-pinot] Jackie-Jiang commented on issue #4230: NULL value support for all data types

2019-05-30 Thread GitBox
Jackie-Jiang commented on issue #4230: NULL value support for all data types
URL: 
https://github.com/apache/incubator-pinot/issues/4230#issuecomment-497436948
 
 
   I would prefer always assume all columns are nullable unless users 
explicitly mark it nonNull (which is very rare, but might be useful for boolean 
type. A default null value is required).
   For the existing segment, leave it as is. For default column, generate empty 
presence vector if the column is nullable.
   For query execution, we should automatically filter out NULLs as needed, and 
also support explicit filters on NULLs.


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


With regards,
Apache Git Services

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



[GitHub] [incubator-pinot] npawar commented on issue #4252: pinot_kafka_0.9.0.1 module

2019-05-30 Thread GitBox
npawar commented on issue #4252: pinot_kafka_0.9.0.1 module
URL: https://github.com/apache/incubator-pinot/pull/4252#issuecomment-497427818
 
 
   FYI @chenboat @jamesyfshao  This directly affects the kafka implementation 
you currently use. There should be no impact, but please take a look


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: SoC - Separate out Tuning from Translator (#4250)

2019-05-30 Thread akshayrai09
This is an automated email from the ASF dual-hosted git repository.

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


The following commit(s) were added to refs/heads/master by this push:
 new 7abfb12  SoC - Separate out Tuning from Translator (#4250)
7abfb12 is described below

commit 7abfb12e1ad96614c1811f1276eedd3af16af051
Author: Akshay Rai 
AuthorDate: Thu May 30 11:09:51 2019 -0700

SoC - Separate out Tuning from Translator (#4250)

Currently tuning is tied together with the translator. Each time we need to 
tune the detection, we have to call the translator which tries to re-convert 
the yaml config to detection object. This PR separates the tuner from the 
translator so that it can be triggered directly on the detection object.

Changes:
* Introduced ConfigTranslator interface for detection and subscription
* Enforce validation of raw and parsed config through interface 
(template-pattern)
* Moved out tuning related code under DetectionConfigTuner
* Move translator specific code under translator package
* Remove YamlTranslationResult; Cleaned up buildMetricUrn; Consistently use 
componentKey throughout instead of componentName
* Added and updated unit tests
---
 .../api/application/ApplicationResource.java   |   2 +-
 .../thirdeye/detection/DetectionPipeline.java  |  17 +-
 .../pinot/thirdeye/detection/DetectionUtils.java   |  18 ++-
 .../annotation/registry/DetectionRegistry.java |   4 +-
 .../onboard/YamlOnboardingTaskRunner.java  |  18 +--
 .../detection/validators/ConfigValidator.java  |   2 +-
 .../validators/SubscriptionConfigValidator.java|   2 +-
 .../detection/wrapper/AnomalyDetectorWrapper.java  |   2 +-
 .../detection/wrapper/AnomalyFilterWrapper.java|   2 +-
 .../wrapper/BaselineFillingMergeWrapper.java   |  12 +-
 .../thirdeye/detection/wrapper/GrouperWrapper.java |   2 +-
 .../detection/yaml/DetectionConfigTuner.java   | 178 +
 .../thirdeye/detection/yaml/YamlResource.java  |  36 +++--
 .../detection/yaml/YamlTranslationResult.java  |  87 --
 .../CompositePipelineConfigTranslator.java | 113 -
 .../yaml/translator/ConfigTranslator.java  |  55 +++
 .../YamlDetectionAlertConfigTranslator.java|  48 +++---
 .../YamlDetectionConfigTranslator.java |  57 ++-
 .../YamlDetectionTranslatorLoader.java |   2 +-
 .../thirdeye/rootcause/impl/MetricEntity.java  |  13 ++
 .../thirdeye/detection/DetectionUtilsTest.java |  51 ++
 .../yaml/MockYamlDetectionConfigTranslator.java|  21 ---
 .../yaml/YamlDetectionConfigTranslatorTest.java|  60 ---
 .../CompositePipelineConfigTranslatorTest.java |  38 +++--
 .../YamlDetectionAlertConfigTranslatorTest.java|  13 +-
 .../yaml/translator/YamlTranslationResult.java |  54 +++
 .../compositePipelineTranslatorTestResult-1.json   |   6 +-
 .../compositePipelineTranslatorTestResult-2.json   |   0
 .../yaml/{ => translator}/pipeline-config-1.yaml   |   2 +-
 .../yaml/{ => translator}/pipeline-config-2.yaml   |   0
 .../yaml/{ => translator}/pipeline-config-3.yaml   |   0
 31 files changed, 524 insertions(+), 391 deletions(-)

diff --git 
a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/api/application/ApplicationResource.java
 
b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/api/application/ApplicationResource.java
index 639e79a..7a2edf4 100644
--- 
a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/api/application/ApplicationResource.java
+++ 
b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/api/application/ApplicationResource.java
@@ -45,7 +45,7 @@ import org.apache.pinot.thirdeye.detection.ConfigUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import static 
org.apache.pinot.thirdeye.detection.yaml.YamlDetectionAlertConfigTranslator.*;
+import static 
org.apache.pinot.thirdeye.detection.yaml.translator.YamlDetectionAlertConfigTranslator.*;
 
 
 @Path(value = "/application")
diff --git 
a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/DetectionPipeline.java
 
b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/DetectionPipeline.java
index d8b2d66..dafc844 100644
--- 
a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/DetectionPipeline.java
+++ 
b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/DetectionPipeline.java
@@ -90,22 +90,21 @@ public abstract class DetectionPipeline {
 Map instancesMap = config.getComponents();
 Map componentSpecs = config.getComponentSpecs();
 if (componentSpecs != null) {
-  for (String componentName : componentSpecs.keySet()) {
-Map componentSpec = MapUtils.getMap(componentSpecs, 
componentName);
-if 

[GitHub] [incubator-pinot] akshayrai merged pull request #4250: SoC - Separate out Tuning from Translator

2019-05-30 Thread GitBox
akshayrai merged pull request #4250: SoC - Separate out Tuning from Translator
URL: https://github.com/apache/incubator-pinot/pull/4250
 
 
   


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


With regards,
Apache Git Services

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



[incubator-pinot] 01/01: Release branch only:Upgrade helix to 0.8.4 for release

2019-05-30 Thread nehapawar
This is an automated email from the ASF dual-hosted git repository.

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

commit 1ebcfd99fedad43c655462433cda31aec1bfac04
Author: Neha Pawar 
AuthorDate: Tue May 28 16:02:18 2019 -0700

Release branch only:Upgrade helix to 0.8.4 for release
---
 pom.xml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pom.xml b/pom.xml
index 83ed620..cadc6c4 100644
--- a/pom.xml
+++ b/pom.xml
@@ -117,7 +117,7 @@
 
 1.7.6
 1.8.0
-0.8.2
+0.8.4
 
 0.9.0.1
 0.7


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



[incubator-pinot] branch helix_084_release_branch created (now 1ebcfd9)

2019-05-30 Thread nehapawar
This is an automated email from the ASF dual-hosted git repository.

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


  at 1ebcfd9  Release branch only:Upgrade helix to 0.8.4 for release

This branch includes the following new commits:

 new 1ebcfd9  Release branch only:Upgrade helix to 0.8.4 for release

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] kishoreg commented on issue #4252: pinot_kafka_0.9.0.1 module

2019-05-30 Thread GitBox
kishoreg commented on issue #4252: pinot_kafka_0.9.0.1 module
URL: https://github.com/apache/incubator-pinot/pull/4252#issuecomment-497423677
 
 
   I missed the comment from @snleee of naming. I agree with him and prefer 
0.9.x and I think Kafka guarantees that API's won't change in minor versions. 
The problem with naming it 0.9.0.1 is that its not a common convention and 
users might think that it works only for that version of Kafka which is not the 
case. See Flink naming convention for example 
https://github.com/apache/flink/tree/master/flink-connectors.
   
   Another suggestion is to create a subfolder for all implementations if 
possible. We have too many modules at the root pinot-orc, pinot-parquet, 
pinot-hadoop-filesytem. If it's not too hard, we can move kafka module into a 
subfolder.
   


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


With regards,
Apache Git Services

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



[GitHub] [incubator-pinot] mcvsubbu commented on issue #4252: pinot_kafka_0.9.0.1 module

2019-05-30 Thread GitBox
mcvsubbu commented on issue #4252: pinot_kafka_0.9.0.1 module
URL: https://github.com/apache/incubator-pinot/pull/4252#issuecomment-497413455
 
 
   I am fine with this. @ananthdurai , @kishoreg  anyone have any opinions? 
@ananthdurai  are you good  in terms of implementing a new adapter for newer 
versions of kafka? Please follow the conversation in this PR for naming 
conventions and decide accordingly.


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


With regards,
Apache Git Services

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



[GitHub] [incubator-pinot] xiaohui-sun opened a new pull request #4258: [TE] Change default auto onboard frequency to 6 hours

2019-05-30 Thread GitBox
xiaohui-sun opened a new pull request #4258: [TE] Change default auto onboard 
frequency to 6 hours
URL: https://github.com/apache/incubator-pinot/pull/4258
 
 
   15 mins is too frequent and changed to 6 hours.


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


With regards,
Apache Git Services

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



[GitHub] [incubator-pinot] codecov-io edited a comment on issue #4243: Add support for passing headers in pinot client

2019-05-30 Thread GitBox
codecov-io edited a comment on issue #4243: Add support for passing headers in 
pinot client
URL: https://github.com/apache/incubator-pinot/pull/4243#issuecomment-496501504
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4243?src=pr=h1) 
Report
   > Merging 
[#4243](https://codecov.io/gh/apache/incubator-pinot/pull/4243?src=pr=desc) 
into 
[master](https://codecov.io/gh/apache/incubator-pinot/commit/45a8430bc3d981b44e2d8e242c75480d7dc593e6?src=pr=desc)
 will **increase** coverage by `0.08%`.
   > The diff coverage is `71.42%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-pinot/pull/4243/graphs/tree.svg?width=650=4ibza2ugkz=150=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4243?src=pr=tree)
   
   ```diff
   @@ Coverage Diff  @@
   ## master#4243  +/-   ##
   
   + Coverage 67.24%   67.32%   +0.08% 
 Complexity   20   20  
   
 Files  1041 1042   +1 
 Lines 5151451543  +29 
 Branches   7216 7221   +5 
   
   + Hits  3464034703  +63 
   + Misses1450614475  -31 
   + Partials   2368 2365   -3
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4243?src=pr=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[...ache/pinot/client/PinotClientTransportFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jbGllbnQvUGlub3RDbGllbnRUcmFuc3BvcnRGYWN0b3J5LmphdmE=)
 | `100% <100%> (ø)` | `0 <0> (?)` | |
   | 
[...ient/JsonAsyncHttpPinotClientTransportFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jbGllbnQvSnNvbkFzeW5jSHR0cFBpbm90Q2xpZW50VHJhbnNwb3J0RmFjdG9yeS5qYXZh)
 | `66.66% <100%> (-33.34%)` | `0 <0> (ø)` | |
   | 
[...inot/client/JsonAsyncHttpPinotClientTransport.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jbGllbnQvSnNvbkFzeW5jSHR0cFBpbm90Q2xpZW50VHJhbnNwb3J0LmphdmE=)
 | `63.41% <62.5%> (-2.3%)` | `0 <0> (ø)` | |
   | 
[...ava/org/apache/pinot/client/ConnectionFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jbGllbnQvQ29ubmVjdGlvbkZhY3RvcnkuamF2YQ==)
 | `45.45% <75%> (+5.45%)` | `0 <0> (ø)` | :arrow_down: |
   | 
[...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==)
 | `50% <0%> (-50%)` | `0% <0%> (ø)` | |
   | 
[...ot/core/realtime/stream/PartitionCountFetcher.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9zdHJlYW0vUGFydGl0aW9uQ291bnRGZXRjaGVyLmphdmE=)
 | `54.16% <0%> (-20.84%)` | `0% <0%> (ø)` | |
   | 
[.../pinot/controller/ControllerLeadershipManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9Db250cm9sbGVyTGVhZGVyc2hpcE1hbmFnZXIuamF2YQ==)
 | `74.41% <0%> (-15.59%)` | `0% <0%> (ø)` | |
   | 
[...e/impl/dictionary/LongOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09uSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=)
 | `82.6% <0%> (-13.05%)` | `0% <0%> (ø)` | |
   | 
[.../impl/dictionary/LongOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09mZkhlYXBNdXRhYmxlRGljdGlvbmFyeS5qYXZh)
 | `83.92% <0%> (-10.72%)` | `0% <0%> (ø)` | |
   | 
[...roller/helix/core/PinotTableIdealStateBuilder.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90VGFibGVJZGVhbFN0YXRlQnVpbGRlci5qYXZh)
 | `80.45% <0%> (-4.6%)` | `0% <0%> (ø)` | |
   | ... and [33 
more](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4243?src=pr=continue).
   > **Legend** - [Click here to learn 

[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #4218: Add RealtimeConsumptionCatchupServiceCallback

2019-05-30 Thread GitBox
mcvsubbu commented on a change in pull request #4218: Add 
RealtimeConsumptionCatchupServiceCallback
URL: https://github.com/apache/incubator-pinot/pull/4218#discussion_r289082518
 
 

 ##
 File path: 
pinot-broker/src/main/java/org/apache/pinot/broker/broker/helix/HelixBrokerStarter.java
 ##
 @@ -237,16 +238,30 @@ public void start()
 .addPreConnectCallback(() -> 
brokerMetrics.addMeteredGlobalValue(BrokerMeter.HELIX_ZOOKEEPER_RECONNECTS, 
1L));
 
 // Register the service status handler
-LOGGER.info("Registering service status handler");
+registerServiceStatusHandler();
+
+LOGGER.info("Finish starting Pinot broker");
+  }
+
+  /**
+   * Fetches the resources to monitor and registers the {@link 
org.apache.pinot.common.utils.ServiceStatus.ServiceStatusCallback}s
+   */
+  private void registerServiceStatusHandler() {
+List resourcesToMonitor = new ArrayList<>(1);
+IdealState resourceIdealState = 
_helixAdmin.getResourceIdealState(_clusterName, Helix.BROKER_RESOURCE_INSTANCE);
+if (resourceIdealState != null && resourceIdealState.isEnabled()) {
 
 Review comment:
   If each service handler we want to have end up being empty, then yes. we do 
need to set one that always returns GOOD. Perhaps that can be the default 
setting for ServiceStatus.getServiceStatus ?


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


With regards,
Apache Git Services

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



[GitHub] [incubator-pinot] xiaohui-sun commented on a change in pull request #4250: SoC - Separate out Tuning from Translator

2019-05-30 Thread GitBox
xiaohui-sun commented on a change in pull request #4250: SoC - Separate out 
Tuning from Translator
URL: https://github.com/apache/incubator-pinot/pull/4250#discussion_r289081921
 
 

 ##
 File path: 
thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/DetectionPipeline.java
 ##
 @@ -90,22 +90,21 @@ private void initComponents() throws Exception {
 Map instancesMap = config.getComponents();
 Map componentSpecs = config.getComponentSpecs();
 if (componentSpecs != null) {
-  for (String componentName : componentSpecs.keySet()) {
-Map componentSpec = MapUtils.getMap(componentSpecs, 
componentName);
-if (!instancesMap.containsKey(componentName)){
-  instancesMap.put(componentName, createComponent(componentSpec));
+  for (String componentKey : componentSpecs.keySet()) {
+Map componentSpec = MapUtils.getMap(componentSpecs, 
componentKey);
+if (!instancesMap.containsKey(componentKey)){
+  instancesMap.put(componentKey, createComponent(componentSpec));
 }
   }
 
-  for (String componentName : componentSpecs.keySet()) {
-Map componentSpec = MapUtils.getMap(componentSpecs, 
componentName);
+  for (String componentKey : componentSpecs.keySet()) {
 
 Review comment:
   Sure. Thanks for the details.


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


With regards,
Apache Git Services

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



[GitHub] [incubator-pinot] npawar commented on a change in pull request #4218: Add RealtimeConsumptionCatchupServiceCallback

2019-05-30 Thread GitBox
npawar commented on a change in pull request #4218: Add 
RealtimeConsumptionCatchupServiceCallback
URL: https://github.com/apache/incubator-pinot/pull/4218#discussion_r289081225
 
 

 ##
 File path: 
pinot-broker/src/main/java/org/apache/pinot/broker/broker/helix/HelixBrokerStarter.java
 ##
 @@ -237,16 +238,30 @@ public void start()
 .addPreConnectCallback(() -> 
brokerMetrics.addMeteredGlobalValue(BrokerMeter.HELIX_ZOOKEEPER_RECONNECTS, 
1L));
 
 // Register the service status handler
-LOGGER.info("Registering service status handler");
+registerServiceStatusHandler();
+
+LOGGER.info("Finish starting Pinot broker");
+  }
+
+  /**
+   * Fetches the resources to monitor and registers the {@link 
org.apache.pinot.common.utils.ServiceStatus.ServiceStatusCallback}s
+   */
+  private void registerServiceStatusHandler() {
+List resourcesToMonitor = new ArrayList<>(1);
+IdealState resourceIdealState = 
_helixAdmin.getResourceIdealState(_clusterName, Helix.BROKER_RESOURCE_INSTANCE);
+if (resourceIdealState != null && resourceIdealState.isEnabled()) {
 
 Review comment:
   Hmm yes, fixed. 
   Follow up question: We still want to set the ServiceStatusHandlers, even if 
resourcesToMonitor is empty right? That way the health checks will get response 
GOOD instead of having no service handlers?


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: Unify controller base url in integration test (#4257)

2019-05-30 Thread 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 951a781  Unify controller base url in integration test (#4257)
951a781 is described below

commit 951a781cd044fa63476a917b2fede1d29cd2e0d0
Author: Jialiang Li 
AuthorDate: Thu May 30 10:02:41 2019 -0700

Unify controller base url in integration test (#4257)
---
 .../integration/tests/NewConfigApplyIntegrationTest.java | 12 ++--
 .../integration/tests/PinotURIUploadIntegrationTest.java |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git 
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/NewConfigApplyIntegrationTest.java
 
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/NewConfigApplyIntegrationTest.java
index 8f0045d..aee09f0 100644
--- 
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/NewConfigApplyIntegrationTest.java
+++ 
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/NewConfigApplyIntegrationTest.java
@@ -65,32 +65,32 @@ public class NewConfigApplyIntegrationTest extends 
BaseClusterIntegrationTest {
 FileUtils.copyDirectoryToDirectory(configFiles.get(2).getParentFile(), new 
File("."));
 
 // Create a new table using the command line tools without a profile
-runAdminCommand("ApplyTableConfig", "-controllerUrl", 
"http://localhost:8998/;, "-tableConfigFile",
+runAdminCommand("ApplyTableConfig", "-controllerUrl", 
_controllerBaseApiUrl, "-tableConfigFile",
 configFiles.get(0).getAbsolutePath());
 
 // Check that the table exists
-String tableConfiguration = 
sendGetRequestRaw("http://localhost:8998/v2/tables/mytable;);
+String tableConfiguration = sendGetRequestRaw(_controllerBaseApiUrl + 
"/v2/tables/mytable");
 CombinedConfig tableConfigurationObject = 
CombinedConfigLoader.loadCombinedConfig(tableConfiguration);
 
assertEquals(tableConfigurationObject.getOfflineTableConfig().getTableName(), 
"mytable_OFFLINE");
 
assertEquals(tableConfigurationObject.getOfflineTableConfig().getValidationConfig().getReplicationNumber(),
 3);
 
 // Update the table using a configuration profile
-runAdminCommand("ApplyTableConfig", "-controllerUrl", 
"http://localhost:8998/;, "-tableConfigFile",
+runAdminCommand("ApplyTableConfig", "-controllerUrl", 
_controllerBaseApiUrl, "-tableConfigFile",
 configFiles.get(1).getAbsolutePath(), "-profile", "test2");
 
 // Check that the table is updated
-tableConfiguration = 
sendGetRequestRaw("http://localhost:8998/v2/tables/mytable;);
+tableConfiguration = sendGetRequestRaw(_controllerBaseApiUrl + 
"/v2/tables/mytable");
 tableConfigurationObject = 
CombinedConfigLoader.loadCombinedConfig(tableConfiguration);
 
assertEquals(tableConfigurationObject.getOfflineTableConfig().getTableName(), 
"mytable_OFFLINE");
 
assertEquals(tableConfigurationObject.getOfflineTableConfig().getValidationConfig().getReplicationNumber(),
 4);
 
assertEquals(tableConfigurationObject.getOfflineTableConfig().getIndexingConfig().getLoadMode(),
 "MMAP");
 
 // Update the table using a configuration profile
-runAdminCommand("ApplyTableConfig", "-controllerUrl", 
"http://localhost:8998/;, "-tableConfigFile",
+runAdminCommand("ApplyTableConfig", "-controllerUrl", 
_controllerBaseApiUrl, "-tableConfigFile",
 configFiles.get(0).getAbsolutePath(), "-profile", "test1");
 
 // Check that the table is updated
-tableConfiguration = 
sendGetRequestRaw("http://localhost:8998/v2/tables/mytable;);
+tableConfiguration = sendGetRequestRaw(_controllerBaseApiUrl + 
"/v2/tables/mytable");
 tableConfigurationObject = 
CombinedConfigLoader.loadCombinedConfig(tableConfiguration);
 
assertEquals(tableConfigurationObject.getOfflineTableConfig().getTableName(), 
"mytable_OFFLINE");
 
assertEquals(tableConfigurationObject.getOfflineTableConfig().getValidationConfig().getReplicationNumber(),
 3);
diff --git 
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/PinotURIUploadIntegrationTest.java
 
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/PinotURIUploadIntegrationTest.java
index bfdb354..65cf969 100644
--- 
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/PinotURIUploadIntegrationTest.java
+++ 
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/PinotURIUploadIntegrationTest.java
@@ -270,7 +270,7 @@ public class PinotURIUploadIntegrationTest extends 
BaseClusterIntegrationTestSet
   private List getAllSegments(String tablename)
   throws IOException {
 List allSegments = new ArrayList<>();
-HttpHost controllerHttpHost = new HttpHost("localhost", 8998);
+HttpHost controllerHttpHost = 

[GitHub] [incubator-pinot] jackjlli merged pull request #4257: Unify controller base url in integration test

2019-05-30 Thread GitBox
jackjlli merged pull request #4257: Unify controller base url in integration 
test
URL: https://github.com/apache/incubator-pinot/pull/4257
 
 
   


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 a change in pull request #4243: Add support for passing headers in pinot client

2019-05-30 Thread GitBox
sunithabeeram commented on a change in pull request #4243: Add support for 
passing headers in pinot client
URL: https://github.com/apache/incubator-pinot/pull/4243#discussion_r289029741
 
 

 ##
 File path: 
pinot-api/src/main/java/org/apache/pinot/client/PinotClientTransportFactory.java
 ##
 @@ -18,9 +18,11 @@
  */
 package org.apache.pinot.client;
 
+import java.util.Map;
+
 /**
  * Factory for client transports.
  */
 interface PinotClientTransportFactory {
-  PinotClientTransport buildTransport();
+  PinotClientTransport buildTransport(Map headers);
 
 Review comment:
   The code structure seems a bit weird to me. If ConnectionFactory is the one 
that can be extended, then shouldn't that actually be an interface and made 
Public.Stable? Its odd that we have a specific implementation there and expect 
clean extensibility.
   
   Given the lack of clarity, it would be great if the javadocs for 
ConnectionFactory and PinotClientConnectionFactory be updated to expand on how 
they should be used - ie, what should change when new transports are 
considered/added. @kishoreg, hope you can provide appropriate input here for 
Jitendra.


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] jitendratheta commented on a change in pull request #4243: Add support for passing headers in pinot client

2019-05-30 Thread GitBox
jitendratheta commented on a change in pull request #4243: Add support for 
passing headers in pinot client
URL: https://github.com/apache/incubator-pinot/pull/4243#discussion_r288968381
 
 

 ##
 File path: 
pinot-api/src/main/java/org/apache/pinot/client/PinotClientTransportFactory.java
 ##
 @@ -18,9 +18,11 @@
  */
 package org.apache.pinot.client;
 
+import java.util.Map;
+
 /**
  * Factory for client transports.
  */
 interface PinotClientTransportFactory {
-  PinotClientTransport buildTransport();
+  PinotClientTransport buildTransport(Map headers);
 
 Review comment:
   @sunithabeeram @mayankshriv Can this thread be resolved if you are ok with 
the changes?


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


With regards,
Apache Git Services

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



[GitHub] [incubator-pinot] codecov-io edited a comment on issue #4243: Add support for passing headers in pinot client

2019-05-30 Thread GitBox
codecov-io edited a comment on issue #4243: Add support for passing headers in 
pinot client
URL: https://github.com/apache/incubator-pinot/pull/4243#issuecomment-496501504
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4243?src=pr=h1) 
Report
   > Merging 
[#4243](https://codecov.io/gh/apache/incubator-pinot/pull/4243?src=pr=desc) 
into 
[master](https://codecov.io/gh/apache/incubator-pinot/commit/45a8430bc3d981b44e2d8e242c75480d7dc593e6?src=pr=desc)
 will **increase** coverage by `0.05%`.
   > The diff coverage is `71.42%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-pinot/pull/4243/graphs/tree.svg?width=650=4ibza2ugkz=150=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4243?src=pr=tree)
   
   ```diff
   @@ Coverage Diff @@
   ## master   #4243  +/-   ##
   ===
   + Coverage 67.24%   67.3%   +0.05% 
 Complexity   20  20  
   ===
 Files  10411042   +1 
 Lines 51514   51543  +29 
 Branches   72167221   +5 
   ===
   + Hits  34640   34689  +49 
   + Misses14506   14494  -12 
   + Partials   23682360   -8
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4243?src=pr=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[...ache/pinot/client/PinotClientTransportFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jbGllbnQvUGlub3RDbGllbnRUcmFuc3BvcnRGYWN0b3J5LmphdmE=)
 | `100% <100%> (ø)` | `0 <0> (?)` | |
   | 
[...ient/JsonAsyncHttpPinotClientTransportFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jbGllbnQvSnNvbkFzeW5jSHR0cFBpbm90Q2xpZW50VHJhbnNwb3J0RmFjdG9yeS5qYXZh)
 | `66.66% <100%> (-33.34%)` | `0 <0> (ø)` | |
   | 
[...inot/client/JsonAsyncHttpPinotClientTransport.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jbGllbnQvSnNvbkFzeW5jSHR0cFBpbm90Q2xpZW50VHJhbnNwb3J0LmphdmE=)
 | `63.41% <62.5%> (-2.3%)` | `0 <0> (ø)` | |
   | 
[...ava/org/apache/pinot/client/ConnectionFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jbGllbnQvQ29ubmVjdGlvbkZhY3RvcnkuamF2YQ==)
 | `45.45% <75%> (+5.45%)` | `0 <0> (ø)` | :arrow_down: |
   | 
[...r/validation/RealtimeSegmentValidationManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci92YWxpZGF0aW9uL1JlYWx0aW1lU2VnbWVudFZhbGlkYXRpb25NYW5hZ2VyLmphdmE=)
 | `43.33% <0%> (-36.67%)` | `0% <0%> (ø)` | |
   | 
[...apache/pinot/common/metrics/ValidationMetrics.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9WYWxpZGF0aW9uTWV0cmljcy5qYXZh)
 | `20.28% <0%> (-23.19%)` | `0% <0%> (ø)` | |
   | 
[...ot/core/realtime/stream/PartitionCountFetcher.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9zdHJlYW0vUGFydGl0aW9uQ291bnRGZXRjaGVyLmphdmE=)
 | `54.16% <0%> (-20.84%)` | `0% <0%> (ø)` | |
   | 
[.../pinot/controller/ControllerLeadershipManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9Db250cm9sbGVyTGVhZGVyc2hpcE1hbmFnZXIuamF2YQ==)
 | `74.41% <0%> (-15.59%)` | `0% <0%> (ø)` | |
   | 
[.../impl/dictionary/LongOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09mZkhlYXBNdXRhYmxlRGljdGlvbmFyeS5qYXZh)
 | `89.28% <0%> (-5.36%)` | `0% <0%> (ø)` | |
   | 
[...roller/helix/core/PinotTableIdealStateBuilder.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90VGFibGVJZGVhbFN0YXRlQnVpbGRlci5qYXZh)
 | `80.45% <0%> (-4.6%)` | `0% <0%> (ø)` | |
   | ... and [38 
more](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4243?src=pr=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute  

[GitHub] [incubator-pinot] codecov-io edited a comment on issue #4257: Unify controller base url in integration test

2019-05-30 Thread GitBox
codecov-io edited a comment on issue #4257: Unify controller base url in 
integration test
URL: https://github.com/apache/incubator-pinot/pull/4257#issuecomment-497194131
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4257?src=pr=h1) 
Report
   > Merging 
[#4257](https://codecov.io/gh/apache/incubator-pinot/pull/4257?src=pr=desc) 
into 
[master](https://codecov.io/gh/apache/incubator-pinot/commit/3ce335a2f815d4f0818a60a3f6397ce7f5f26387?src=pr=desc)
 will **increase** coverage by `7.09%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-pinot/pull/4257/graphs/tree.svg?width=650=4ibza2ugkz=150=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4257?src=pr=tree)
   
   ```diff
   @@ Coverage Diff  @@
   ## master#4257  +/-   ##
   
   + Coverage 60.35%   67.45%   +7.09% 
 Complexity   20   20  
   
 Files  1165 1041 -124 
 Lines 5853551534-7001 
 Branches   8144 7220 -924 
   
   - Hits  3532934762 -567 
   + Misses2076414406-6358 
   + Partials   2442 2366  -76
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4257?src=pr=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[...he/pinot/core/query/pruner/ValidSegmentPruner.java](https://codecov.io/gh/apache/incubator-pinot/pull/4257/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9wcnVuZXIvVmFsaWRTZWdtZW50UHJ1bmVyLmphdmE=)
 | `57.14% <0%> (-28.58%)` | `0% <0%> (ø)` | |
   | 
[.../impl/dictionary/FloatOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4257/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPbkhlYXBNdXRhYmxlRGljdGlvbmFyeS5qYXZh)
 | `89.13% <0%> (-6.53%)` | `0% <0%> (ø)` | |
   | 
[...elix/TimeboundaryRefreshMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/4257/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L1RpbWVib3VuZGFyeVJlZnJlc2hNZXNzYWdlSGFuZGxlckZhY3RvcnkuamF2YQ==)
 | `79.59% <0%> (-6.13%)` | `0% <0%> (ø)` | |
   | 
[...mpl/dictionary/DoubleOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4257/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT2ZmSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=)
 | `73.21% <0%> (-5.36%)` | `0% <0%> (ø)` | |
   | 
[...ata/manager/realtime/RealtimeTableDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4257/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVUYWJsZURhdGFNYW5hZ2VyLmphdmE=)
 | `41.17% <0%> (-1.48%)` | `0% <0%> (ø)` | |
   | 
[...g/apache/pinot/common/metrics/AbstractMetrics.java](https://codecov.io/gh/apache/incubator-pinot/pull/4257/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9BYnN0cmFjdE1ldHJpY3MuamF2YQ==)
 | `76% <0%> (-1.34%)` | `0% <0%> (ø)` | |
   | 
[...ation/function/MinMaxRangeAggregationFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/4257/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9NaW5NYXhSYW5nZUFnZ3JlZ2F0aW9uRnVuY3Rpb24uamF2YQ==)
 | `72.09% <0%> (-1.17%)` | `0% <0%> (ø)` | |
   | 
[...core/query/executor/ServerQueryExecutorV1Impl.java](https://codecov.io/gh/apache/incubator-pinot/pull/4257/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9leGVjdXRvci9TZXJ2ZXJRdWVyeUV4ZWN1dG9yVjFJbXBsLmphdmE=)
 | `81.37% <0%> (-0.69%)` | `0% <0%> (ø)` | |
   | 
[...ot/common/protocols/SegmentCompletionProtocol.java](https://codecov.io/gh/apache/incubator-pinot/pull/4257/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcHJvdG9jb2xzL1NlZ21lbnRDb21wbGV0aW9uUHJvdG9jb2wuamF2YQ==)
 | `92.48% <0%> (-0.58%)` | `0% <0%> (ø)` | |
   | 
[...a/org/apache/pinot/core/common/DataBlockCache.java](https://codecov.io/gh/apache/incubator-pinot/pull/4257/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9jb21tb24vRGF0YUJsb2NrQ2FjaGUuamF2YQ==)
 | `79.38% <0%> (ø)` | `0% <0%> (ø)` | :arrow_down: |
   | ... and [147 
more](https://codecov.io/gh/apache/incubator-pinot/pull/4257/diff?src=pr=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4257?src=pr=continue).
   > **Legend** - [Click here to learn 

[GitHub] [incubator-pinot] codecov-io edited a comment on issue #4243: Add support for passing headers in pinot client

2019-05-30 Thread GitBox
codecov-io edited a comment on issue #4243: Add support for passing headers in 
pinot client
URL: https://github.com/apache/incubator-pinot/pull/4243#issuecomment-496501504
 
 
   # 
[Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4243?src=pr=h1) 
Report
   > Merging 
[#4243](https://codecov.io/gh/apache/incubator-pinot/pull/4243?src=pr=desc) 
into 
[master](https://codecov.io/gh/apache/incubator-pinot/commit/45a8430bc3d981b44e2d8e242c75480d7dc593e6?src=pr=desc)
 will **increase** coverage by `0.04%`.
   > The diff coverage is `71.42%`.
   
   [![Impacted file tree 
graph](https://codecov.io/gh/apache/incubator-pinot/pull/4243/graphs/tree.svg?width=650=4ibza2ugkz=150=pr)](https://codecov.io/gh/apache/incubator-pinot/pull/4243?src=pr=tree)
   
   ```diff
   @@ Coverage Diff  @@
   ## master#4243  +/-   ##
   
   + Coverage 67.24%   67.28%   +0.04% 
 Complexity   20   20  
   
 Files  1041 1042   +1 
 Lines 5151451543  +29 
 Branches   7216 7221   +5 
   
   + Hits  3464034683  +43 
   + Misses1450614495  -11 
   + Partials   2368 2365   -3
   ```
   
   
   | [Impacted 
Files](https://codecov.io/gh/apache/incubator-pinot/pull/4243?src=pr=tree) | 
Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | 
[...ache/pinot/client/PinotClientTransportFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jbGllbnQvUGlub3RDbGllbnRUcmFuc3BvcnRGYWN0b3J5LmphdmE=)
 | `100% <100%> (ø)` | `0 <0> (?)` | |
   | 
[...ient/JsonAsyncHttpPinotClientTransportFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jbGllbnQvSnNvbkFzeW5jSHR0cFBpbm90Q2xpZW50VHJhbnNwb3J0RmFjdG9yeS5qYXZh)
 | `66.66% <100%> (-33.34%)` | `0 <0> (ø)` | |
   | 
[...inot/client/JsonAsyncHttpPinotClientTransport.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jbGllbnQvSnNvbkFzeW5jSHR0cFBpbm90Q2xpZW50VHJhbnNwb3J0LmphdmE=)
 | `63.41% <62.5%> (-2.3%)` | `0 <0> (ø)` | |
   | 
[...ava/org/apache/pinot/client/ConnectionFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jbGllbnQvQ29ubmVjdGlvbkZhY3RvcnkuamF2YQ==)
 | `45.45% <75%> (+5.45%)` | `0 <0> (ø)` | :arrow_down: |
   | 
[...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==)
 | `75% <0%> (-25%)` | `0% <0%> (ø)` | |
   | 
[...ot/core/realtime/stream/PartitionCountFetcher.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9zdHJlYW0vUGFydGl0aW9uQ291bnRGZXRjaGVyLmphdmE=)
 | `54.16% <0%> (-20.84%)` | `0% <0%> (ø)` | |
   | 
[.../pinot/controller/ControllerLeadershipManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9Db250cm9sbGVyTGVhZGVyc2hpcE1hbmFnZXIuamF2YQ==)
 | `74.41% <0%> (-15.59%)` | `0% <0%> (ø)` | |
   | 
[...e/impl/dictionary/LongOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09uSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=)
 | `89.13% <0%> (-6.53%)` | `0% <0%> (ø)` | |
   | 
[...roller/helix/core/PinotTableIdealStateBuilder.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90VGFibGVJZGVhbFN0YXRlQnVpbGRlci5qYXZh)
 | `80.45% <0%> (-4.6%)` | `0% <0%> (ø)` | |
   | 
[...nsport/netty/PooledNettyClientResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree#diff-cGlub3QtdHJhbnNwb3J0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC90cmFuc3BvcnQvbmV0dHkvUG9vbGVkTmV0dHlDbGllbnRSZXNvdXJjZU1hbmFnZXIuamF2YQ==)
 | `85.41% <0%> (-4.17%)` | `0% <0%> (ø)` | |
   | ... and [31 
more](https://codecov.io/gh/apache/incubator-pinot/pull/4243/diff?src=pr=tree-more)
 | |
   
   --
   
   [Continue to review full report at 
Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/4243?src=pr=continue).
   > **Legend** - [Click here to learn 
more](https://docs.codecov.io/docs/codecov-delta)

[GitHub] [incubator-pinot] kishoreg commented on a change in pull request #4243: Add support for passing headers in pinot client

2019-05-30 Thread GitBox
kishoreg commented on a change in pull request #4243: Add support for passing 
headers in pinot client
URL: https://github.com/apache/incubator-pinot/pull/4243#discussion_r288871635
 
 

 ##
 File path: 
pinot-api/src/main/java/org/apache/pinot/client/PinotClientTransportFactory.java
 ##
 @@ -18,9 +18,11 @@
  */
 package org.apache.pinot.client;
 
+import java.util.Map;
+
 /**
  * Factory for client transports.
  */
 interface PinotClientTransportFactory {
-  PinotClientTransport buildTransport();
+  PinotClientTransport buildTransport(Map headers);
 
 Review comment:
   I don't see anyone extending PinotClientTransportFactory outside of Pinot 
code. I am fine with making it private


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] jitendratheta commented on a change in pull request #4243: Add support for passing headers in pinot client

2019-05-30 Thread GitBox
jitendratheta commented on a change in pull request #4243: Add support for 
passing headers in pinot client
URL: https://github.com/apache/incubator-pinot/pull/4243#discussion_r288869748
 
 

 ##
 File path: 
pinot-api/src/main/java/org/apache/pinot/client/PinotClientTransportFactory.java
 ##
 @@ -18,9 +18,11 @@
  */
 package org.apache.pinot.client;
 
+import java.util.Map;
+
 /**
  * Factory for client transports.
  */
 interface PinotClientTransportFactory {
-  PinotClientTransport buildTransport();
+  PinotClientTransport buildTransport(Map headers);
 
 Review comment:
   Or else I was thinking why not to keep the annotation as Private for class 
`PinotClientTransportFactory` as it is not exposed outside as public?


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 #4257: Unify controller base url in integration test

2019-05-30 Thread GitBox
jackjlli commented on a change in pull request #4257: Unify controller base url 
in integration test
URL: https://github.com/apache/incubator-pinot/pull/4257#discussion_r288868602
 
 

 ##
 File path: 
pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
 ##
 @@ -57,7 +57,7 @@
 public abstract class ControllerTest {
   public static final String LOCAL_HOST = "localhost";
 
-  private static final int DEFAULT_CONTROLLER_PORT = 8998;
+  private static final int DEFAULT_CONTROLLER_PORT = 9550;
 
 Review comment:
   Some background process is running and occupying the port number. It's been 
addressed. 


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