feat: ensure max state memory size#1458
Conversation
doc-han
changed the title
|
@doc-han this looks great but I think we can add a better test to make sure the limit is set and working. Probably in integration tests? Just set the limit quite low and build a state object big enough to trigger it. Search for Array.fill for patterns of this sort of thing |
|
There was a flaky test on dataclip loading which I've resolved on this PR |
|
@doc-han just a changeset and that log line needed and we can release this. Thank you, nice work! |
|
Ready! |
| }) | ||
| .option('state-memory', { | ||
| description: | ||
| 'Maximum size of the state object returned by each step, in mb. Defaults to 25% of run-memory. Env: WORKER_MAX_STATE_MEMORY_MB', |
There was a problem hiding this comment.
I want to think about this before merging
I'd missed/forgotten that we're doing this already. Which means the limit in prod right now isn't 500mb (as is the default in the function), it's more like 250mb. And that might be big enough to cause problems still.
Should we reduce the default to 10% of working memory? Is that too strict and small?
Also should we be adding some logging around this? For users or GCP?
I do see a logger.debug line which prints the state size limit but I can't see that being logged in production - what's that all about?
josephjclark
left a comment
josephjclark
left a comment
There was a problem hiding this comment.
Picked up a couple of questions in a final review pass before merging. I need to think about these before going ahead.
3 participants
Short Description
This PR completes the plumbing required to get state limit set for a worker.
2 ways to go about it
state_limit_mbfrom Lightning--state-memory/WORKER_MAX_STATE_MEMORY_MBFixes #1459
Implementation Details
A more detailed breakdown of the changes, including motivations (if not provided in the issue).
QA Notes
List any considerations/cases/advice for testing/QA here.
AI Usage
Please disclose whether you've used AI anywhere in this PR (it's cool, we just
want to know!):
You can read more details in our
Responsible AI Policy