under-pressure
under-pressure copied to clipboard
replace setInterval with a setTimeout
This PR fixes https://github.com/fastify/under-pressure/issues/164 replacing the setInterval timer with setTimeout.
Checklist
- [x] run
npm run test
andnpm run benchmark
- [ ] tests and/or benchmarks are included
- [x] documentation is changed or added
- [x] commit message and code follows the Developer's Certification of Origin and the Code of conduct
At the moment the only missing part is the implementation of the unit tests for this fix. Do you have any suggestions on which test I should create? @mcollina
I would just use a mock to verify this.
I think it needs to document the usage of
setTimeout
. It is not truly identical tosetInterval
and the user need to know that theinterval
is not guarantee to be run in exact same rate when system is under pressure / long running process. The choice is based on we do not want to add more pressure to the system.I think here also worth this changes.
https://github.com/fastify/under-pressure/blob/a0c3b2062cdf522c1d74acc3a79923876aa5495c/index.js#L85-L86
I addressed the requested changes @climba03003
I switched this PR into a Draft PR because some analysis on the issue are still in progress.
Can we merge master into this PR, let the pipeline run and then if it passes, squash merge, please?
Can we merge master into this PR, let the pipeline run and then if it passes, squash merge, please?
What does this mean?
I just did this in my repo to check if it really merges correct.
LGTM
If we have these concerns, we should consider enabling the setting in repos which require PR branches to be up to date with the base branch of a PR
I was just worried in this case, as I modified the tests few days heavily. No worries.