Skip to content

fix: collect ram requirements from SDL storage#3192

Merged
stalniy merged 1 commit into
mainfrom
feat/ram-storage-collection
May 20, 2026
Merged

fix: collect ram requirements from SDL storage#3192
stalniy merged 1 commit into
mainfrom
feat/ram-storage-collection

Conversation

@stalniy
Copy link
Copy Markdown
Contributor

@stalniy stalniy commented May 18, 2026

Why

Ref CON-346

What

Summary by CodeRabbit

  • Bug Fixes
    • RAM-class storage volumes are now correctly included in provider memory capacity calculations and filtering, ensuring providers are accurately assessed against combined memory and storage requirements.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 18, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 14a696c2-9c12-49bf-a75f-c7d86a3c715f

📥 Commits

Reviewing files that changed from the base of the PR and between 4eaa65f and 9c3f767.

📒 Files selected for processing (3)
  • apps/provider-inventory/src/repositories/bid-screening/bid-screening.aggregator.spec.ts
  • apps/provider-inventory/src/repositories/bid-screening/bid-screening.aggregator.ts
  • apps/provider-inventory/src/repositories/bid-screening/bid-screening.repository.integration.ts

📝 Walkthrough

Walkthrough

The PR refactors memory aggregation in bid screening to account for RAM-class storage volumes as part of effective per-unit memory. The aggregation logic accumulates RAM contributions during storage scanning, and the test suite validates totals partitioning and per-replica memory calculations including the new behavior.

Changes

RAM-class storage memory aggregation

Layer / File(s) Summary
Effective memory aggregation logic
apps/provider-inventory/src/repositories/bid-screening/bid-screening.aggregator.ts
aggregateCriteria initializes effectiveMemory from unit.resources.memory.quantity, accumulates RAM-class storage quantities into it during a single loop, and derives totalMemory and maxPerReplicaMemory from the accumulated value; ephemeral and persistent storage totals remain separate.
Unit tests for aggregation totals and per-replica memory
apps/provider-inventory/src/repositories/bid-screening/bid-screening.aggregator.spec.ts
Three totals tests verify RAM-class contribution to totalMemory without affecting ephemeral/persistent totals, summing across multiple RAM volumes, and correct partitioning when all three volume classes coexist on a single unit. New maxPerReplica test confirms RAM-class quantities are included in per-replica memory maxima.
Integration test for memory headroom filtering
apps/provider-inventory/src/repositories/bid-screening/bid-screening.repository.integration.ts
New integration test verifies findCandidates correctly filters providers based on combined per-replica memory demand (base memory plus RAM-class volumes), seeding tight and sufficient providers.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • akash-network/console#3184: Introduced the aggregator logic that previously skipped RAM-class volumes in totals; this PR updates the same code path to include them.

Suggested labels

size: L

Suggested reviewers

  • baktun14
  • ygrishajev
✨ 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 feat/ram-storage-collection

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

@codecov
Copy link
Copy Markdown

codecov Bot commented May 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.24%. Comparing base (8f9b64e) to head (9c3f767).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3192      +/-   ##
==========================================
- Coverage   63.99%   63.24%   -0.76%     
==========================================
  Files        1098     1057      -41     
  Lines       26705    25671    -1034     
  Branches     6481     6313     -168     
==========================================
- Hits        17091    16236     -855     
+ Misses       8411     8245     -166     
+ Partials     1203     1190      -13     
Flag Coverage Δ *Carryforward flag
api 84.28% <ø> (ø) Carriedforward from 8f9b64e
deploy-web 47.44% <ø> (ø) Carriedforward from 8f9b64e
log-collector ?
notifications 91.06% <ø> (ø) Carriedforward from 8f9b64e
provider-console 81.48% <ø> (ø) Carriedforward from 8f9b64e
provider-inventory 80.25% <100.00%> (+0.04%) ⬆️
provider-proxy 86.08% <ø> (ø) Carriedforward from 8f9b64e
tx-signer ?

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
...sitories/bid-screening/bid-screening.aggregator.ts 100.00% <100.00%> (ø)

... and 41 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@stalniy stalniy changed the title Feat/ram storage collection fix: collect ram requirements from SDL storage May 18, 2026
@stalniy stalniy force-pushed the feat/ram-storage-collection branch 2 times, most recently from f2898e0 to 889f2a2 Compare May 18, 2026 14:13
@github-actions github-actions Bot added size: S and removed size: M labels May 18, 2026
@stalniy stalniy marked this pull request as ready for review May 18, 2026 14:14
@stalniy stalniy requested a review from a team as a code owner May 18, 2026 14:14
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 `@apps/provider-inventory/src/providers/drizzle.provider.ts`:
- Line 1: Replace the bare DefaultLogger usage with the PostgresLoggerService
wrapper: import DefaultLogger from "drizzle-orm/logger" (instead of
"drizzle-orm"), and instantiate the logger as new DefaultLogger({ writer: new
PostgresLoggerService({ useFormat: config.SQL_LOG_FORMAT === "pretty" }) });
update the provider's logger property to use that wrapped instance (referencing
DefaultLogger and PostgresLoggerService and the config.SQL_LOG_FORMAT flag).
🪄 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: Pro

Run ID: ec28f324-dd0f-4148-ad0a-abac4206c4cd

📥 Commits

Reviewing files that changed from the base of the PR and between 0ae0146 and 889f2a2.

📒 Files selected for processing (4)
  • apps/provider-inventory/src/providers/drizzle.provider.ts
  • apps/provider-inventory/src/repositories/bid-screening/bid-screening.aggregator.spec.ts
  • apps/provider-inventory/src/repositories/bid-screening/bid-screening.aggregator.ts
  • apps/provider-inventory/src/repositories/bid-screening/bid-screening.repository.integration.ts

Comment thread apps/provider-inventory/src/providers/drizzle.provider.ts Outdated
@stalniy stalniy force-pushed the feat/ram-storage-collection branch from 889f2a2 to 4eaa65f Compare May 20, 2026 11:27
Comment thread apps/provider-inventory/src/providers/drizzle.provider.ts Outdated
@stalniy stalniy enabled auto-merge May 20, 2026 11:27
@stalniy stalniy force-pushed the feat/ram-storage-collection branch from 4eaa65f to 9c3f767 Compare May 20, 2026 11:31
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 `@apps/provider-inventory/src/providers/drizzle.provider.ts`:
- Line 1: Remove the unused DefaultLogger import from the top of
drizzle.provider.ts (it's not passed into the drizzle() call); either delete the
import altogether or, if you intend to enable logging now, replace it by wiring
the existing PostgresLoggerService wrapper used in
apps/api/src/core/providers/postgres.provider.ts into the drizzle() options so a
logger is actually provided to drizzle().
🪄 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: Pro

Run ID: d0de7edd-5e5e-47e6-8cd2-d5ae9c68fb0b

📥 Commits

Reviewing files that changed from the base of the PR and between 889f2a2 and 4eaa65f.

📒 Files selected for processing (4)
  • apps/provider-inventory/src/providers/drizzle.provider.ts
  • apps/provider-inventory/src/repositories/bid-screening/bid-screening.aggregator.spec.ts
  • apps/provider-inventory/src/repositories/bid-screening/bid-screening.aggregator.ts
  • apps/provider-inventory/src/repositories/bid-screening/bid-screening.repository.integration.ts

Comment thread apps/provider-inventory/src/providers/drizzle.provider.ts Outdated
@stalniy stalniy added this pull request to the merge queue May 20, 2026
Merged via the queue into main with commit f540c3f May 20, 2026
56 checks passed
@stalniy stalniy deleted the feat/ram-storage-collection branch May 20, 2026 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants