Graylog2 / Graylog2/graylog2-server
Review `setInterval` usage for recurring requests
- Dominant language
- Java
- Stars
- 8.1k
- Forks
- 1.1k
- Avg merge
- 1d 20h
- Merged PRs (30d)
- 217
Description
We have various polling requests across the front end that either keep content on pages up to date or poll for some notification.
## Expected Behavior
In case the response times increase due to excessive load on Graylog servers or downstream services, the front end should take care that it doesn't schedule recurring requests concurrently.
Those will only increase the load and use up threads from pools without positively affecting the user experience.
Specifically, any code path that uses `setInterval` with API calls needs to take care not to schedule more requests is there are outstanding promises.
## Current Behavior
Some existing calls already consider in-flight requests, most notably the critical metrics and notification polling.
However, there are various places where that isn't the case (e.g., System messages on the Overview page and many other more page-specific calls).
## Possible Solution
Raw use of `setInterval` with API calls is likely a code smell, because it appears to be fast and harmless, when in fact in production it can lead to resource exhaustion.
There are legitimate `setInterval` use cases, e.g. if the body of the callback is not an IO operation, but for API calls extra care needs to be taken.
Ideally we would introduce some kind of abstraction to make polling calls safe to use, and add a linter check to safeguard against `setInterval` usage (allowing it where its use is legitimate).
## Steps to Reproduce (for bugs)
1. add `Thread.sleep(5000);` to https://github.com/Graylog2/graylog2-server/blob/8ce9b3f5323b52e43c59c26777688015236d4ef9/graylog2-server/src/main/java/org/graylog2/rest/resources/system/MessagesResource.java#L74
2. Visit the System/Overview page
3. Open the network tab in the browser dev tools
4. Observe multiple pending requests for the endpoint.
Contributor guide
Assessment
This issue has not been assessed yet.