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

BUG?: why not break after handle all request?(tryRemoveOldApiRequests) #168

Open
trueeyu opened this issue Nov 9, 2015 · 2 comments
Open

Comments

@trueeyu
Copy link

trueeyu commented Nov 9, 2015

why not break after handle all request?

is it usefull to retry ?

func (this *ZookeeperCoordinator) tryRemoveOldApiRequests(group string, api ConsumerGroupApi) error {
    requests := make([]string, 0)
    var err error

    apiPath := fmt.Sprintf("%s/%s", newZKGroupDirs(this.config.Root, group).ConsumerApiDir, api)
    for i := 0; i <= this.config.MaxRequestRetries; i++ {
        requests, _, err = this.zkConn.Children(apiPath)
        if err != nil {
            continue
        }
        for _, request := range requests {
            var data []byte
            var t int64
            childPath := fmt.Sprintf("%s/%s", apiPath, request)
            if api == Rebalance {
                if data, _, err = this.zkConn.Get(childPath); err != nil && err == zk.ErrNoNode {
                    // It's possible another consumer deleted the node before we could read it's data
                    continue
                }
                if t, err = strconv.ParseInt(string(data), 10, 64); err != nil {
                    t = int64(0) // If the data isn't a timestamp ensure it will be deleted anyway.
                }
            } else if api == BlueGreenDeploymentAPI {
                if t, err = strconv.ParseInt(string(request), 10, 64); err != nil {
                    break
                }
            }

            // Delete if this zk node has an expired timestamp
            if time.Unix(t, 0).Before(time.Now().Add(-10 * time.Minute)) {
                // If the data is not a timestamp or is a timestamp but has reached expiration delete it
                err = this.deleteNode(childPath)
                if err != nil && err != zk.ErrNoNode {
                    break
                }
            }
        }
    }

    return err
}
@trueeyu trueeyu changed the title why not break when handle all request?(tryRemoveOldApiRequests) BUG?: why not break when handle all request?(tryRemoveOldApiRequests) Nov 27, 2015
@trueeyu trueeyu changed the title BUG?: why not break when handle all request?(tryRemoveOldApiRequests) BUG?: why not break after handle all request?(tryRemoveOldApiRequests) Nov 27, 2015
@trueeyu
Copy link
Author

trueeyu commented Nov 27, 2015

@serejja

@serejja
Copy link
Contributor

serejja commented Nov 27, 2015

Hey @trueeyu sorry for taking so long to get back to you, I'm quite loaded on other projects right now, I'll try to find some time to dig into this issue later today or tomorrow. Thanks

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