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

feat: Prevent OOM in NoticeContainer and speed up hasValidationErrors #895

Merged
merged 1 commit into from
May 6, 2021

Conversation

aababilov
Copy link
Collaborator

@aababilov aababilov commented May 5, 2021

Store up to 10 M validation notices. This is a measure to prevent OOM
in the rare case when each row in a large file (such as stop_times.txt
or shapes.txt) produces a notice. Since this case is rare, we just
introduce a total limit on the amount of notices instead of counting
amount of notices of each type.

Note that system errors are not limited since we don't expect to have
a lot of them.

Now we have a boolean field to flag if there are validation errors. This
way hasValidationErrors() now takes O(1) instead of O(n).

We also documented the methods and return unmodifiableList from getters.
We cannot use ImmutableList since the notice lists are always growing.

Store up to 10 M validation notices. This is a measure to prevent OOM
in the rare case when each row in a large file (such as stop_times.txt
or shapes.txt) produces a notice. Since this case is rare, we just
introduce a total limit on the amount of notices instead of counting
amount of notices of each type.

Note that system errors are not limited since we don't expect to have
a lot of them.

Now we have a boolean field to flag if there are validation errors. This
way hasValidationErrors() now takes O(1) instead of O(n).
Copy link
Contributor

@lionel-nj lionel-nj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on that! LGTM

@aababilov
Copy link
Collaborator Author

Thanks for review!

@aababilov aababilov merged commit 2f1f71f into MobilityData:master May 6, 2021
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