gxthrj commented on a change in pull request #453:
URL: 
https://github.com/apache/apisix-ingress-controller/pull/453#discussion_r633098741



##########
File path: pkg/apisix/cluster.go
##########
@@ -132,13 +144,20 @@ func (c *cluster) syncCache() {
        }()
 
        backoff := wait.Backoff{
-               Duration: time.Second,
-               Factor:   2,
-               Steps:    6,
-       }
-       err := wait.ExponentialBackoff(backoff, c.syncCacheOnce)
+               Duration: 2 * time.Second,
+               Factor:   1,
+               Steps:    5,
+       }
+       var lastSyncErr error
+       err := wait.ExponentialBackoff(backoff, func() (done bool, _ error) {
+               // not possible return: false, nil

Review comment:
       ```suggestion
                // impossibly return: false, nil
   ```

##########
File path: pkg/ingress/controller.go
##########
@@ -423,3 +449,26 @@ func (c *Controller) syncSSL(ctx context.Context, ssl 
*apisixv1.Ssl, event types
        }
        return err
 }
+
+func (c *Controller) checkClusterHealthy(ctx context.Context) {
+       for {
+               select {
+               case <-ctx.Done():
+               case <-time.After(5 * time.Second):
+               }
+
+               // Retry three times in a row, and exit if all of them fail.
+               backoff := wait.Backoff{

Review comment:
       Put logic into the `case <-time.After(5 * time.Second)`

##########
File path: pkg/ingress/controller.go
##########
@@ -423,3 +455,21 @@ func (c *Controller) syncSSL(ctx context.Context, ssl 
*apisixv1.Ssl, event types
        }
        return err
 }
+
+func (c *Controller) checkClusterHealth(ctx context.Context, cancelFunc 
context.CancelFunc) {
+       for {
+               select {
+               case <-ctx.Done():
+               case <-time.After(5 * time.Second):
+               }
+
+               err := 
c.apisix.Cluster(c.cfg.APISIX.DefaultClusterName).HealthCheck(ctx)
+               if err != nil {
+                       // Finally failed health check, then give up leader.
+                       log.Warnf("failed to check health for default cluster: 
%s, give up leader", err)
+                       cancelFunc()

Review comment:
       ```suggestion
   defer cancelFunc()
   ```

##########
File path: pkg/ingress/controller.go
##########
@@ -423,3 +455,21 @@ func (c *Controller) syncSSL(ctx context.Context, ssl 
*apisixv1.Ssl, event types
        }
        return err
 }
+
+func (c *Controller) checkClusterHealth(ctx context.Context, cancelFunc 
context.CancelFunc) {
+       for {
+               select {
+               case <-ctx.Done():
+               case <-time.After(5 * time.Second):
+               }
+
+               err := 
c.apisix.Cluster(c.cfg.APISIX.DefaultClusterName).HealthCheck(ctx)
+               if err != nil {
+                       // Finally failed health check, then give up leader.
+                       log.Warnf("failed to check health for default cluster: 
%s, give up leader", err)
+                       cancelFunc()
+                       return

Review comment:
       Should we set the state to `_cacheSyncing` before return?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to