Skip to content

Commit

Permalink
replace setInterval with a setTimeout (#174)
Browse files Browse the repository at this point in the history 
* replace setInterval with a setTimeout

* replace health check timer with setTimeout

Replace the externalHealthCheckTimer with setTimeout and renamed some methods

* add additional information section in the readme

Added the additional information section in the README file in order to explain why setTimeout has replaced setInterval

* test: add timer reentrance

Co-authored-by: Simone Busoli <simone.busoli@gmail.com>
  • Loading branch information
@davideroffo @simoneb
davideroffo and simoneb authored Aug 24, 2022
1 parent 4d9483d commit 4e73a53
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 63 deletions.
13 changes: 13 additions & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,19 @@ fastify.register(require('@fastify/under-pressure'), {

```
<a name="additional-information"></a>
## Additional information
<a name="set-timeout-vs-set-interval"></a>
#### `setTimeout` vs `setInterval`
Under the hood the `@fastify/under-pressure` uses the `setTimeout` method to perform its polling checks. The choice is based on the fact that we do not want to add additional pressure to the system.
In fact, it is known that `setInterval` will call repeatedly at the scheduled time regardless of whether the previous call ended or not, and if the server is already under load, this will likely increase the problem, because those `setInterval` calls will start piling up. `setTimeout`, on the other hand, is called only once and does not cause the mentioned problem.
One note to consider is that because the two methods are not identical, the timer function is not guaranteed to run at exactly the same rate when the system is under pressure or running a long-running process.
<a name="acknowledgements"></a>
## Acknowledgements
Expand Down
14 changes: 12 additions & 2 deletions index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ async function underPressure (fastify, opts) {

fastify.decorate('memoryUsage', memoryUsage)

const timer = setInterval(updateMemoryUsage, sampleInterval)
const timer = setTimeout(beginMemoryUsageUpdate, sampleInterval)
timer.unref()

let externalsHealthy = false
Expand All @@ -82,7 +82,12 @@ async function underPressure (fastify, opts) {
await doCheck()

if (healthCheckInterval > 0) {
externalHealthCheckTimer = setInterval(doCheck, healthCheckInterval)
const beginCheck = async () => {
await doCheck()
externalHealthCheckTimer.refresh()
}

externalHealthCheckTimer = setTimeout(beginCheck, healthCheckInterval)
externalHealthCheckTimer.unref()
}
} else {
Expand Down Expand Up @@ -155,6 +160,11 @@ async function underPressure (fastify, opts) {
}
}

function beginMemoryUsageUpdate () {
updateMemoryUsage()
timer.refresh()
}

function updateMemoryUsage () {
const mem = process.memoryUsage()
heapUsed = mem.heapUsed
Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
"fastify": "^4.0.0-rc.2",
"semver": "^7.3.2",
"simple-get": "^4.0.0",
"sinon": "^14.0.0",
"snazzy": "^9.0.0",
"standard": "^17.0.0",
"tap": "^16.2.0",
Expand Down
167 changes: 106 additions & 61 deletions test/pressurehandler.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const Fastify = require('fastify')
const { monitorEventLoopDelay } = require('perf_hooks')
const underPressure = require('../index')
const { valid, satisfies, coerce } = require('semver')
const sinon = require('sinon')

const wait = promisify(setTimeout)
const isSupportedVersion = satisfies(valid(coerce(process.version)), '12.19.0 || >=14.0.0')
Expand All @@ -18,92 +19,136 @@ function block (msec) {
}

test('health check', async t => {
t.plan(3)
const fastify = Fastify()
test('simple', async t => {
t.plan(3)
const fastify = Fastify()

fastify.register(underPressure, {
healthCheck: async () => false,
healthCheckInterval: 1,
pressureHandler: (req, rep, type, value) => {
t.equal(type, underPressure.TYPE_HEALTH_CHECK)
t.equal(value, undefined)
rep.send('B')
}
})

fastify.register(underPressure, {
healthCheck: async () => false,
healthCheckInterval: 1,
pressureHandler: (req, rep, type, value) => {
t.equal(type, underPressure.TYPE_HEALTH_CHECK)
t.equal(value, undefined)
rep.send('B')
}
fastify.get('/', (req, rep) => rep.send('A'))

t.equal((await fastify.inject().get('/').end()).body, 'B')
})

fastify.get('/', (req, rep) => rep.send('A'))
test('delayed handling with promise success', async t => {
const fastify = Fastify()

t.equal((await fastify.inject().get('/').end()).body, 'B')
})
fastify.register(underPressure, {
healthCheck: async () => false,
healthCheckInterval: 1,
pressureHandler: async (req, rep, type, value) => {
await wait(250)
rep.send('B')
}
})

test('health check - delayed handling with promise success', async t => {
t.plan(1)
const fastify = Fastify()
fastify.get('/', (req, rep) => rep.send('A'))

fastify.register(underPressure, {
healthCheck: async () => false,
healthCheckInterval: 1,
pressureHandler: async (req, rep, type, value) => {
await wait(250)
rep.send('B')
}
t.equal((await fastify.inject().get('/').end()).body, 'B')
})

fastify.get('/', (req, rep) => rep.send('A'))
test('delayed handling with promise error', async t => {
const fastify = Fastify()

t.equal((await fastify.inject().get('/').end()).body, 'B')
})
const errorMessage = 'promiseError'

test('health check - delayed handling with promise error', async t => {
t.plan(2)
const fastify = Fastify()
fastify.register(underPressure, {
healthCheck: async () => false,
healthCheckInterval: 1,
pressureHandler: async (req, rep, type, value) => {
await wait(250)
throw new Error(errorMessage)
}
})

const errorMessage = 'promiseError'
fastify.get('/', (req, rep) => rep.send('A'))

fastify.register(underPressure, {
healthCheck: async () => false,
healthCheckInterval: 1,
pressureHandler: async (req, rep, type, value) => {
await wait(250)
throw new Error(errorMessage)
}
const response = await fastify.inject().get('/').end()
t.equal(response.statusCode, 500)
t.equal(JSON.parse(response.body).message, errorMessage)
})

fastify.get('/', (req, rep) => rep.send('A'))
test('no handling', async t => {
const fastify = Fastify()

const response = await fastify.inject().get('/').end()
t.equal(response.statusCode, 500)
t.equal(JSON.parse(response.body).message, errorMessage)
})
fastify.register(underPressure, {
healthCheck: async () => false,
healthCheckInterval: 1,
pressureHandler: (req, rep, type, value) => { }
})

test('health check - no handling', async t => {
t.plan(1)
const fastify = Fastify()
fastify.get('/', (req, rep) => rep.send('A'))

fastify.register(underPressure, {
healthCheck: async () => false,
healthCheckInterval: 1,
pressureHandler: (req, rep, type, value) => { }
t.equal((await fastify.inject().get('/').end()).body, 'A')
})

fastify.get('/', (req, rep) => rep.send('A'))
test('return response', async t => {
const fastify = Fastify()

t.equal((await fastify.inject().get('/').end()).body, 'A')
})
fastify.register(underPressure, {
healthCheck: async () => false,
healthCheckInterval: 1,
pressureHandler: (req, rep, type, value) => 'B'
})

test('health check - return response', async t => {
t.plan(1)
const fastify = Fastify()
fastify.get('/', (req, rep) => rep.send('A'))

fastify.register(underPressure, {
healthCheck: async () => false,
healthCheckInterval: 1,
pressureHandler: (req, rep, type, value) => 'B'
t.equal((await fastify.inject().get('/').end()).body, 'B')
})

fastify.get('/', (req, rep) => rep.send('A'))
test('interval reentrance', async t => {
const clock = sinon.useFakeTimers()
t.teardown(() => sinon.restore())

const healthCheckInterval = 500

const fastify = Fastify()

const healthCheck = sinon.fake(async () => {
await wait(healthCheckInterval * 2)
return true
})

t.equal((await fastify.inject().get('/').end()).body, 'B')
fastify.register(underPressure, {
healthCheck,
healthCheckInterval
})

// not called until fastify has finished initializing
sinon.assert.callCount(healthCheck, 0)

await fastify.ready()

// called immediately when registering the plugin
sinon.assert.callCount(healthCheck, 1)

// wait until next execution
await clock.tickAsync(healthCheckInterval)

// scheduled by the timer
sinon.assert.callCount(healthCheck, 2)

await clock.tickAsync(healthCheckInterval)

// still running the previous invocation
sinon.assert.callCount(healthCheck, 2)

// wait until the last call resolves and schedules another invocation
await healthCheck.lastCall.returnValue

await clock.tickAsync(healthCheckInterval)

// next timer invocation
sinon.assert.callCount(healthCheck, 3)
})
})

test('event loop delay', { skip: !monitorEventLoopDelay }, t => {
Expand Down

0 comments on commit 4e73a53

Please sign in to comment.