[ritojoao]

Refs #1455

Hi! Thanks for maintaining this project. This is a small fix for the legacy collector exceeding --collection-timeout when PostgreSQL stops responding.

Problem

With a slow legacy query (for example SELECT 42 AS value FROM pg_sleep(30)) and --collection-timeout=2s, pausing PostgreSQL during the query made the scrape finish only after PostgreSQL came back (about 6 seconds in the report), instead of around 2 seconds.

Root cause

The legacy query already uses QueryContext, so the timeout does cancel the query. However, when the context is done, lib/pq sends a cancel request to the server and then waits for the answer. If PostgreSQL is not responding, that wait is not bounded by the collection timeout. The legacy loop also kept starting the next namespace queries after the deadline had passed.

Fix

• queryNamespaceMappingWithContext runs the namespace query in a goroutine and returns as soon as the context is done. The result channel is buffered, so the goroutine never blocks on sending its result. The goroutine and its connection can still stay blocked until PostgreSQL responds again.
• queryNamespaceMappings stops starting new queries once the context is done and records the context error for the remaining namespaces.

The change is limited to exporter/namespace.go. There are no new flags, metrics or dependencies.

Known limitation

This PR bounds the scrape that is running when PostgreSQL stops responding. It does not bound connection startup. While PostgreSQL stays unresponsive, later /metrics and /probe requests can block while opening a new connection, past the collection timeout (confirmed by @IlyaStarov with goroutine profiles). Collection recovers once PostgreSQL responds again.

I kept this PR small on purpose. Connection-level handling overlaps with #1450, and I am happy to look at it separately.

Tests

• New TestQueryNamespaceMappingsHonorsContextWhenQueryIsStuck uses a fake database/sql driver whose query ignores the context and blocks, like an unresponsive server. It fails without the fix and passes with it.
• go test -race ./exporter/ ./collector/ ./cmd/... and go vet ./... pass (Go 1.x in the golang:1 image).
• I did not re-run the Docker docker pause script from the issue; the unit test covers the same behavior.

Please let me know if you would like anything changed. Thank you!

[IlyaStarov]

Thanks! I confirmed that the original reproduction now finishes in about 2.03s with --collection-timeout=2s. However, subsequent /probe and /metrics requests while PostgreSQL remains paused hit a 3s client timeout. Goroutine profiles show those requests blocked during connection startup. After unpause, collection recovers. Could you clarify this remaining limitation in the description?

[ritojoao]

Thank you for testing this so carefully, @IlyaStarov, and for the goroutine profiles.

You are right. My note said a later scrape would be bounded by its own timeout, and that is not true for connection startup. I have updated the description with a "Known limitation" section: this PR bounds the scrape that is in flight when PostgreSQL stops responding, but later requests can still block while opening a new connection until PostgreSQL responds again.

I would like to keep this PR focused on the in-flight case. I am happy to look at connection startup in a follow-up, since it overlaps with #1450. Please let me know if you would prefer it in this PR instead.

[IlyaStarov]

Thanks for clarifying! Keeping this PR focused on the in-flight query timeout makes sense to me.
One wording suggestion: “so nothing leaks” is too strong. The buffered channel prevents a blocked result send, but the query goroutine and connection can remain blocked until PostgreSQL responds again.
Since connection startup can still exceed the collection timeout, could we use Refs #1455 instead of Fixes #1455 and keep that issue open to track the remaining work? Connection-level handling could then be addressed in a subsequent PR, taking #1450 into account.

[ritojoao]

Thanks again, @IlyaStarov. Both points are fair.

I changed Fixes #1455 to Refs #1455, so the issue stays open for the connection startup part, and I reworded the goroutine note: the buffered channel only keeps the result send from blocking, while the goroutine and its connection can stay blocked until PostgreSQL responds again.

I am happy to look at the connection-level handling in a follow-up PR, taking #1450 into account.