fix(function-autoscaler): shard discovery queries

[borao]

TL;DR

Shard unfiltered function-discovery invocation queries by UUID prefix, continue using independent TimeseriesDb results when one query fails, and correctly surface per-function errors returned through the scaling JoinSet.

Additional Details (optional for docs, build, test, refactor, ci, chore, style, and revert PRs)

• Splits unfiltered discovery queries into four fixed, non-overlapping UUID-prefix shards: [0-3]., [4-7]., [89ab]., and [c-f]..
• Applies sharding to both invocation-service and gRPC-proxy discovery queries.
• Limits discovery TimeseriesDb concurrency to four queries across invocation and worker sources.
• Keeps per-function scaling queries unsharded.
• Executes all invocation-service and gRPC-proxy queries even when another source or shard fails.
• Runs worker/BYOC discovery independently of invocation discovery.
• Retains and deduplicates successful partial discovery results. The cycle fails only when neither invocation nor worker discovery produces a usable response.
• Keeps per-function recent-invocation checks fail-closed if either invocation source fails, avoiding an unsafe scale-to-zero decision.
• Handles both layers returned by JoinSet<Result<()>>, continues processing independent functions and buckets, and returns an aggregate error after all tasks finish.

This PR intentionally does not change the PromQL lookback window, range-query evaluation mode, retry policy, or TimeseriesDb metrics.

For the Reviewer

Please focus on:

• Shard construction and selector injection in work/discovery.rs.
• Partial-result behavior across invocation, gRPC-proxy, and worker sources.
• The shared discovery query concurrency limit.
• Nested JoinSet error handling in work/mod.rs.

For QA (optional for docs, build, test, refactor, ci, chore, style, and revert PRs)

Validated from the function-autoscaler subtree:

cargo fmt -p rs-autoscaler
cargo clippy -p rs-autoscaler --all-targets -- -D warnings
cargo test -p rs-autoscaler

Results: 127 tests passed, 11 integration tests ignored, and no failures.

Issues

NO-REF

Checklist

• I am familiar with the Contributing Guidelines.
• I have signed off my commits for Developer Certificate of Origin (DCO) compliance.
• New or existing tests cover these changes.
• The documentation is up to date with these changes.

Summary by CodeRabbit

• Bug Fixes
  • Improved function discovery reliability by querying multiple sources and shards concurrently.
  • Partial discovery results are retained when individual queries or sources fail.
  • Discovery now fails only when all available sources are unavailable.
  • Improved handling of overlapping results and recovery from worker-based discovery failures.
  • Per-function lookups now fail safely when required data cannot be retrieved.
  • Scaling-input collection continues processing remaining functions and reports task failures collectively.

[coderabbitai]

📝 Walkthrough

Walkthrough

Changes

Autoscaler scaling workflow

Layer / File(s)	Summary
Sharded discovery queries src/control-plane-services/function-autoscaler/crates/server/src/work/discovery.rs	Discovery builds four function-ID shards across two metric sources. Queries run concurrently under a shared limit. Partial results are retained, while per-function lookups fail closed when either source fails.
TimeseriesDb discovery integration src/control-plane-services/function-autoscaler/crates/server/src/work/discovery.rs	TimeseriesDb discovery combines recent-invocation and worker results. It preserves worker counts and fails only when both sources fail.
Scaling task failure aggregation src/control-plane-services/function-autoscaler/crates/server/src/work/mod.rs	Scaling-input processing drains asynchronous tasks, counts task errors and panics or cancellations, adds function and version context, and returns an aggregated error.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ActiveFunctionDiscovery
  participant InvocationMetrics
  participant TimeseriesDb
  ActiveFunctionDiscovery->>InvocationMetrics: Run four sharded queries per source
  ActiveFunctionDiscovery->>TimeseriesDb: Run recent-invocation and worker queries
  InvocationMetrics-->>ActiveFunctionDiscovery: Return successful and failed query results
  TimeseriesDb-->>ActiveFunctionDiscovery: Return worker and invocation results
  ActiveFunctionDiscovery->>ActiveFunctionDiscovery: Merge results and preserve precedence

Loading

Suggested reviewers: dmikhaylovnv

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title uses the required Conventional Commits format and accurately describes the primary function-autoscaler discovery fix.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/function-autoscaler/discovery-queries

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

🛡️ CodeQL Analysis

🚨 Found 2 issue(s)

Severity Breakdown:

• 🔴 Errors: 0
• 🟡 Warnings: 0
• 🔵 Notes: 0

📋 Top Issues

🔗 View full details in Security tab

_{🕐 Last updated: 2026-08-04 05:01:26 UTC | Commit: a5a9057}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (3)

src/control-plane-services/function-autoscaler/crates/server/src/work/discovery.rs (3)

923-946: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

fast_backoff, ts_client, and vm_series are duplicated across test modules.

work/mod.rs defines test helpers with the same three names. Its vm_series takes a metric-label string and a value, while this one takes a metric name and two Uuid values. Two helpers with one name and different contracts make the tests harder to follow.

Move the shared helpers into one test-support module (for example work/test_support.rs gated by #[cfg(test)]) and give the two vm_series variants distinct names or one unified signature.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/control-plane-services/function-autoscaler/crates/server/src/work/discovery.rs`
around lines 923 - 946, Consolidate the duplicated test helpers fast_backoff,
ts_client, and vm_series from the current test module and work/mod.rs into a
single #[cfg(test)] test-support module. Resolve the conflicting vm_series
contracts by giving the variants distinct descriptive names or defining one
unified signature, then update all test call sites to use the shared helpers.

---

644-652: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Each query error is both logged and returned as text.

Lines 583-592 log every failed query with tracing::error!, and the same messages are then joined into the returned anyhow! error. The caller logs that aggregate error again. Stringifying also drops the source chain of the original errors.

Choose one path. Two options work here:

• Keep the per-query tracing::error! calls for diagnostics and return a short aggregate error that reports counts only.
• Or downgrade the per-query logs to debug and attach the first error as the anyhow source with .context(...) so the chain survives.

As per coding guidelines: "preserve originating errors with %w or equivalent wrapping, and do not both log and return the same error".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/control-plane-services/function-autoscaler/crates/server/src/work/discovery.rs`
around lines 644 - 652, Avoid duplicating failed-query details between logging
and the returned error in the discovery error path. Prefer retaining the
existing per-query tracing::error! diagnostics while changing the anyhow!
message in the successful_queries/function_version_id_filter guard to report
only aggregate failure counts, rather than joining failures into returned text.

Source: Coding guidelines

---

508-554: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Per-function callers pass no semaphore, so their queries are unbounded.

get_recently_invoked_functions calls this function with query_semaphore: None. Each call then issues its two source queries concurrently with no global bound. work/mod.rs invokes it per function version inside scaling-input processing, so total in-flight TimeseriesDb requests scale with the number of functions processed in parallel.

Consider passing the same shared semaphore from the scaling loop, or hold one process-wide semaphore for all discovery and per-function queries. This keeps the load on TimeseriesDb predictable.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/control-plane-services/function-autoscaler/crates/server/src/work/discovery.rs`
around lines 508 - 554, Ensure per-function callers of
get_recently_invoked_functions_with_semaphore use the same shared query
semaphore as discovery instead of passing None. Thread the semaphore through
get_recently_invoked_functions and the scaling-input loop in work/mod.rs,
preserving the existing semaphore acquisition around each TimeseriesDb query so
all discovery and per-function requests share one global concurrency bound.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@src/control-plane-services/function-autoscaler/crates/server/src/work/discovery.rs`:
- Around line 297-329: Update the workers_result merge in the active_map
population to preserve worker metrics: when an entry from recent invocations
already exists, merge the worker record’s num_workers into that existing
ActiveFunctionDetails instead of discarding the worker record. Keep the current
precedence for other fields and leave scaling inputs unchanged, ensuring
persisted history receives the reported worker count.

In
`@src/control-plane-services/function-autoscaler/crates/server/src/work/mod.rs`:
- Around line 605-611: Update drain_scaling_tasks and
make_scaling_requests_for_table to propagate, rather than log or discard, inner
task errors while still draining every task and reporting the failure count.
Return an aggregate error containing the source error chain plus bucket, table,
and count context, and remove duplicate lower-level logging; update
drain_scaling_tasks_counts_inner_and_join_errors to assert the propagated error
includes “sentinel task error”.

---

Nitpick comments:
In
`@src/control-plane-services/function-autoscaler/crates/server/src/work/discovery.rs`:
- Around line 923-946: Consolidate the duplicated test helpers fast_backoff,
ts_client, and vm_series from the current test module and work/mod.rs into a
single #[cfg(test)] test-support module. Resolve the conflicting vm_series
contracts by giving the variants distinct descriptive names or defining one
unified signature, then update all test call sites to use the shared helpers.
- Around line 644-652: Avoid duplicating failed-query details between logging
and the returned error in the discovery error path. Prefer retaining the
existing per-query tracing::error! diagnostics while changing the anyhow!
message in the successful_queries/function_version_id_filter guard to report
only aggregate failure counts, rather than joining failures into returned text.
- Around line 508-554: Ensure per-function callers of
get_recently_invoked_functions_with_semaphore use the same shared query
semaphore as discovery instead of passing None. Thread the semaphore through
get_recently_invoked_functions and the scaling-input loop in work/mod.rs,
preserving the existing semaphore acquisition around each TimeseriesDb query so
all discovery and per-function requests share one global concurrency bound.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 3e9188cf-cd57-43c8-8cf8-dc50217a2039

📥 Commits

Reviewing files that changed from the base of the PR and between 892b704 and a5a9057.

📒 Files selected for processing (2)

• src/control-plane-services/function-autoscaler/crates/server/src/work/discovery.rs
• src/control-plane-services/function-autoscaler/crates/server/src/work/mod.rs

[dmikhaylovnv]

LGTM

[balajinvda]

🎉 This PR is included in version nvcf-function-autoscaler-v1.20.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀