Skip to content
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

Make InvokerOptions.in_memory_queue_length_limit respect global memory limit of a node #2402

Open
tillrohrmann opened this issue Dec 10, 2024 · 2 comments

Comments

@tillrohrmann
Copy link
Contributor

Right now, the invoker uses the SegmentQueue to queue up invocations it cannot immediately execute. The number of in memory invocations is controlled via InvokerOptions.in_memory_queue_length_limit. With the change to run a dedicated invoker per PartitionProcessor, the total memory consumption due to SegmentQueues can be #partitions * in_memory_queue_length_limit * sizeof which by default roughly equals to 7.7 GB of memory. Ideally, we can integrate this memory consumption into the global memory limit of a node (needs to be added).

@slinkydeveloper
Copy link
Contributor

slinkydeveloper commented Dec 11, 2024

IDK if this makes so much sense, isn't there a risk that a single partition overloads this in memory limit and all the other invokers always end up with this limit exhausted, writing to disk even though they have capacity?

Maybe the story here is that both the in_memory_queue_length_limit and the invoker concurrency limit should be per node (it also somewhat makes sense, the invoker concurrency limit is bound to open files, which is an OS resource), essentially returning to the behavior before the invoker was made per partition.

I think this brings back the deadlock situation between PP(s), but we have this solved by the inactivity timeout that fires in.

@tillrohrmann
Copy link
Contributor Author

Changing the concurrency limit to be per node makes also sense to me because of what you said.

One way to make the memory consumption configurable per node is to divide the configured memory by the total number of partition processors. This is not ideal if one partition needs more memory and will need support for reconfiguration once we support changing the number of running partition processors per node but at least it would let users set an upper bound. A bit more elaborate approach is to reserve for every partition a min amount of memory and then have a free floating budget that is being taken on a first come first serve basis.

A completely different approach could be to get rid of the SegmentQueue and try to do the queuing of ready invocations in the partition processor if the concurrency limit is reached.

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

No branches or pull requests

2 participants