Skip to content

Commit

Permalink
Remove import jobs stats from /health/info (#1336)
Browse files Browse the repository at this point in the history
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

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](DEFRA/water-abstraction-import#1023), 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](https://eaflood.atlassian.net/browse/WATER-4535). Either way, the block of job stats we display on the `/health/info` page 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/info` page in readiness.
  • Loading branch information
Cruikshanks authored Sep 26, 2024
1 parent a38973c commit 87442cf
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 280 deletions.
95 changes: 0 additions & 95 deletions app/services/health/fetch-import-jobs.service.js

This file was deleted.

31 changes: 5 additions & 26 deletions app/services/health/info.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,23 @@ const exec = util.promisify(ChildProcess.exec)

const ChargingModuleRequest = require('../../requests/charging-module.request.js')
const CreateRedisClientService = require('./create-redis-client.service.js')
const FetchImportJobsService = require('./fetch-import-jobs.service.js')
const FetchSystemInfoService = require('./fetch-system-info.service.js')
const { formatLongDateTime } = require('../../presenters/base.presenter.js')
const BaseRequest = require('../../requests/base.request.js')
const LegacyRequest = require('../../requests/legacy.request.js')

const servicesConfig = require('../../../config/services.config.js')

/**
* Checks status and gathers info for each of the services which make up WRLS
*
* Returns data required to populate our `/service-status` page, eg. task activity status, virus checker status, service
* version numbers, etc.
*
* Each data set is returned in the format needed to populate the gov.uk table elements ie. an array containing one
* array per row, where each row array contains multiple `{ text: '...' }` elements, one for each cell in the row.
*/
*
* @returns {object} data about each service formatted for the view
*/
async function go () {
const addressFacadeData = await _getAddressFacadeData()
const chargingModuleData = await _getChargingModuleData()
Expand Down Expand Up @@ -83,7 +85,6 @@ async function _getLegacyAppData () {
if (result.succeeded) {
service.version = result.response.body.version
service.commit = result.response.body.commit
service.jobs = service.name === 'Import' ? await _getImportJobsData() : []
} else {
service.version = _parseFailedRequestResult(result)
service.commit = ''
Expand All @@ -103,16 +104,6 @@ async function _getChargingModuleData () {
return _parseFailedRequestResult(result)
}

async function _getImportJobsData () {
try {
const importJobs = await FetchImportJobsService.go()

return _mapArrayToTextCells(importJobs)
} catch (error) {
return []
}
}

async function _getRedisConnectivityData () {
let redis

Expand Down Expand Up @@ -141,18 +132,6 @@ async function _getVirusScannerData () {
}
}

function _mapArrayToTextCells (rows) {
return rows.map((row) => {
return [
...[{ text: row.name }],
...[{ text: row.completedCount }],
...[{ text: row.failedCount }],
...[{ text: row.activeCount }],
...[{ text: row.maxCompletedonDate ? formatLongDateTime(row.maxCompletedonDate) : '-' }]
]
})
}

function _parseFailedRequestResult (result) {
if (result.response.statusCode) {
return `ERROR: ${result.response.statusCode} - ${result.response.body}`
Expand Down
63 changes: 19 additions & 44 deletions app/views/info.njk
Original file line number Diff line number Diff line change
Expand Up @@ -26,55 +26,30 @@
<p class="govuk-body">{{ chargingModuleData }}</p>

{% for app in appData %}
<hr class="govuk-section-break govuk-section-break--m govuk-section-break--visible">
<hr class="govuk-section-break govuk-section-break--m govuk-section-break--visible">

<h2 class="govuk-heading-m">{{ app.name }}</h2>
{{ govukSummaryList({
classes: 'govuk-summary-list--no-border',
rows: [
{
key: {
text: "Version"
},
value: {
text: app.version
}
},
{
key: {
text: "Commit hash"
},
value: {
text: app.commit
}
}
]
}) }}

{% if app.jobs.length %}
{{ govukTable({
firstCellIsHeader: false,
head: [
<h2 class="govuk-heading-m">{{ app.name }}</h2>
{{ govukSummaryList({
classes: 'govuk-summary-list--no-border',
rows: [
{
text: "Import task name (data for the last 24 hrs)"
key: {
text: "Version"
},
value: {
text: app.version
}
},
{
text: "Completed"
},
{
text: "Failed"
},
{
text: "Active"
},
{
text: "Last job completed"
key: {
text: "Commit hash"
},
value: {
text: app.commit
}
}
],
rows: app.jobs
]
}) }}
{% endif %}

{% endfor %}
{% endfor %}
</div>
{% endblock %}
Loading

0 comments on commit 87442cf

Please sign in to comment.