Merge pull request #141519 from dhartunian/sql-stats-quantile-warnings

release-24.3: sqlstats: clarify latency percentile limitations
cockroachdb · Feb 21, 2025 · 9ab9d8c · 9ab9d8c
2 parents d9f534e + c977795
commit 9ab9d8c
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 20 deletions.
diff --git a/pkg/sql/appstatspb/app_stats.go b/pkg/sql/appstatspb/app_stats.go
@@ -285,24 +285,4 @@ func (s *LatencyInfo) Add(other LatencyInfo) {
 	if other.Max > s.Max {
 		s.Max = other.Max
 	}
-	s.checkPercentiles()
-}
-
-// checkPercentiles is a patchy solution and not ideal.
-// When the execution count for a period is smaller than 500,
-// the percentiles sample is including previous aggregation periods,
-// making the p99 possible be greater than the max.
-// For now, we just do a check and update the percentiles to the max
-// possible size.
-// TODO(maryliag): use a proper sample size (#99070)
-func (s *LatencyInfo) checkPercentiles() {
-	if s.P99 > s.Max {
-		s.P99 = s.Max
-	}
-	if s.P90 > s.Max {
-		s.P90 = s.Max
-	}
-	if s.P50 > s.Max {
-		s.P50 = s.Max
-	}
 }
diff --git a/pkg/ui/workspaces/cluster-ui/src/statsTableUtil/statsTableUtil.tsx b/pkg/ui/workspaces/cluster-ui/src/statsTableUtil/statsTableUtil.tsx
@@ -964,6 +964,14 @@ export const statisticsTableTitles: StatisticTableTitleType = {
           <p>
             The 50th latency percentile for sampled {contentModifier} executions
             with this fingerprint.
+            <br />
+            <br />
+            <strong>Warning:</strong> the data source for latency percentiles is
+            different from the source for other execution statistics. These
+            percentiles are not calculated from the same set of executions as
+            the other columns and can be inconsistent. The data is provided for
+            informational purposes here and is not expected to be consistent
+            with max, min, or average latency that is also presented.
           </p>
         }
       >
@@ -990,6 +998,14 @@ export const statisticsTableTitles: StatisticTableTitleType = {
           <p>
             The 90th latency percentile for sampled {contentModifier} executions
             with this fingerprint.
+            <br />
+            <br />
+            <strong>Warning:</strong> the data source for latency percentiles is
+            different from the source for other execution statistics. These
+            percentiles are not calculated from the same set of executions as
+            the other columns and can be inconsistent. The data is provided for
+            informational purposes here and is not expected to be consistent
+            with max, min, or average latency that is also presented.
           </p>
         }
       >
@@ -1016,6 +1032,14 @@ export const statisticsTableTitles: StatisticTableTitleType = {
           <p>
             The 99th latency percentile for sampled {contentModifier} executions
             with this fingerprint.
+            <br />
+            <br />
+            <strong>Warning:</strong> the data source for latency percentiles is
+            different from the source for other execution statistics. These
+            percentiles are not calculated from the same set of executions as
+            the other columns and can be inconsistent. The data is provided for
+            informational purposes here and is not expected to be consistent
+            with max, min, or average latency that is also presented.
           </p>
         }
       >