elodina / go_kafka_client

Apache Kafka Client Library for Go
http://www.elodina.net
Apache License 2.0
275 stars 74 forks source link

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

Open trueeyu opened 8 years ago

trueeyu commented 8 years ago

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 commented 8 years ago

@serejja

serejja commented 8 years ago

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