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

Change logic for determining minimum polling frequency. #823

Merged
merged 1 commit into from
Jul 20, 2015

Conversation

rjnagal
Copy link
Contributor

@rjnagal rjnagal commented Jul 20, 2015

Since polling is tied to housekeeping, minimum supported polling
frequency is 1s.

Users can specify polling frequency higher than 1s. The polling loop
will be called at the minimum frequency specified in config as long as
its higher than the minimum supported frequency.

Since polling is tied to housekeeping, minimum supported polling
frequency is 1s.

Users can specify polling frequency higher than 1s. The polling loop
will be called at the minimum frequency specified in config as long as
its higher than the minimum supported frequency.
@rjnagal
Copy link
Contributor Author

rjnagal commented Jul 20, 2015

@anushree-n I think this is the model we want to go with. WDYT?

@@ -85,6 +82,12 @@ func NewCollector(collectorName string, configfile string) (*GenericCollector, e
}
}

// Minimum supported polling frequency is 1s.
minSupportedFrequency := 1 * time.Second
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory it should be the value of minHousekeeping flag :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, but importing HousekeepingInterval adds a cyclic dependency between manager and collector. I'll clean that up next.

@anushree-n
Copy link
Contributor

I believe the earlier model took the minimum of next stats collection and next custom metrics collection as the next housekeeping time.

If the minimum supported frequency is 1s, yes, this looks good! :)

@rjnagal
Copy link
Contributor Author

rjnagal commented Jul 20, 2015

yeah, the intention is to not go below the housekeeping interval.

@rjnagal
Copy link
Contributor Author

rjnagal commented Jul 20, 2015

self-merging ....

rjnagal added a commit that referenced this pull request Jul 20, 2015
Change logic for determining minimum polling frequency.
@rjnagal rjnagal merged commit 7ec04b7 into google:master Jul 20, 2015
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 this pull request may close these issues.

3 participants