Remove import jobs stats from /health/info #1336
Merged
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
https://eaflood.atlassian.net/browse/WATER-4670 After making some changes to [water-abstraction-import](https://github.com/DEFRA/water-abstraction-import) we found it was no longer completing when run. The issue seemed to be - a step we introduced to let [water-abstraction-system](https://github.com/DEFRA/water-abstraction) see importing licences was overloading the environment because it was acting like a DOS on the system app - each job was taking much longer than realised to complete, and the existing job schedule was overlapping them, again leading to limited system resources Initially, we were going to create a new 'job' that would daisy chain the existing jobs to avoid the overlap. Plus we'd drop the step that pinged the **water-abstraction-system** app. But then we realised based on current timings the whole process risked still running when users logged on in the AM. So, we've always suspected that the features the previous team added to improve performance (pg-boss message queues and caching to redis) were actually impacting it. We've now gone through all the 'jobs' that were involved in importing data from NALD and stripped out this overhead. Our timings are much, _much_ better proving our suspicions. What this means though is the block of job stats we display in the `/health/info` page is now redundant. In fact, with the jobs not being there when the update **water-abstraction-import** ships this page will error because the attempt to retrieve them will fail. So, this strips out all logic related to job info from the `/health/info` page.
Cruikshanks
added
the
housekeeping
Cruikshanks
requested review from
robertparkinson,
Jozzey,
jonathangoulding,
Beckyrose200 and
rvsiyad
jonathangoulding
approved these changes
Sep 26, 2024
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.
https://eaflood.atlassian.net/browse/WATER-4670
After making some changes to water-abstraction-import we found it was no longer completing when run. The issue seemed to be
We've always suspected that the features the previous team added to improve performance (pg-boss message queues and caching to Redis) were actually impacting it.
Initially, we planned to go through all the 'jobs' involved in importing data from NALD and strip out this overhead. We carried out a spike, and our timings are much, much better, proving our suspicions.
However, this was a live issue, so we removed the step and tweaked the times to get the import working again.
In the meantime, we'll either complete the spike or WATER-4535, a replacement for the licence import. Either way, the block of job stats we display on the
/health/infopage will be redundant. In fact, we don't use or look at it anyway, even when there is an issue.So, this removes all logic related to job information from the
/health/infopage in readiness.