[
https://issues.apache.org/jira/browse/GOBBLIN-2187?focusedWorklogId=951022&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-951022
]
ASF GitHub Bot logged work on GOBBLIN-2187:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 06/Jan/25 12:37
Start Date: 06/Jan/25 12:37
Worklog Time Spent: 10m
Work Description: phet commented on code in PR #4090:
URL: https://github.com/apache/gobblin/pull/4090#discussion_r1904099321
##########
gobblin-temporal/src/main/java/org/apache/gobblin/temporal/ddm/activity/impl/GenerateWorkUnitsImpl.java:
##########
@@ -95,7 +95,11 @@ public WorkUnitsSizeSummary asSizeSummary(int numQuantiles) {
private static List<Double> getQuantiles(TDigest digest, int numQuantiles)
{
List<Double> quantileMinSizes = Lists.newArrayList();
for (int i = 1; i <= numQuantiles; i++) {
- quantileMinSizes.add(digest.quantile((i * 1.0) / numQuantiles));
+ double currQuantileMinSize = digest.quantile((i * 1.0) / numQuantiles);
+ if (Double.isNaN(currQuantileMinSize)) {
+ currQuantileMinSize = 0.0;
+ }
+ quantileMinSizes.add(currQuantileMinSize);
Review Comment:
aren't we protected from DIV-0 by L83?
```
Preconditions.checkArgument(numQuantiles > 0, "numQuantiles must be > 0");
```
##########
gobblin-temporal/src/test/java/org/apache/gobblin/temporal/ddm/activity/impl/GenerateWorkUnitsImplTest.java:
##########
@@ -174,6 +174,31 @@ public void testDigestWorkUnitsSize() {
400.0 }); // with only one 20-quantile remaining, non-MWU [5]
completes the "100-percentile" (all WUs)
}
+ @Test
+ public void testDigestWorkUnitsSizeWithEmptyWorkUnits() {
+ List<WorkUnit> workUnits = new ArrayList<>();
+ GenerateWorkUnitsImpl.WorkUnitsSizeDigest wuSizeDigest =
GenerateWorkUnitsImpl.digestWorkUnitsSize(workUnits);
+
+ Assert.assertEquals(wuSizeDigest.getTotalSize(), 0L);
+ Assert.assertEquals(wuSizeDigest.getTopLevelWorkUnitsSizeDigest().size(),
0);
+
Assert.assertEquals(wuSizeDigest.getConstituentWorkUnitsSizeDigest().size(), 0);
+
+ int numQuantilesDesired = 10;
+ WorkUnitsSizeSummary wuSizeInfo =
wuSizeDigest.asSizeSummary(numQuantilesDesired);
+ Assert.assertEquals(wuSizeInfo.getTotalSize(), 0L);
+ Assert.assertEquals(wuSizeInfo.getTopLevelWorkUnitsCount(), 0);
+ Assert.assertEquals(wuSizeInfo.getConstituentWorkUnitsCount(), 0);
+ Assert.assertEquals(wuSizeInfo.getQuantilesCount(), numQuantilesDesired);
+ Assert.assertEquals(wuSizeInfo.getQuantilesWidth(), 1.0 /
numQuantilesDesired);
+ Assert.assertEquals(wuSizeInfo.getTopLevelQuantilesMinSizes().size(),
numQuantilesDesired); // same as `asSizeInfo` param
+ Assert.assertEquals(wuSizeInfo.getConstituentQuantilesMinSizes().size(),
numQuantilesDesired); // same as `asSizeInfo` param
Review Comment:
out-of-date - should read:
```
// same as `asSizeSummary` param
```
please also correct on L151-152
Issue Time Tracking
-------------------
Worklog Id: (was: 951022)
Remaining Estimate: 0h
Time Spent: 10m
> Fix NaN issue while generating WUSizeSummary
> --------------------------------------------
>
> Key: GOBBLIN-2187
> URL: https://issues.apache.org/jira/browse/GOBBLIN-2187
> Project: Apache Gobblin
> Issue Type: Bug
> Components: gobblin-core
> Reporter: Vivek Rai
> Assignee: Abhishek Tiwari
> Priority: Major
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Recently GoT executions were enhanced to return WUSizeSummary with more info,
> but in case workunits are zero those have NaN value due to divide by zero
> case,
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
