You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/08/23 22:20:04 UTC
[GitHub] [pinot] Jackie-Jiang opened a new pull request, #9271: Fix message count
Jackie-Jiang opened a new pull request, #9271:
URL: https://github.com/apache/pinot/pull/9271
Fix a bug introduced in #9201
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #9271: Fix message count
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #9271:
URL: https://github.com/apache/pinot/pull/9271#issuecomment-1224975569
# [Codecov](https://codecov.io/gh/apache/pinot/pull/9271?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#9271](https://codecov.io/gh/apache/pinot/pull/9271?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5a48c82) into [master](https://codecov.io/gh/apache/pinot/commit/e14fba0c2b158da4d6a85dfa7a91a3ad5932bb3e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e14fba0) will **increase** coverage by `3.50%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 5a48c82 differs from pull request most recent head 0065990. Consider uploading reports for the commit 0065990 to get more accurate results
```diff
@@ Coverage Diff @@
## master #9271 +/- ##
============================================
+ Coverage 63.59% 67.10% +3.50%
- Complexity 4740 4814 +74
============================================
Files 1809 1381 -428
Lines 96993 71991 -25002
Branches 14832 11524 -3308
============================================
- Hits 61686 48306 -13380
+ Misses 30756 20179 -10577
+ Partials 4551 3506 -1045
```
| Flag | Coverage Δ | |
|---|---|---|
| unittests1 | `67.10% <100.00%> (-0.01%)` | :arrow_down: |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/9271?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...manager/realtime/LLRealtimeSegmentDataManager.java](https://codecov.io/gh/apache/pinot/pull/9271/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvTExSZWFsdGltZVNlZ21lbnREYXRhTWFuYWdlci5qYXZh) | `53.39% <100.00%> (ø)` | |
| [...org/apache/pinot/core/common/ObjectSerDeUtils.java](https://codecov.io/gh/apache/pinot/pull/9271/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9jb21tb24vT2JqZWN0U2VyRGVVdGlscy5qYXZh) | `89.63% <0.00%> (-0.82%)` | :arrow_down: |
| [...ller/api/access/BasicAuthAccessControlFactory.java](https://codecov.io/gh/apache/pinot/pull/9271/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvYWNjZXNzL0Jhc2ljQXV0aEFjY2Vzc0NvbnRyb2xGYWN0b3J5LmphdmE=) | | |
| [...not/controller/tuner/TableConfigTunerRegistry.java](https://codecov.io/gh/apache/pinot/pull/9271/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci90dW5lci9UYWJsZUNvbmZpZ1R1bmVyUmVnaXN0cnkuamF2YQ==) | | |
| [...estion/batch/common/SegmentGenerationJobUtils.java](https://codecov.io/gh/apache/pinot/pull/9271/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1iYXRjaC1pbmdlc3Rpb24vcGlub3QtYmF0Y2gtaW5nZXN0aW9uLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcGx1Z2luL2luZ2VzdGlvbi9iYXRjaC9jb21tb24vU2VnbWVudEdlbmVyYXRpb25Kb2JVdGlscy5qYXZh) | | |
| [...on/tasks/mergerollup/MergeRollupTaskGenerator.java](https://codecov.io/gh/apache/pinot/pull/9271/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvbWVyZ2Vyb2xsdXAvTWVyZ2VSb2xsdXBUYXNrR2VuZXJhdG9yLmphdmE=) | | |
| [...oller/api/events/DefaultMetadataEventNotifier.java](https://codecov.io/gh/apache/pinot/pull/9271/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvZXZlbnRzL0RlZmF1bHRNZXRhZGF0YUV2ZW50Tm90aWZpZXIuamF2YQ==) | | |
| [...ender/data/generator/PatternSequenceGenerator.java](https://codecov.io/gh/apache/pinot/pull/9271/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9kYXRhL2dlbmVyYXRvci9QYXR0ZXJuU2VxdWVuY2VHZW5lcmF0b3IuamF2YQ==) | | |
| [...ontroller/api/resources/ConfigSuccessResponse.java](https://codecov.io/gh/apache/pinot/pull/9271/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL0NvbmZpZ1N1Y2Nlc3NSZXNwb25zZS5qYXZh) | | |
| [...ecommender/data/generator/RangeFloatGenerator.java](https://codecov.io/gh/apache/pinot/pull/9271/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9kYXRhL2dlbmVyYXRvci9SYW5nZUZsb2F0R2VuZXJhdG9yLmphdmE=) | | |
| ... and [420 more](https://codecov.io/gh/apache/pinot/pull/9271/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] sajjad-moradi commented on a diff in pull request #9271: Fix message count
Posted by GitBox <gi...@apache.org>.
sajjad-moradi commented on code in PR #9271:
URL: https://github.com/apache/pinot/pull/9271#discussion_r953174637
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java:
##########
@@ -498,8 +498,7 @@ protected boolean consumeLoop()
* otherwise
*/
private boolean processStreamEvents(MessageBatch messagesAndOffsets, long idlePipeSleepTimeMillis) {
-
- int messageCount = messagesAndOffsets.getUnfilteredMessageCount();
+ int messageCount = messagesAndOffsets.getMessageCount();
_rateLimiter.throttle(messageCount);
Review Comment:
Since we don't provide any end.offset when we fetch messages, the only filtered out message type is the null message. Since we don't do any processing with those messages, it's ok to not count them towards rate limiting (although we've actually fetched them).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang merged pull request #9271: Fix message count
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged PR #9271:
URL: https://github.com/apache/pinot/pull/9271
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #9271: Fix message count
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #9271:
URL: https://github.com/apache/pinot/pull/9271#discussion_r953156737
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java:
##########
@@ -498,8 +498,7 @@ protected boolean consumeLoop()
* otherwise
*/
private boolean processStreamEvents(MessageBatch messagesAndOffsets, long idlePipeSleepTimeMillis) {
-
- int messageCount = messagesAndOffsets.getUnfilteredMessageCount();
+ int messageCount = messagesAndOffsets.getMessageCount();
_rateLimiter.throttle(messageCount);
Review Comment:
@sajjad-moradi IMO the rate limiter should be applied to the filtered messages instead of unfiltered messages because the purpose of it is to limit the ingestion rate
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org