MSQ: Use long instead of double for estimatedRetainedBytes. (#13272)

Fixes a problem where, due to the inexactness of floating-point math, we
would potentially drift while tracking retained byte counts and run into
assertion failures in assertRetainedByteCountsAreTrackedCorrectly.
This commit is contained in:
Gian Merlino 2022-10-28 08:31:52 -07:00 committed by GitHub
parent 5429b9d764
commit 4f0145fb85
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 10 additions and 9 deletions

View File

@ -58,7 +58,7 @@ public class ClusterByStatisticsCollectorImpl implements ClusterByStatisticsColl
private final int maxRetainedBytes; private final int maxRetainedBytes;
private final int maxBuckets; private final int maxBuckets;
private double totalRetainedBytes; private long totalRetainedBytes;
private ClusterByStatisticsCollectorImpl( private ClusterByStatisticsCollectorImpl(
final ClusterBy clusterBy, final ClusterBy clusterBy,
@ -369,8 +369,8 @@ public class ClusterByStatisticsCollectorImpl implements ClusterByStatisticsColl
*/ */
private void downSample() private void downSample()
{ {
double newTotalRetainedBytes = totalRetainedBytes; long newTotalRetainedBytes = totalRetainedBytes;
final double targetTotalRetainedBytes = totalRetainedBytes / 2; final long targetTotalRetainedBytes = totalRetainedBytes / 2;
final List<BucketHolder> sortedHolders = new ArrayList<>(buckets.size()); final List<BucketHolder> sortedHolders = new ArrayList<>(buckets.size());
@ -384,7 +384,8 @@ public class ClusterByStatisticsCollectorImpl implements ClusterByStatisticsColl
// Downsample least-dense buckets first. (They're less likely to need high resolution.) // Downsample least-dense buckets first. (They're less likely to need high resolution.)
sortedHolders.sort( sortedHolders.sort(
Comparator.comparing((BucketHolder holder) -> Comparator.comparing((BucketHolder holder) ->
(double) holder.keyCollector.estimatedTotalWeight() / holder.keyCollector.estimatedRetainedKeys()) (double) holder.keyCollector.estimatedTotalWeight()
/ holder.keyCollector.estimatedRetainedKeys())
); );
int i = 0; int i = 0;

View File

@ -128,7 +128,7 @@ public class DelegateOrMinKeyCollector<TDelegate extends KeyCollector<TDelegate>
} }
@Override @Override
public double estimatedRetainedBytes() public long estimatedRetainedBytes()
{ {
if (delegate != null) { if (delegate != null) {
return delegate.estimatedRetainedBytes(); return delegate.estimatedRetainedBytes();

View File

@ -172,7 +172,7 @@ public class DistinctKeyCollector implements KeyCollector<DistinctKeyCollector>
} }
@Override @Override
public double estimatedRetainedBytes() public long estimatedRetainedBytes()
{ {
return retainedBytes; return retainedBytes;
} }

View File

@ -57,7 +57,7 @@ public interface KeyCollector<CollectorType extends KeyCollector<CollectorType>>
* Returns an estimate of the number of bytes currently retained by this collector. This may change over time as * Returns an estimate of the number of bytes currently retained by this collector. This may change over time as
* more keys are added. * more keys are added.
*/ */
double estimatedRetainedBytes(); long estimatedRetainedBytes();
/** /**
* Downsample this collector, dropping about half of the keys that are currently retained. Returns true if * Downsample this collector, dropping about half of the keys that are currently retained. Returns true if

View File

@ -102,9 +102,9 @@ public class QuantilesSketchKeyCollector implements KeyCollector<QuantilesSketch
} }
@Override @Override
public double estimatedRetainedBytes() public long estimatedRetainedBytes()
{ {
return averageKeyLength * estimatedRetainedKeys(); return Math.round(averageKeyLength * estimatedRetainedKeys());
} }
@Override @Override