You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "tibrewalpratik17 (via GitHub)" <gi...@apache.org> on 2024/02/14 13:03:20 UTC
[PR] Fix bug in logging in UpsertCompaction task [pinot]
tibrewalpratik17 opened a new pull request, #12419:
URL: https://github.com/apache/pinot/pull/12419
label:
`bugfix`
At present, the log message comes like:
```
Unable to get validDocIdsMetadata from all servers. Expected: 0, Actual: 1
```
This is wrong due to following reasons:
- In the loop, we are iterating over the responses of all server-requests, and incrementing segmentCount by 1. Should be serverCount.
- We are using the above wrong segmentCount value with expected segmentNames passed value.
Adding both logs for unequal segment count and unequal server count to improve debuggability.
Also renaming all logs to have "validDocIds" for easier debugging. At present, some logs had "valid doc ids" and some had "validDocIds".
cc @snleee
--
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
Re: [PR] Fix bug in logging in UpsertCompaction task [pinot]
Posted by "tibrewalpratik17 (via GitHub)" <gi...@apache.org>.
tibrewalpratik17 closed pull request #12419: Fix bug in logging in UpsertCompaction task
URL: https://github.com/apache/pinot/pull/12419
--
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
Re: [PR] Fix bug in logging in UpsertCompaction task [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #12419:
URL: https://github.com/apache/pinot/pull/12419#issuecomment-1943807503
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/12419?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: `16 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`e1e22bd`)](https://app.codecov.io/gh/apache/pinot/commit/e1e22bd4edac5794f8bea104bf43afa43f551641?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 61.74% compared to head [(`7ef221f`)](https://app.codecov.io/gh/apache/pinot/pull/12419?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 61.77%.
> Report is 4 commits behind head on master.
| [Files](https://app.codecov.io/gh/apache/pinot/pull/12419?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...t/controller/util/ServerSegmentMetadataReader.java](https://app.codecov.io/gh/apache/pinot/pull/12419?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL1NlcnZlclNlZ21lbnRNZXRhZGF0YVJlYWRlci5qYXZh) | 0.00% | [10 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12419?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...l/indexsegment/immutable/ImmutableSegmentImpl.java](https://app.codecov.io/gh/apache/pinot/pull/12419?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pbmRleHNlZ21lbnQvaW1tdXRhYmxlL0ltbXV0YWJsZVNlZ21lbnRJbXBsLmphdmE=) | 28.57% | [5 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12419?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...oller/api/resources/PinotTableRestletResource.java](https://app.codecov.io/gh/apache/pinot/pull/12419?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFibGVSZXN0bGV0UmVzb3VyY2UuamF2YQ==) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12419?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #12419 +/- ##
============================================
+ Coverage 61.74% 61.77% +0.03%
- Complexity 207 1155 +948
============================================
Files 2436 2436
Lines 133089 133103 +14
Branches 20615 20618 +3
============================================
+ Hits 82172 82226 +54
+ Misses 44878 44844 -34
+ Partials 6039 6033 -6
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/12419/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/12419/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/12419/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (ø)` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/12419/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (ø)` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/12419/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (ø)` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/12419/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.70% <11.11%> (+0.01%)` | :arrow_up: |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/12419/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.64% <11.11%> (+0.02%)` | :arrow_up: |
| [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/12419/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.76% <11.11%> (+0.03%)` | :arrow_up: |
| [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/12419/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.59% <11.11%> (+0.01%)` | :arrow_up: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/12419/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.77% <11.11%> (+0.03%)` | :arrow_up: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/12419/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `61.77% <11.11%> (+0.03%)` | :arrow_up: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/12419/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.90% <0.00%> (+0.01%)` | :arrow_up: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/12419/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `27.75% <11.11%> (+0.02%)` | :arrow_up: |
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=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/12419?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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
Re: [PR] Fix bug in logging in UpsertCompaction task [pinot]
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on code in PR #12419:
URL: https://github.com/apache/pinot/pull/12419#discussion_r1491707054
##########
pinot-controller/src/main/java/org/apache/pinot/controller/util/ServerSegmentMetadataReader.java:
##########
@@ -269,12 +269,18 @@ public List<ValidDocIdsMetadataInfo> getValidDocIdsMetadataFromServer(String tab
serverURLsAndBodies.size());
}
- if (segmentNames != null && returnedSegmentsCount != segmentNames.size()) {
- LOGGER.error("Unable to get validDocIdsMetadata from all servers. Expected: {}, Actual: {}", segmentNames.size(),
- returnedSegmentsCount);
+ if (returnedServersCount != serverURLsAndBodies.size()) {
+ LOGGER.error("Unable to get validDocIdsMetadata from all servers. Expected: {}, Actual: {}",
+ serverURLsAndBodies.size(), returnedServersCount);
}
- LOGGER.info("Retrieved valid doc id metadata for {} segments from {} servers.", returnedSegmentsCount,
- serverURLsAndBodies.size());
+
+ if (segmentNames != null && !segmentNames.isEmpty() && segmentNames.size() != validDocIdsMetadataInfos.size()) {
+ LOGGER.error("Unable to get validDocIdsMetadata for all segments. Expected: {}, Actual: {}",
+ serverURLsAndBodies.size(), returnedServersCount);
Review Comment:
bug?
```
serverURLsAndBodies.size(), returnedServersCount);
```
--
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