uptime-kuma
uptime-kuma copied to clipboard
louislam
Reduce monitor minimum interval to 1 second
Description
This will change the minimum interval value from 20 to 1 second. However, the user will be warned if they reduce the minimum interval below 20 seconds that performance may suffer
Fixes #1645
Type of change
Please delete any options that are not relevant.
- User interface (UI)
Checklist
- [x] My code follows the style guidelines of this project
- [x] I ran ESLint and other linters for modified files
- [x] I have performed a self-review of my own code and tested it
- [x] I have commented my code, particularly in hard-to-understand areas (including JSDoc for methods)
- [x] My changes generate no new warnings
Please fix ESLint warnings
Oops, sorry about that. I saw that the test had passed and didn't think to check the output. I wonder if it might be an idea to fail the test on warnings as well as errors. This would be a good spot if github had another result type for warnings.
@louislam is there a reason this has not yet been merged? This is something that's preventing deployment for a couple sites for us.
Hello, I have downloaded version 1.17.1 for docker, but when in the type of monitor that is ping, I set the heartbeat interval value to 1 second, it does not let me advance but asks me for at least 20 seconds.
As I read this feature was added, but in ping it does not allow me.
Could it be that this added does not contemplate the ping in 1 second?
Greetings
This has not yet been merged and as such is not present in this release
I may prefer making this harder to be set. Maybe we can add a env var UPTIME_KUMA_DISABLE_INTERVAL_LIMIT. If user set it to true, they can set a small interval.
Why? There will just be tutorials about how to do it and everyone will do it anyway.
I think I have to agree with @ylluminate. Adding an env var would I think over complicate setting a smaller value. I don't really know what adding another environment variable would accomplish here other than making it harder to set. Whilst making it harder to set could prevent a user setting the value too low and then encountering performance issues, I think that the warning also serves the same purpose without adding extra complexity.
It is because setting it to very low likely causes issues such as random timeout, very high cpu usage, very high database usage and the database locking issue. Uptime Kuma also is not designed for that purpose.
If it is too easy for users to use this feature. They will likely forget your warning message. Then they will just create a bug report here and think Uptime Kuma is unstable.
So I want this to be a hidden feature.
In this case, it would be important that this feature is activated in the graphical environment and that when doing so, a large warning appears indicating the possible consequences. And the way to disable it manually in case of failures is documented. You can even set as experimental function.
I just tested it with 1 second, it was so unstable and my Uptime Kuma was under heavy loading and slow response. The monitor is dead even I changed back to 3600s. So that why I want it to to harder be set or invisible to normal users.
Using env var is just a suggestion. Other methods could be considered.
In this case, it would be important that this feature is activated in the graphical environment and that when doing so, a large warning appears indicating the possible consequences. And the way to disable it manually in case of failures is documented. You can even set as experimental function.
It is also a possible way, just like chrome://flags/.
I see where you are comming from. I certainly think it should be a graphical option over an environment variable. Perhaps if the user sets the value to below 20, they have to click through a prompt when saving to reinforce the message that this is a possibly unstable feature.
I see where you are comming from. I certainly think it should be a graphical option over an environment variable. Perhaps if the user sets the value to below 20, they have to click through a prompt when saving to reinforce the message that this is a possibly unstable feature.
Yes, it would be a good, very visual way of warning the user about the change they are going to make.
I have now added a confirmation dialog for when the use sets the interval to below 20.
Technical description from commit message:
A prompt has been added to confirm with the user that they wish to reduce the interval value to possibly unstable values. A random (pseudo random) code is generated which they must input in order to confirm their descision. Failiure to confirm their descision will result in an error when saving. As not to annoy the user too much, the promt is only required when it is detected that the interval value has been changed.
Screenshot
This PR should now be ready for review.
@louislam Could you take another look at this. The user now has to confirm the change by inputing a random code. I think this should provide a good balance between allowing users to reduce the interval below 20 seconds and ensuring that users understand that it could lead to instability.
Fixed the merge conflicts + updated util.ts to match new min/max settings
Hi, I would like to try out this PR but I'm struggling to build a docker container. Could you describe high level (details are welcome but I'll try to figure it out) how I should go about this?
What I tried:
- build an image using the
docker/dockerfile(I kept only build_healthcheck, build and release in the file). But when I try to run a container from the image I get this error message:Cannot find 'dist/index.html', did you install correctly - build an image from my own docker file where I start from
louislam/uptime-kuma:1and replaced theapp/srcdirectory with the one from this PR. I can run a container from this image but still the UI won't allow me to set anything below 20 seconds.
Thanks!
You should be able to use the pre built docker image for this PR by following these instructions in the wiki https://github.com/louislam/uptime-kuma/wiki/Test-Pull-Requests
@louislam Is the new system of making the user enter a code to confirm that they want to reduce the minimum interval value OK? I think that this provides a good balance between allowing users to set the interval lower if they want whilst showing that this is not supported.
I think it will be likely running into issue after a period of time with low interval. I think the performance issue is caused by the bad implementation of uptime calculation.
I hope I can fix it first (#2750) before this pr.
Hello. Looks like #2750 might be getting fixed with 1.23? Will the under 20 seconds feature be released with that also by chance?
Thanks.
Loking at https://github.com/louislam/uptime-kuma/milestone/34, I think those would be a lot of changes. @Computroniks Could you Rebase your PR on https://github.com/louislam/uptime-kuma/pull/2750?
This would allow for checking if This change needs to wait for the changes proposed in https://github.com/louislam/uptime-kuma/issues/3286 (help validating these claims would be highly appreciated ^^)
Could you add the following to the Readme: Resolves #1015
Yep, just done that. Sorry for taking a while to respond, I have been working on other projects for work and school which have taken up all of my free time. I will rebase off #2750 later today probably and get this PR ready again.
No worries, this would ship in 2.0 anyway at the earliest, given that some of the performance fixes will not materialsie until then.
And I kind of fucked up:
i just had another look at #2750
=> Currently this is an empty template
=> Rebasing really does not change anything
=> We are going to have to wait until the performance issue is resolved (a few performance improvements have gone into 1.23. Could you rebase on that release and check if the fixes in master already were okay for ~100 monitors?)
If this gets added, could you also add something, that prevents Uptime-Kuma to load always every single ping, even if the chart is set to one week, because first, this is way too much to load, which causes big performance issues for both client- and server-side.
For understanding, i set my ping delay to 20 seconds, and this system is up for 1.5 days and my browser almost crashes, when i load the one week chart.
Edit: This chart above is a single 1MB RAW request in the SOCKET connection. With a full week of logs, it would be ~4.5MB
The new Chart implementation in #4264 will not be loading individual pings anymore.
