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

Improve peak flow arrival time sql #335

Closed
CoreyKrewson-NOAA opened this issue Mar 1, 2023 · 4 comments · Fixed by #425
Closed

Improve peak flow arrival time sql #335

CoreyKrewson-NOAA opened this issue Mar 1, 2023 · 4 comments · Fixed by #425
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@CoreyKrewson-NOAA
Copy link
Contributor

The MRF peak flow arrival time sql times out occasionally. This seems to only happen when all MRF sql files are being ran at once so it seems to be just slow enough to timeout but be ok when it is the only thing running. Can we find any efficiency improvements in the sql to stop causing these timeouts?

@TylerSchrag-NOAA
Copy link
Contributor

One reason this service is slow is that it replicates the 'high water return time' logic from the arrival time script, and they often run at the same time (I told Katherine this was OK for now, and that I had this fix in mind for later).

What I plan to do in this next sprint is to add a service dependency column to the admin.services table. I'll setup a check at the start of the postprocess_sql (Service) function that looks for the current reference time in that dependency service table, throws a custom exception if the current reference time isn't present yet, and waits on a retrier. That way I can have this script just wait until high water arrival time runs, and pull that attribute column directly from the publish table.

The extra column could be argued to be messy, but I think this could help us in other ways as well (and I still like that it forces the documentation in one spot, even if the db tables can be a bit messy). And it should only be ~10 lines of code in the lambda function, and remove about 10 in the peak flow sql files.

@TylerSchrag-NOAA TylerSchrag-NOAA added the enhancement New feature or request label Mar 10, 2023
@CoreyKrewson-NOAA
Copy link
Contributor Author

Sounds good. I am a little confused on how these are dependent on each other. They both calculate the same metric (arrival time) but on different data (high water flow vs max flow)

@TylerSchrag-NOAA
Copy link
Contributor

They both calculate and report 'High Water Return Time (hours)'... although peak flow still calls it 'Below Bank Return Time (hours)'... which we should make consistent. Maybe 'High Water End Time (hours)' is better for both.

@CoreyKrewson-NOAA
Copy link
Contributor Author

ahh I see what you mean. Yeah that makes sense

@CoreyKrewson-NOAA CoreyKrewson-NOAA added this to the General Enhancements milestone Mar 10, 2023
@CoreyKrewson-NOAA CoreyKrewson-NOAA modified the milestones: General Enhancements, V2.1.0 Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
2 participants