query: cache engines in remoteEndpoints to reuse computed MinT / MaxT / LabelSets values across Engines() calls

[SuperPaintman]

• I added CHANGELOG entry for this change.
• Change is not relevant to the end user.

Changes

See #8597 for more details.

Cache RemoteEndpoints.Engines() results, so repeated calls return the same slice of engine.

This improves two things:

• It gives optimizers a consistent view of Engines() (used in PassthroughOptimizer and DistributedExecutionOptimizer).
• It avoids recomputing lazy fields (MinT / MaxT / LabelSets) multiple times in optimizers. In the current implementation, .Engines() recreates all remoteEngine's, and effectively resets their cached state.

Verification

• Added a unit test.
• Verified the change in an internal fork (has been running for about a week).

---

Issue: #8597

CC: @MichaHoffmann

[MichaHoffmann]

I think this looks reasonable to me; optimizers will have a consistent view of the Engines() then. Currently its used by Passhtrough and Distributed Optimizers.

[SuperPaintman]

We can verify that the underlying slice data is exactly the same:

testutil.Equals(t, unsafe.Pointer(unsafe.SliceData(engines1)), unsafe.Pointer(unsafe.SliceData(engines2)))

But I think, that's an overkill.

This would also be affected if the internal representation changes, e.g.:

-return r.engines
+return slices.Clone(r.engines)

[GiedriusS]

Why not make this even simpler and do this once in the constructor instead of wrapping everything into sync.Once? In Thanos case, it just creates a slice with a read lock so it should be very fast, no?

[SuperPaintman]

I think this change can look odd outside of the broader context and other changes.

In Thanos case, it just creates a slice with a read lock so it should be very fast, no?

I'm changing that in another PR. Engines() will do more work than just creating a new slice.

And it also needs more info than high-level API endpoints have (e.g. here).

So now I think it makes more sense to have this cache in the promql-engine: thanos-io/promql-engine#680

[fpetkovski]

Won't this introduce a bug with dns endpoints? How would new endpoints ever get added to the slice?

[SuperPaintman]

We create new remoteEndpoints for each query, and then Engines() is almost immediately called in query optimizers (e.g. here).

So this cache lives only for and during one query, and it doesn't affect adding / updating endpoints.

[SuperPaintman]

Moved it into promql-engine: thanos-io/promql-engine#680