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

GetInstancesMaxLag occasionally unnecessarily noisy #180

Open
sjmudd opened this issue Mar 10, 2016 · 1 comment
Open

GetInstancesMaxLag occasionally unnecessarily noisy #180

sjmudd opened this issue Mar 10, 2016 · 1 comment

Comments

@sjmudd
Copy link
Contributor

sjmudd commented Mar 10, 2016

This code looks wrong given the way it's called in various places.

// GetInstancesMaxLag returns the maximum lag in a set of instances
func GetInstancesMaxLag(instances [](*Instance)) (maxLag int64, err error) {
        if len(instances) == 0 {
                return 0, log.Errorf("No instances found in GetInstancesMaxLag")
        }
...
}

It looks to me as if len(instances) may be 0 in some cases. I've seen this error and think that probably in this case you should just return 0, nil as otherwise you need to special case various routines that call this to check first.

@shlomi-noach
Copy link
Contributor

Returning 0 means there a 0 lag. But there isn't such a lag.

However do notice that the functions that call this code use:

    if err != nil {
        return 0, err
    }

as in https://github.com/outbrain/orchestrator/blob/master/go/inst/instance_dao.go#L1112-L1114

Therefore you get the 0 you wished for, and it's on you to care or not care about the error.

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

No branches or pull requests

2 participants