[BEAM-12755] Stop throwing exceptions during formatting and calculation of a percentile #15329
Conversation
…on of a percentile
|
Run Java PreCommit |
|
Run Java_Examples_Dataflow PreCommit |
|
Run Java_Examples_Dataflow_Java11 PreCommit |
| */ | ||
| private synchronized double getLinearInterpolation(double percentile) { | ||
| long totalNumOfRecords = getTotalCount(); | ||
| if (totalNumOfRecords == 0) { |
There was a problem hiding this comment.
Maybe this code shouldn't ever be called if totalNumOfRecords == 0
This feature is only used to print some info logging.
So ti could just print something else in this case
WDYT @ihji ?
| private synchronized double getLinearInterpolation(double percentile) { | ||
| long totalNumOfRecords = getTotalCount(); | ||
| if (totalNumOfRecords == 0) { | ||
| throw new RuntimeException("histogram has no record."); |
There was a problem hiding this comment.
Just curious. Did you decide to make this change because you saw this exception thrown in a user pipeline? (Its possible the code is designed to not invoke this method in this case). Just wondering if actual pipelines are hitting this?
Or did you just decide to make this change to make to remove the exception, make the code safer, etc.
There was a problem hiding this comment.
User pipelines were hitting this, b/189940287
|
Run Java PreCommit |
2 participants
String.format already uses HALF_UP rounding mode.
Also using NaN for an invalid value instead of throwing an exception which is difficult to expect users to work with.
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username).[BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replaceBEAM-XXXwith the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
ValidatesRunnercompliance status (on master branch)Examples testing status on various runners
Post-Commit SDK/Transform Integration Tests Status (on master branch)
Pre-Commit Tests Status (on master branch)
See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI.