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.

Sign up for GitHub

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

fix(MobilityDataValidation): slow on large gtfs files #525

Closed
wants to merge 4 commits into from
Closed

fix(MobilityDataValidation): slow on large gtfs files #525

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e958960
fix(MobilityDataValidation): slow on large gtfs files
miles-grant-ibigroup Mar 24, 2023
0ebf222
refactor: correct comment
miles-grant-ibigroup May 24, 2023
9e411d9
Merge branch 'dev' into mb-validator-slow-on-large-gtfs
miles-grant-ibigroup May 24, 2023
638c7a8
Revert "refactor: correct comment"
miles-grant-ibigroup May 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/main/java/com/conveyal/datatools/manager/models/FeedVersion.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,8 +393,8 @@ public void validateMobility(MonitorableJob.Status status) {
status.update("MobilityData Analysis...", 11);

// Wait for the file to be entirely copied into the directory.
// TODO: base this on the file being completely saved rather than a fixed amount of time.
Thread.sleep(5000);
// 5 seconds + ~1 second per 10mb
Thread.sleep(5000 + (this.fileSize / 10000));
Copy link
Contributor

Choose a reason for hiding this comment

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

What unit is this.fileSize? According to the code, it is set with [newGtfs]File.length() which is in bytes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure, but this does fix things

Copy link
Contributor

Choose a reason for hiding this comment

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

@miles-grant-ibigroup Can you fix the comment so it corresponds to the code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did I get the decimal math right in 0ebf222?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm sorry, the initial comment was correct, juggling with the units was confusing (10,000,000 / 10,000 = 1000).

File gtfsZip = this.retrieveGtfsFile();
// Namespace based folders avoid clash for validation being run on multiple versions of a feed.
// TODO: do we know that there will always be a namespace?
Expand Down