Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Print message in TableSizeReader only when num of errors > 0 #2990

Merged
merged 2 commits into from Jul 26, 2018

Conversation

jackjlli
Copy link
Contributor

Currently it prints the message even when there's no error.
Adding this if statement can help reduce most of the noises in controller log.

@codecov-io
Copy link

codecov-io commented Jul 25, 2018

Codecov Report

Merging #2990 into master will decrease coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2990      +/-   ##
==========================================
- Coverage   69.93%   69.89%   -0.05%     
==========================================
  Files         913      913              
  Lines       43288    43289       +1     
  Branches     5917     5918       +1     
==========================================
- Hits        30274    30257      -17     
- Misses      10984    10999      +15     
- Partials     2030     2033       +3
Impacted Files Coverage Δ
...inkedin/pinot/controller/util/TableSizeReader.java 91.59% <100%> (+0.07%) ⬆️
...pinot/core/operator/docidsets/OrBlockDocIdSet.java 84.9% <0%> (-13.21%) ⬇️
...ore/query/scheduler/resources/ResourceManager.java 87.09% <0%> (-9.68%) ⬇️
...impl/dictionary/DoubleOnHeapMutableDictionary.java 68.88% <0%> (-6.67%) ⬇️
.../impl/dictionary/FloatOnHeapMutableDictionary.java 86.66% <0%> (-6.67%) ⬇️
...ng/builder/DefaultRealtimeRoutingTableBuilder.java 61.9% <0%> (-4.77%) ⬇️
...nsport/netty/PooledNettyClientResourceManager.java 85.41% <0%> (-4.17%) ⬇️
...om/linkedin/pinot/transport/netty/NettyServer.java 80% <0%> (-4%) ⬇️
...e/operator/dociditerators/BitmapDocIdIterator.java 60.71% <0%> (-3.58%) ⬇️
.../pinot/core/query/scheduler/PriorityScheduler.java 78.08% <0%> (-2.74%) ⬇️
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 688ba11...03bbf94. Read the comment docs.

LOGGER.info("Could not get size for segment {} from {} servers. Using segmentLevelMax {} to estimate the size",
segmentEntry.getKey(), errors, segmentLevelMax);
if (errors > 0) {
// at least one server reported size for this segment
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the comment outside of the if-block.

Copy link
Contributor

@sunithabeeram sunithabeeram left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Fix the minor comment.

@jackjlli jackjlli merged commit d5a4ad8 into master Jul 26, 2018
@jackjlli jackjlli deleted the update-table-size-reader-log branch July 26, 2018 01:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants