[GitHub] [beam] ajamato commented on a change in pull request #15329: [BEAM-12755] Stop throwing exceptions during formatting and calculation of a percentile

[GitBox <gi...@apache.org>]

ajamato commented on a change in pull request #15329:
URL: https://github.com/apache/beam/pull/15329#discussion_r688682516



##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/util/HistogramData.java
##########
@@ -196,7 +189,7 @@ public double p50() {
   private synchronized double getLinearInterpolation(double percentile) {
     long totalNumOfRecords = getTotalCount();
     if (totalNumOfRecords == 0) {

Review 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 ?

##########
File path: sdks/java/core/src/main/java/org/apache/beam/sdk/util/HistogramData.java
##########
@@ -196,7 +189,7 @@ public double p50() {
   private synchronized double getLinearInterpolation(double percentile) {
     long totalNumOfRecords = getTotalCount();
     if (totalNumOfRecords == 0) {
-      throw new RuntimeException("histogram has no record.");

Review 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.




-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org