Cap concurrent requests to get Neptune schema #58
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, for a graph with `n` nodes or edges, there would be `n` concurrent requests made to Neptune via `queryNeptune`. Depending on your instance class or VPC settings, you could hit errors such as `MemoryLimitExceededException` or connection limits/sockets abruptly closing, with increasing likelihood as the size of the graph increased. Initially observed abrupt socket closing just like aws#20. Now, the concurrent request Promises are resolved in batches using `mapAll`. For now, the batch size is hardcoded to 20. For very small instance classes (e.g. from the "Development and testing" template), `MemoryLimitExceededException`s are still likely. Further improvements could include: - relating the batch size to the instance class - allowing the user to specify the batch size in the process args - exponential/dynamic backoff on transient errors as indicated in https://docs.aws.amazon.com/neptune/latest/userguide/errors-engine-codes.html#errors-query
andreachild
reviewed
Jan 23, 2025
andreachild
reviewed
Jan 24, 2025
|
LGTM |
Cole-Greer
approved these changes
Jan 24, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #, if available: #20
Description of changes:
Previously, for a graph with
nnodes or edges, there would benconcurrent requests made to Neptune viaqueryNeptune. Depending on your instance class or VPC settings, you could hit errors such asMemoryLimitExceededExceptionor connection limits/sockets abruptly closing, with increasing likelihood as the size of the graph increased. Initially observed abrupt socket closing just like #20.Now, the concurrent request Promises are resolved in batches using
mapAll. For now, the batch size is hardcoded to 20. For very small instance classes (e.g. from the "Development and testing" template),MemoryLimitExceededExceptions are still likely.Further improvements could include:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.