Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configurable TimeMonitor interval #925

Closed
boaz-wiesner opened this issue Nov 2, 2022 · 4 comments · Fixed by #926
Closed

Configurable TimeMonitor interval #925

boaz-wiesner opened this issue Nov 2, 2022 · 4 comments · Fixed by #926

Comments

@boaz-wiesner
Copy link

At the moment, the TimeMonitor interval is hardcoded to 1 minute. In druid clusters with many datasources, running a full cycle of time checks can take longer than a minute, and lead to continuous polling of the cluster, which isn't optimal.

Moving this to a cluster level configurable parameter would definitely solve the problem.

The relevant line seems to be:

this.regularCheckInterval = 60000;

Thanks!

@boaz-wiesner
Copy link
Author

boaz-wiesner commented Nov 3, 2022

Another default that is currently hardcoded is the default "time" dimension (timeAttribute). Introspected sources end up using this default, and currently there's no way to configure it.

Allowing this default to be changed cluster-wide would enable automatic introspection of datasources not using __time as their time dimension. At the moment, if you have an introspected source that uses a different dimension, it won't work.

const timeAttribute = $("__time");

@adrianmroz-allegro
Copy link
Contributor

I think cluster level property in config for regularCheckInterval sounds good. As always we need to figure out good name. sourceMaxTimeRefreshInterval? MaxTime is turnilo centric, that how we talk about it in codebase. Maybe there is better term in Druid lingo? @mkuthan wanna weigh in? sourceTimeBoundaryRefreshInterval?

@adrianmroz-allegro
Copy link
Contributor

Another default that is currently hardcoded is the default "time" dimension (timeAttribute).

That's deliberate. To my understanding __time attribute name is required by Druid. It is a special dimension used to partition data. In theory you could have additional "time" dimensions but turnilo can't work with them, we handle only one time dimension that's partitioning column.

@boaz-wiesner
Copy link
Author

As always we need to figure out good name. sourceMaxTimeRefreshInterval? MaxTime is turnilo centric, that how we talk about it in codebase. Maybe there is better term in Druid lingo? sourceTimeBoundaryRefreshInterval?

I'm for something like timeBoundaryQueryInterval or sourceTimeBoundaryRefreshInterval, but as long as the name is documented then I don't have a strong opinion one way or the other.

That's deliberate. To my understanding __time attribute name is required by Druid

Yep, my bad - we use alternative fields for additional partitioning and applicative uses, but for Turnilo's use case of running a TimeBoundary query it should be enough.

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

Successfully merging a pull request may close this issue.

2 participants