diff --git a/docs/alb.md b/docs/alb.md index eba23bfbc..ef296e310 100644 --- a/docs/alb.md +++ b/docs/alb.md @@ -85,6 +85,8 @@ The recommended application for using the **Time Series Merge** mechanism is as Separate from an HA use case, it is possible to use Time Series Merge as a Federation broker that merges responses from different, non-redundant tsdb endpoints; for example, to aggregate metrics from a solution running clusters in multiple regions, with separate, in-region-only tsdb deployments. In this use case, it is recommended to [inject labels](./prometheus.md#injecting-labels) into the responses to protect against data collisions across series. Label injection is demonstrated in the snippet below. +For request paths that are not mergeable by the configured time series provider, TSM does not fan the request out. Those requests are dispatched directly to the first live pool target. The same first-live-target fallback is used when a request cannot be prepared for the merge path. + #### Merge Strategy By default, TSM deduplicates values when merging series with identical labels — for each timestamp, only one value is kept. This works well for HA configurations where backends hold redundant copies of the same data. diff --git a/pkg/backends/alb/mech/tsm/time_series_merge.go b/pkg/backends/alb/mech/tsm/time_series_merge.go index f30b5b9c8..e4b02c694 100644 --- a/pkg/backends/alb/mech/tsm/time_series_merge.go +++ b/pkg/backends/alb/mech/tsm/time_series_merge.go @@ -28,7 +28,6 @@ import ( "github.com/trickstercache/trickster/v2/pkg/backends/alb/errors" "github.com/trickstercache/trickster/v2/pkg/backends/alb/mech" "github.com/trickstercache/trickster/v2/pkg/backends/alb/mech/fanout" - "github.com/trickstercache/trickster/v2/pkg/backends/alb/mech/rr" "github.com/trickstercache/trickster/v2/pkg/backends/alb/mech/types" "github.com/trickstercache/trickster/v2/pkg/backends/alb/names" "github.com/trickstercache/trickster/v2/pkg/backends/alb/options" @@ -56,9 +55,8 @@ const ( type handler struct { mech.PoolHolder - mergePaths []string // paths handled by the alb client that are enabled for tsmerge - nonmergeHandler types.PoolMechanism // when methodology is tsmerge, this handler is for non-mergeable paths - outputFormat string // the provider output format (e.g., "prometheus") + mergePaths []string // paths handled by the alb client that are enabled for tsmerge + outputFormat string // the provider output format (e.g., "prometheus") tsmOptions options.TimeSeriesMergeOptions maxCaptureBytes int maxFanoutCaptureBytes int @@ -102,10 +100,7 @@ func RegistryEntry() types.RegistryEntry { } func New(o *options.Options, factories rt.Lookup) (types.Mechanism, error) { - nmh, _ := rr.New(nil, nil) - nmpm, _ := nmh.(types.PoolMechanism) out := &handler{ - nonmergeHandler: nmpm, tsmOptions: o.TSMOptions, maxCaptureBytes: o.MaxCaptureBytes, maxFanoutCaptureBytes: o.MaxFanoutCaptureBytes, @@ -153,13 +148,10 @@ func (h *handler) Name() types.Name { return ShortName } -// SetPool overrides PoolHolder.SetPool to also propagate the pool to the -// non-merge handler used for paths that bypass the TSM merge path. +// SetPool overrides PoolHolder.SetPool so pool-derived caches can be +// invalidated when the pool is replaced. func (h *handler) SetPool(p pool.Pool) { h.PoolHolder.SetPool(p) - if h.nonmergeHandler != nil { - h.nonmergeHandler.SetPool(p) - } h.poolVersion.Add(1) } diff --git a/pkg/backends/alb/mech/tsm/time_series_merge_ctor_test.go b/pkg/backends/alb/mech/tsm/time_series_merge_ctor_test.go index 6ac1b8726..56787eece 100644 --- a/pkg/backends/alb/mech/tsm/time_series_merge_ctor_test.go +++ b/pkg/backends/alb/mech/tsm/time_series_merge_ctor_test.go @@ -22,8 +22,6 @@ import ( "testing" alberr "github.com/trickstercache/trickster/v2/pkg/backends/alb/errors" - "github.com/trickstercache/trickster/v2/pkg/backends/alb/mech/rr" - "github.com/trickstercache/trickster/v2/pkg/backends/alb/mech/types" "github.com/trickstercache/trickster/v2/pkg/backends/alb/options" "github.com/trickstercache/trickster/v2/pkg/backends/prometheus" "github.com/trickstercache/trickster/v2/pkg/backends/providers" @@ -88,23 +86,3 @@ func TestHandlerStopPool(t *testing.T) { h.SetPool(p) h.StopPool() } - -func TestHandlerSetPoolPropagatesNonmergeHandler(t *testing.T) { - t.Parallel() - - nmh, err := rr.New(nil, nil) - if err != nil { - t.Fatalf("rr.New: %v", err) - } - nmpm, ok := nmh.(types.PoolMechanism) - if !ok { - t.Fatal("rr mechanism should implement PoolMechanism") - } - h := &handler{nonmergeHandler: nmpm} - p, _, _ := albpool.NewHealthy([]http.Handler{http.NotFoundHandler()}) - defer p.Stop() - h.SetPool(p) - if nmpm.Pool() != p { - t.Fatal("SetPool did not propagate pool to nonmergeHandler") - } -} diff --git a/pkg/backends/alb/mech/tsm/time_series_merge_test.go b/pkg/backends/alb/mech/tsm/time_series_merge_test.go index c69963d34..d41abc5ab 100644 --- a/pkg/backends/alb/mech/tsm/time_series_merge_test.go +++ b/pkg/backends/alb/mech/tsm/time_series_merge_test.go @@ -92,6 +92,30 @@ func TestHandleResponseMerge(t *testing.T) { } } +func TestTSMNonMergePathUsesFirstLiveMember(t *testing.T) { + p, _, _ := albpool.NewHealthy([]http.Handler{ + albpool.NamedHandler("first"), + albpool.NamedHandler("second"), + }) + defer p.Stop() + albpool.WaitHealthy(t, p, 2) + + h := &handler{mergePaths: []string{"/api/v1/query_range"}} + h.SetPool(p) + + for i := 0; i < 3; i++ { + w := httptest.NewRecorder() + r := httptest.NewRequest(http.MethodGet, "https://trickstercache.org/api/v1/query?query=up", nil) + h.ServeHTTP(w, r) + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want %d", w.Code, http.StatusOK) + } + if got := w.Body.String(); got != "first" { + t.Fatalf("body = %q, want first", got) + } + } +} + // A panicking pool member must not crash the request. RecoverFanoutPanic("tsm", // ...) at time_series_merge.go must catch it and mark the slot failed so the // merge surfaces the partial-failure (phit) signal.