Skip to content

Increase AQL memory limit to 1GiB from 20MB#88

Merged
JackWilb merged 1 commit intomainfrom
inc-query-mem-limit
Jan 19, 2022
Merged

Increase AQL memory limit to 1GiB from 20MB#88
JackWilb merged 1 commit intomainfrom
inc-query-mem-limit

Conversation

@JackWilb
Copy link
Copy Markdown
Member

@JackWilb JackWilb commented Dec 3, 2021

Depends on #89

This fixes an issue I've been having on UPDB with applying filters to a query. When I would add the filters, I'd hit this memory limit.

Increasing the limit here fixes the problem. There are still issue on the main client with showing the number of nodes and the "node types". I think that we can modify the queries that run on /nodes/?offset=0&limit=10 to reduce the memory use, since it shouldn't need to load every table to get the count.

@JackWilb JackWilb requested review from jjnesbitt and waxlamp December 3, 2021 23:43
@jjnesbitt
Copy link
Copy Markdown
Member

This would likely not fix the issue in production if you truly do need the whole 1GB, as currently our prod dyno only has 512MB of RAM. I think increasing it from 20MB is a good idea though, so if you think you problem is solved with less than 512MB of RAM, I'd say let's adjust this to meet that requirement and proceed. If 512MB still isn't enough, we might have to consider upgrading our dyno type.

@JackWilb
Copy link
Copy Markdown
Member Author

JackWilb commented Dec 6, 2021

This would likely not fix the issue in production if you truly do need the whole 1GB, as currently our prod dyno only has 512MB of RAM. I think increasing it from 20MB is a good idea though, so if you think you problem is solved with less than 512MB of RAM, I'd say let's adjust this to meet that requirement and proceed. If 512MB still isn't enough, we might have to consider upgrading our dyno type.

The memory limit isn't directly used to change the amount of memory the dyno uses, it controls how much memory can be use on the arango server to generate the query results. The use case here is that the query takes around 1GB to run there and returns only 100 rows, which is far smaller than the 512MB of RAM on the dyno.

I'm not sure we should be using memory_limit to help save us from RAM issues on the dyno, since that severely limits the types of queries we can send to arango. Maybe we could use a different parameter to handle larger query sets (batch_size, possibly)? Here's a link to the docs (search for memory_limit to see the relevant lines): https://buildmedia.readthedocs.org/media/pdf/python-driver-for-arangodb/latest/python-driver-for-arangodb.pdf

@jjnesbitt
Copy link
Copy Markdown
Member

The memory limit isn't directly used to change the amount of memory the dyno uses, it controls how much memory can be use on the arango server to generate the query results. The use case here is that the query takes around 1GB to run there and returns only 100 rows, which is far smaller than the 512MB of RAM on the dyno.

I'm not sure we should be using memory_limit to help save us from RAM issues on the dyno, since that severely limits the types of queries we can send to arango. Maybe we could use a different parameter to handle larger query sets (batch_size, possibly)? Here's a link to the docs (search for memory_limit to see the relevant lines): buildmedia.readthedocs.org/media/pdf/python-driver-for-arangodb/latest/python-driver-for-arangodb.pdf

I see, my bad then. In that case, I think this is fine. If we need to constrict memory on the dyno itself, I think that can be addressed separately.

@jjnesbitt
Copy link
Copy Markdown
Member

I think I know why the tests are failing, so I can fix that up, and then approve this.

@JackWilb JackWilb force-pushed the inc-query-mem-limit branch from e035c3e to 6cebc13 Compare January 19, 2022 18:39
@JackWilb
Copy link
Copy Markdown
Member Author

Rebased onto main to trigger testing again

@JackWilb
Copy link
Copy Markdown
Member Author

Since the tests are still failling because of s3_file_field, I'll merge

@JackWilb JackWilb merged commit 80b6e47 into main Jan 19, 2022
@JackWilb JackWilb deleted the inc-query-mem-limit branch January 19, 2022 18:43
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.

2 participants