Skip to content
This repository was archived by the owner on Aug 2, 2022. It is now read-only.

file_space_handler extremely verbose message. #9649

Closed
deckb opened this issue Nov 9, 2020 · 12 comments
Closed

file_space_handler extremely verbose message. #9649

deckb opened this issue Nov 9, 2020 · 12 comments
Assignees

Comments

@deckb
Copy link
Contributor

deckb commented Nov 9, 2020

When the threshold is hit there starts to be message every 2 seconds alerting that there will be an issue at some point. It would be nice to have a configuration option that limits the output of this message to a certain cadence or if the message would output every n bytes/KiB/MiB

if ( is_threshold_exceeded() && shutdown_on_exceeded ) {

@bogniq bogniq self-assigned this Dec 10, 2020
@bogniq
Copy link
Collaborator

bogniq commented Dec 11, 2020

@deckb Thank you for reporting this!
There is an option resource-monitor-interval-seconds to set up the interval from 1 sec to 300 sec (default value is 2 sec). Do you think this option is good enough to solve this issue?

auto interval = options.at("resource-monitor-interval-seconds").as<uint32_t>();

(also, I'm working to reproduce this problem and try different options).

@deckb
Copy link
Contributor Author

deckb commented Dec 11, 2020

I'm not sure I want to change how often the check is performed just how often the warning message is posted. Something like resource-monitor-warning-interval where 0 means output every check else output every checks. So if:

resource-monitor-interval-seconds = 2
reousrce-monitor-warning-interval = 30

The message would be every minute. Is that too confusing?

@bogniq
Copy link
Collaborator

bogniq commented Dec 11, 2020

Yes, thank you for reminding that checking and warning need separate interval settings. I think reousrce-monitor-warning-interval can also be in seconds, with some limitations (e.g. it should be no less than resource-monitor-interval-seconds). How do you think?

Also, your alternative suggestion is to set the interval by every n bytes/KiB/MiB. Does it mean the warning is output every time the available space is reduced by n bytes/KiB/MiB? This option vs. warning time interval, which one do you think the user would prefer (or both needed)? Thanks.

@deckb
Copy link
Contributor Author

deckb commented Dec 11, 2020

Yes, thank you for reminding that checking and warning need separate interval settings. I think reousrce-monitor-warning-interval can also be in seconds, with some limitations (e.g. it should be no less than resource-monitor-interval-seconds). How do you think?

That would work as well. The way I described was a bit clunky and even as I wrote it I didn't love my answer.

Also, your alternative suggestion is to set the interval by every n bytes/KiB/MiB. Does it mean the warning is output every time the available space is reduced by n bytes/KiB/MiB?

Yes that is exactly right.

This option vs. warning time interval, which one do you think the user would prefer (or both needed)? Thanks.

If easy to do both I would say both. If one or the other I would choose the time interval.

@bogniq
Copy link
Collaborator

bogniq commented Dec 17, 2020

I'm not sure I want to change how often the check is performed just how often the warning message is posted. Something like resource-monitor-warning-interval where 0 means output every check else output every checks. So if:

resource-monitor-interval-seconds = 2
reousrce-monitor-warning-interval = 30

The message would be every minute. Is that too confusing?

@deckb After discussion with other developers, I think your initial suggestion above is better than setting the warning interval in seconds. The option's name hasn't been finalized, and I'm considering something like warning_every_n_resource_monitor_intervals (too long?). Could you help to compose a better name?

Also, multiple options may confuse the users, so I'd prefer to not adding another option by every n bytes/KiB/MiB. What do you think? Thanks.

@deckb
Copy link
Contributor Author

deckb commented Dec 17, 2020

Also, multiple options may confuse the users, so I'd prefer to not adding another option by every n bytes/KiB/MiB. What do you think? Thanks.

That makes a ton of sense.

The option's name hasn't been finalized, and I'm considering something like warning_every_n_resource_monitor_intervals (too long?). Could you help to compose a better name?

I think it is too long. Imagine typing that on the command-line :).

I think as long as the comment/description is clear the name could be left as resource-monitor-warning-interval

Something like:
// Number of checks that occur between output of warning message. To calculate time between checks: resource-monitor-warning-interval * resource-monitor-interval-seconds. By default set to 30

@bogniq
Copy link
Collaborator

bogniq commented Dec 17, 2020

Thank you. Also, what do you think is an appropriate upper limit for resource-monitor-warning-interval? (I'm thinking 100, so if the monitor interval is 2 sec, then the max warning interval is 200 sec, too short?)

@deckb
Copy link
Contributor Author

deckb commented Dec 18, 2020

What is the thought process behind having an upper limit?

@bogniq
Copy link
Collaborator

bogniq commented Dec 18, 2020

@deckb My initial thought is to prevent a user input a huge number (by accident or not), which makes the warning interval too long to be useful (e.g. if the monitor interval is 2 sec, and the warning interval is set to be 100000, then it's more than two days). Do you think such hard-coded upper limit is necessary for user? Thanks.

@deckb
Copy link
Contributor Author

deckb commented Dec 18, 2020

Yes that makes a ton of sense. I do think that 200 seconds is too short. How does either a 15 minutes sound?

@bogniq
Copy link
Collaborator

bogniq commented Dec 18, 2020

Yes, I think 15 minute is good. When using the default 2-sec monitor interval, 15 minutes means the upper limit is 450. I will use this number.

@nksanthosh
Copy link
Contributor

Resolved in #9826

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants