-
Notifications
You must be signed in to change notification settings - Fork 14
Limit state size in runtime #1209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Handling the options here is a nightmare. I'l gonna remove the default one from the runtime - it doesn't make any damn sense, jim |
I don't think this is the right option in the right place - I'll check into that later
|
Ok so this new limit should be working in the engine and runtime, with tests and a not totally insane API Tomorrow I gotta tidy up the worker and get that configured correctly. Don't want to invest much in the API but I do want to be sure it's working |
|
Ok, I've fixed the size to either double the dataclip limit or 25% of the available memory limit, whichever is bigger. That should give everyone enough to play with. I have not exposed these as options for now, because there's a lot of ceremony associated with that. I may do it in the morning. I'm also a little unhappy that this limit isn't logged anywhere. I don't want to put it in the run log with the timeout etc. Maybe I'll log it to GCP. |
Drop the payload size thing - runtime memory will always be higher
| profilePollInteval: context.options.profilePollInterval, | ||
| // work out the max size of the state object at the end of each step | ||
| // This must be fairly high to prevent crashes | ||
| stateLimitMb: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is currently set in the engine and worker with different rules!
Maybe the worker just passes in a fixed value, if it has one, and the engine defaults to a % of runtime memory. That's probably cleanest.
Short Description
Adds a test to the runtime which measures the size of state objects before writing them to state. If they're too big, it'll throw an error.
Fixes #1203
Implementation Details
One big concern here: if the serializer fails to stringify, it seems to throw an uncatchable error, which WILL result in the run being lost. You can reproduce this by commenting out the Promise handling in the replacer. The test just times out. I am unable to trap this error anywhere.
QA Notes
List any considerations/cases/advice for testing/QA here.
AI Usage
Please disclose how you've used AI in this work (it's cool, we just want to know!):
You can read more details in our Responsible AI Policy