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

why two function for default_roll_up and last_over_time #5499

Closed
3 tasks
changshun-shi opened this issue Dec 20, 2023 · 3 comments
Closed
3 tasks

why two function for default_roll_up and last_over_time #5499

changshun-shi opened this issue Dec 20, 2023 · 3 comments
Assignees
Labels
question The question issue

Comments

@changshun-shi
Copy link

Is your question request related to a specific component?

vmselect

Describe the question in detail

According to this issue(#1526),
The code became as follows. but I don't know the difference between them, they looks the same.

func rollupDefault(rfa *rollupFuncArg) float64 {
	values := rfa.values
	if len(values) == 0 {
		// Do not take into account rfa.prevValue, since it may lead
		// to inconsistent results comparing to Prometheus on broken time series
		// with irregular data points.
		return nan
	}
	// Intentionally do not skip the possible last Prometheus staleness mark.
	// See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1526 .
	return values[len(values)-1]
}

func rollupLast(rfa *rollupFuncArg) float64 {
	values := rfa.values
	if len(values) == 0 {
		// Do not take into account rfa.prevValue, since it may lead
		// to inconsistent results comparing to Prometheus on broken time series
		// with irregular data points.
		return nan
	}
	return values[len(values)-1]
}

So why two same functions instead of one before the commit for the 1526 issue

Troubleshooting docs

@hagen1778
Copy link
Collaborator

So why two same functions instead of one before the commit for the 1526 issue

I don't know answer to this. I've created a PR and mentioned the author of this change.

@changshun-shi
Copy link
Author

👍

@hagen1778
Copy link
Collaborator

@changshun-shi the duplicate was removed in #5502. Thanks for reporting!

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

No branches or pull requests

2 participants