chore(function-autoscaler): remove Cassandra history writes - #754
Conversation
📝 WalkthroughWalkthroughThe autoscaler no longer writes or reads Cassandra history prediction rows. Active-function batch operations target only current rows. Request processing keeps prediction results in memory. Cassandra operations now emit timing and status logs, and related settings, models, statements, and tests are simplified. ChangesCassandra history retirement
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Signed-off-by: Bora Oztekin <boztekin@nvidia.com>
a939030 to
a109fb5
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/control-plane-services/function-autoscaler/crates/server/src/cassandra/cassandra_service.rs (3)
48-96: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate failure logs from
with_cassandra_timingand its callers. The helper logs each error withtracing::warn!and then returns the same error. Callers that keep their ownmatchlog it again withtracing::error!, so one Cassandra failure produces two records. The coding guidelines forbid both logging and returning the same error.
src/control-plane-services/function-autoscaler/crates/server/src/cassandra/cassandra_service.rs#L48-L96: remove thetracing::warn!call in theErrarm and keep only the span field recording.src/control-plane-services/function-autoscaler/crates/server/src/cassandra/cassandra_service.rs#L737-L743: keep the caller-sidedelete_lockerror log as the single record, or drop thematchif the helper keeps logging.src/control-plane-services/function-autoscaler/crates/server/src/cassandra/cassandra_service.rs#L765-L771: apply the same choice toinsert_to_nodes.src/control-plane-services/function-autoscaler/crates/server/src/cassandra/cassandra_service.rs#L808-L814: apply the same choice todelete_node.🤖 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/cassandra/cassandra_service.rs` around lines 48 - 96, Remove the caller-duplicating tracing::warn! failure log from with_cassandra_timing while preserving its error span-field recording. In src/control-plane-services/function-autoscaler/crates/server/src/cassandra/cassandra_service.rs lines 737-743, 765-771, and 808-814, retain the caller-side error logs for delete_lock, insert_to_nodes, and delete_node so each failure is recorded once.Source: Coding guidelines
594-602: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMark the delete statement as idempotent.
A single-row DELETE by primary key is idempotent. Without
set_is_idempotent(true), the driver does not apply speculative execution or retries on timeout for this statement.♻️ Proposed change
let mut prepared = session.prepare(stmt_active_function).await?; prepared.set_consistency(scylla::statement::Consistency::Quorum); + prepared.set_is_idempotent(true);🤖 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/cassandra/cassandra_service.rs` around lines 594 - 602, Update the prepared DELETE statement in the delete_active_function flow to mark it as idempotent by calling the statement’s set_is_idempotent(true) alongside its consistency configuration, before executing it through with_cassandra_timing.
542-571: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSet the batch INSERT statement as idempotent.
The resolved dependency is
scylla1.4.1. Withoutset_is_idempotent(true), the configured speculative execution policy is skipped and retry behavior is more restrictive. This TTL upsert is safe to retry.🤖 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/cassandra/cassandra_service.rs` around lines 542 - 571, The prepared batch INSERT statement in the add_new_active_functions_batch flow must be marked idempotent. After preparing stmt_active_function and before executing the chunked functions, call set_is_idempotent(true) on prepared_active_function so speculative execution and retry behavior apply to this safe TTL upsert.
🤖 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.
Nitpick comments:
In
`@src/control-plane-services/function-autoscaler/crates/server/src/cassandra/cassandra_service.rs`:
- Around line 48-96: Remove the caller-duplicating tracing::warn! failure log
from with_cassandra_timing while preserving its error span-field recording. In
src/control-plane-services/function-autoscaler/crates/server/src/cassandra/cassandra_service.rs
lines 737-743, 765-771, and 808-814, retain the caller-side error logs for
delete_lock, insert_to_nodes, and delete_node so each failure is recorded once.
- Around line 594-602: Update the prepared DELETE statement in the
delete_active_function flow to mark it as idempotent by calling the statement’s
set_is_idempotent(true) alongside its consistency configuration, before
executing it through with_cassandra_timing.
- Around line 542-571: The prepared batch INSERT statement in the
add_new_active_functions_batch flow must be marked idempotent. After preparing
stmt_active_function and before executing the chunked functions, call
set_is_idempotent(true) on prepared_active_function so speculative execution and
retry behavior apply to this safe TTL upsert.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 165eb497-76bb-4662-9fee-1cd37cd75fa4
📒 Files selected for processing (6)
src/control-plane-services/function-autoscaler/crates/server/src/cassandra/cassandra_service.rssrc/control-plane-services/function-autoscaler/crates/server/src/cassandra/cassandra_settings.rssrc/control-plane-services/function-autoscaler/crates/server/src/cassandra/statements.rssrc/control-plane-services/function-autoscaler/crates/server/src/models/mod.rssrc/control-plane-services/function-autoscaler/crates/server/src/nvcf_api/nvcf_client.rssrc/control-plane-services/function-autoscaler/crates/server/src/work/discovery.rs
💤 Files with no reviewable changes (4)
- src/control-plane-services/function-autoscaler/crates/server/src/cassandra/cassandra_settings.rs
- src/control-plane-services/function-autoscaler/crates/server/src/cassandra/statements.rs
- src/control-plane-services/function-autoscaler/crates/server/src/models/mod.rs
- src/control-plane-services/function-autoscaler/crates/server/src/work/discovery.rs
TL;DR
Stop persisting function-autoscaler history and prediction results to Cassandra, reducing redundant database traffic while retaining Cassandra-backed active-function state and coordination.
Additional Details
recently_invoked_functions_historyrunning_functions_without_invocations_historyFunctionStateCacheinstead of writing them to Cassandra after each NVCF API response.This PR intentionally does not remove the history tables from schemas or migrations. It also does not change Cassandra-backed locks, healthy-node coordination, active-function persistence, consistency levels, scaling decisions, or TimeseriesDb queries.
For the Reviewer
Please focus on:
cassandra_service.rs.nvcf_client.rs.For QA
Validated from the function-autoscaler subtree:
Results: 127 tests passed, 11 integration tests ignored, and no failures.
During rollout, Cassandra operation traces can be used to confirm that history operations have stopped while active-function and coordination operations remain healthy.
Issues
NO-REF
Checklist
Summary by CodeRabbit