Skip to content

chore(function-autoscaler): remove Cassandra history writes - #754

Open
borao wants to merge 1 commit into
mainfrom
chore/function-autoscaler/remove-history-writes-to-db
Open

chore(function-autoscaler): remove Cassandra history writes#754
borao wants to merge 1 commit into
mainfrom
chore/function-autoscaler/remove-history-writes-to-db

Conversation

@borao

@borao borao commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

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

  • Removes reads and writes for:
    • recently_invoked_functions_history
    • running_functions_without_invocations_history
  • Writes discovery state only to the corresponding active-function table.
  • Keeps prediction results in the existing in-memory FunctionStateCache instead of writing them to Cassandra after each NVCF API response.
  • Removes the unused history CQL statements, service methods, model fields, configuration, and Cassandra integration tests.
  • Improves Cassandra tracing with structured operation spans containing operation name, duration, status, error details, and OpenTelemetry status.
  • Extends tracing coverage to active-function, lock, and healthy-node writes.

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:

  • Active-function insert, transition, and delete behavior in cassandra_service.rs.
  • The cache-only prediction-result path in nvcf_client.rs.
  • Removal of obsolete history statements, configuration, and model fields.
  • The structured Cassandra tracing wrapper and its error reporting.

For QA

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
git diff --check

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

  • 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

  • New Features
    • Improved Cassandra operation monitoring with timing, status, duration, and OpenTelemetry details.
    • Streamlined active-function persistence to focus on current state.
  • Bug Fixes
    • Removed obsolete history and prediction data handling.
    • Simplified request processing and reduced unnecessary persistence operations.
  • Chores
    • Removed unused prediction configuration, fields, and database operations.
    • Updated related tests and health-check coverage.

@borao
borao requested a review from a team as a code owner August 10, 2026 22:05
@borao
borao requested a review from harshm98 August 10, 2026 22:05
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The 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.

Changes

Cassandra history retirement

Layer / File(s) Summary
History contract and request-path removal
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/nvcf_api/nvcf_client.rs, src/control-plane-services/function-autoscaler/crates/server/src/work/discovery.rs
History prediction settings, statement builders, model fields, Cassandra request context, and prediction writes are removed. Prediction results remain in the in-memory cache.
Active-table writes, deletes, and Cassandra timing
src/control-plane-services/function-autoscaler/crates/server/src/cassandra/cassandra_service.rs
Active-function batch writes and deletes target only current rows. Lock and node operations use structured timing instrumentation. Integration coverage now tests active-table insertion, token-range retrieval, and deletion.

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

Suggested reviewers: harshm98, apartha-nv

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title follows Conventional Commits and accurately describes the removal of Cassandra history writes, which is the primary change.
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.
✨ 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 chore/function-autoscaler/remove-history-writes-to-db

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

Signed-off-by: Bora Oztekin <boztekin@nvidia.com>
@borao
borao force-pushed the chore/function-autoscaler/remove-history-writes-to-db branch from a939030 to a109fb5 Compare August 10, 2026 22:35
@borao
borao requested review from a team as code owners August 10, 2026 22:35
@borao
borao requested a review from apartha-nv August 10, 2026 22:35
@borao
borao changed the base branch from feat/function-autoscaler/llm-gateway-scaling to main August 10, 2026 22:36
@github-actions

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (3)
src/control-plane-services/function-autoscaler/crates/server/src/cassandra/cassandra_service.rs (3)

48-96: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate failure logs from with_cassandra_timing and its callers. The helper logs each error with tracing::warn! and then returns the same error. Callers that keep their own match log it again with tracing::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 the tracing::warn! call in the Err arm 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-side delete_lock error log as the single record, or drop the match if the helper keeps logging.
  • src/control-plane-services/function-autoscaler/crates/server/src/cassandra/cassandra_service.rs#L765-L771: apply the same choice to insert_to_nodes.
  • src/control-plane-services/function-autoscaler/crates/server/src/cassandra/cassandra_service.rs#L808-L814: apply the same choice to delete_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 win

Mark 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 win

Set the batch INSERT statement as idempotent.

The resolved dependency is scylla 1.4.1. Without set_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

📥 Commits

Reviewing files that changed from the base of the PR and between 73b1efb and a109fb5.

📒 Files selected for processing (6)
  • src/control-plane-services/function-autoscaler/crates/server/src/cassandra/cassandra_service.rs
  • 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/nvcf_api/nvcf_client.rs
  • src/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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant