Skip to content

Conversation

Aditya1404Sal
Copy link

for granular per-component resource control

Feature or Problem

This PR aims to allow the max_linear_memory field to be configurable by the wadm.yaml, which will ultimately be used for setting per component resource limits.

Related Issues

Memory Limits Per Component

Release Information

Consumer Impact

Testing

Unit Test(s)

Acceptance or Integration

Manual Verification

@Aditya1404Sal Aditya1404Sal requested a review from a team as a code owner May 20, 2025 18:05
@Aditya1404Sal Aditya1404Sal changed the title feat(types): Added LimitsConfig field to ComponentProperties feat(types): Add LimitsConfig field to ComponentProperties May 20, 2025
Copy link
Member

@brooksmtownsend brooksmtownsend left a comment

Choose a reason for hiding this comment

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

Only a spelling nit! LGTM once @thomastaylor312 's comments are addressed.

/// these values at runtime using `wasmcloud:secrets/store`.
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub secrets: Vec<SecretProperty>,
/// This Config holds the componet's metadata properties like memory limits and execution time limits
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// This Config holds the componet's metadata properties like memory limits and execution time limits
/// This Config holds the component's metadata properties like memory limits and execution time limits

@Aditya1404Sal
Copy link
Author

Only a spelling nit! LGTM once @thomastaylor312 's comments are addressed.

Got it—I'll have those resolved ASAP 🫡

…ular per-component resource control

Signed-off-by: Aditya <[email protected]>
@Aditya1404Sal Aditya1404Sal force-pushed the meta_component_properties branch from 8c9ae26 to 6c50ea6 Compare May 30, 2025 08:31
id: option<string>,
config: list<config-property>,
secrets: list<secret-property>,
limits: list<limits-config>,
Copy link
Contributor

Choose a reason for hiding this comment

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

We should bump the version of the package and add a since annotation on these. @brooksmtownsend any preference on whether we do a minor bump or a patch bump? I think with wit, if you add something to a record it is a breaking change, right?

@Aditya1404Sal
Copy link
Author

@thomastaylor312
Just to clarify on the decisions taken in this PR -- max-memory-limit should be parsed as a bytesize string -- something like "10MB" or "1.5GB" and max-execution-time be parsed as humantime values like "15s" or " 140ms" and that's why I've chosen option as the preferred type ,so I still need to implement some additional conversion logic for the ScalerConfig

Is it alright to proceed further with these considerations or keep a u64 type and specifically instruct users on a set unit like megabytes and milliseconds

@thomastaylor312
Copy link
Contributor

Sorry @Aditya1404Sal this fell through the cracks. I think human readable/parsable formats are great so go ahead with that

- Modify LimitsConfig to wadm-types for uni-config scenarios only
- Update ScaleComponent command to include limits field
- Add compute_limits function in scaler/convert.rs using parse-size crate for validation
- Support human-readable formats like 1GB, 512MB, 30s for manifest configuration~

Signed-off-by: Aditya <[email protected]>
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.

3 participants