From d40390061dc415d46487d9a0886554b6887e4c6c Mon Sep 17 00:00:00 2001 From: Jan Eitzinger Date: Thu, 27 Aug 2026 12:16:46 +0200 Subject: [PATCH] fix: unify resample policy target-points table The policy -> target-points mapping existed twice with different values: internal/config (300/600/1000) fed the resampler's MinimumRequiredPoints threshold, while internal/metricdispatch (200/500/1000) fed the requested resolution and the frontend target point count. Because the threshold was larger than the target, the resampler refused to downsample series whose length fell between the two numbers, silently dropping the resolution the backend had asked for. With the medium policy that covered every series between 500 and 600 points. Move the table into internal/config as the single source of truth (import direction rules out the reverse, since metricdispatch already imports config) and keep the 200/500/1000 values, which already drove the requested resolution. metricdispatch.TargetPointsForPolicy now delegates to it, so MinimumRequiredPoints equals the target and resampling happens exactly when a series exceeds it. Co-Authored-By: Claude Opus 5 (1M context) --- internal/config/config.go | 39 ++++++++++++++------ internal/config/config_test.go | 44 +++++++++++++++++++++++ internal/metricdispatch/resamplepolicy.go | 22 ++++++------ 3 files changed, 82 insertions(+), 23 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index ace7b0a8..5865c5ec 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -171,24 +171,41 @@ func Init(mainConfig json.RawMessage) { cclog.Abortf("Config Init: Could not decode config file '%s'.\nError: %s\n", mainConfig, err.Error()) } - if Keys.EnableResampling != nil { - policy := Keys.EnableResampling.DefaultPolicy - if policy == "" { - policy = "medium" - } - resampler.SetMinimumRequiredPoints(targetPointsForPolicy(policy)) - } + initResampler() } -func targetPointsForPolicy(policy string) int { +// initResampler aligns the resampler's MinimumRequiredPoints threshold with the +// configured policy's target point count. The resampler must be allowed to act +// exactly when a series is longer than that target; a different threshold here +// silently drops resample requests for a band of job durations. +func initResampler() { + if Keys.EnableResampling == nil { + return + } + + policy := Keys.EnableResampling.DefaultPolicy + if policy == "" { + policy = DefaultResamplePolicy + } + resampler.SetMinimumRequiredPoints(TargetPointsForPolicy(policy)) +} + +// DefaultResamplePolicy is used when no resample policy is configured. +const DefaultResamplePolicy = "medium" + +// TargetPointsForPolicy returns the target number of data points for a resample +// policy. This is the single source of truth: it feeds both the requested +// resolution (via metricdispatch.ComputeResolution) and the resampler's +// MinimumRequiredPoints threshold. Unknown or empty policies return 0. +func TargetPointsForPolicy(policy string) int { switch policy { case "low": - return 300 + return 200 case "medium": - return 600 + return 500 case "high": return 1000 default: - return 600 + return 0 } } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index e4a700ff..14eb570f 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -10,6 +10,7 @@ import ( ccconf "github.com/ClusterCockpit/cc-lib/v2/ccConfig" cclog "github.com/ClusterCockpit/cc-lib/v2/ccLogger" + "github.com/ClusterCockpit/cc-lib/v2/resampler" ) func TestInit(t *testing.T) { @@ -39,3 +40,46 @@ func TestInitMinimal(t *testing.T) { t.Errorf("wrong addr\ngot: %s \nwant: 127.0.0.1:8080", Keys.Addr) } } + +func TestTargetPointsForPolicy(t *testing.T) { + tests := []struct { + policy string + want int + }{ + {"low", 200}, + {"medium", 500}, + {"high", 1000}, + {"unknown", 0}, + {"", 0}, + } + + for _, tt := range tests { + if got := TargetPointsForPolicy(tt.policy); got != tt.want { + t.Errorf("TargetPointsForPolicy(%q) = %d, want %d", tt.policy, got, tt.want) + } + } +} + +// The resampler must be allowed to act exactly when a series exceeds the target +// point count. A mismatch here silently drops resample requests for a band of +// job durations. +func TestInitSyncsResamplerThreshold(t *testing.T) { + for _, policy := range []string{"low", "medium", "high"} { + Keys.EnableResampling = &ResampleConfig{DefaultPolicy: policy} + initResampler() + + want := TargetPointsForPolicy(policy) + if resampler.MinimumRequiredPoints != want { + t.Errorf("policy %q: MinimumRequiredPoints = %d, want %d", + policy, resampler.MinimumRequiredPoints, want) + } + } + + // Empty policy falls back to the documented default. + Keys.EnableResampling = &ResampleConfig{} + initResampler() + if want := TargetPointsForPolicy(DefaultResamplePolicy); resampler.MinimumRequiredPoints != want { + t.Errorf("empty policy: MinimumRequiredPoints = %d, want %d", + resampler.MinimumRequiredPoints, want) + } +} diff --git a/internal/metricdispatch/resamplepolicy.go b/internal/metricdispatch/resamplepolicy.go index 14703cbc..2c074276 100644 --- a/internal/metricdispatch/resamplepolicy.go +++ b/internal/metricdispatch/resamplepolicy.go @@ -4,7 +4,11 @@ // license that can be found in the LICENSE file. package metricdispatch -import "math" +import ( + "math" + + "github.com/ClusterCockpit/cc-backend/internal/config" +) type ResamplePolicy string @@ -14,18 +18,12 @@ const ( ResamplePolicyHigh ResamplePolicy = "high" ) -// TargetPointsForPolicy returns the target number of data points for a given policy. +// TargetPointsForPolicy returns the target number of data points for a given +// policy. The table lives in the config package so that the requested +// resolution and the resampler's MinimumRequiredPoints threshold can never +// diverge. func TargetPointsForPolicy(policy ResamplePolicy) int { - switch policy { - case ResamplePolicyLow: - return 200 - case ResamplePolicyMedium: - return 500 - case ResamplePolicyHigh: - return 1000 - default: - return 0 - } + return config.TargetPointsForPolicy(string(policy)) } // ComputeResolution computes the resampling resolution in seconds for a given