Fix DVM/DVMP hotlooping, remove remote watch TouchAnnotation, fix requeues so they used values from r.Migrate() #1013
Conversation
| @@ -117,17 +118,20 @@ func (r *ReconcileDirectImageMigration) Reconcile(request reconcile.Request) (re | |||
| if errors.IsNotFound(err) { | |||
| // Object not found, return. Created objects are automatically garbage collected. | |||
| // For additional cleanup logic use finalizers. | |||
| return reconcile.Result{}, nil | |||
| return reconcile.Result{Requeue: false}, nil | |||
There was a problem hiding this comment.
I think it's more obvious what's happening if we explicitly use the requeue flag
pkg/controller/directimagemigration/directimagemigration_controller.go
Outdated
Show resolved
Hide resolved
| @@ -139,7 +143,7 @@ func (r *ReconcileDirectImageMigration) Reconcile(request reconcile.Request) (re | |||
| } | |||
|
|
|||
| if !imageMigration.Status.HasBlockerCondition() { | |||
| _, err = r.migrate(imageMigration) | |||
| requeueAfter, err = r.migrate(imageMigration) | |||
There was a problem hiding this comment.
No longer throwing away the requeueAfter, so now if a requeue duration is selected in phase logic it actually gets used.
djwhatle
changed the title
| @@ -171,7 +171,7 @@ func (r *ReconcileDirectVolumeMigrationProgress) Reconcile(request reconcile.Req | |||
| } | |||
|
|
|||
| if !pvProgress.Status.IsReady() { | |||
| return reconcile.Result{Requeue: true}, nil | |||
| return reconcile.Result{Requeue: true, RequeueAfter: time.Second * 5}, nil | |||
There was a problem hiding this comment.
I wonder if this if block is necessary? looks like the intent is to requeue at 5 seconds irrespective of the status
There was a problem hiding this comment.
Yeah maybe not, let me take a look and see if it's safe to remove
|
I verified there is no observable perf change from this PR, which is what I expected. The goal of this PR is to reduce number of API requests issued when we're waiting on a long-running task without reducing our migration performance. Migration time for rocket-chat (this PR):2 minutes, 55 seconds Migration time for rocket-chat (master)2 minutes, 56 seconds |
| return reconcile.Result{RequeueAfter: requeueAfter}, nil | ||
| } | ||
|
|
||
| return reconcile.Result{Requeue: false}, nil |
There was a problem hiding this comment.
In all the other controllers we have return reconcile.Result{}, nil. Wonder if we should drop this OR update the others. I am fine with either as long as we are consistent.
There was a problem hiding this comment.
Good point, will update.
djwhatle
force-pushed
the
fix-direct-predicates-requeues
branch
2 times, most recently
from
19f104f
to
a9b0e9a
Compare
…ueue times Remove predicate Remove leftover log Move default PollReQ setting for better locality Remove redundant requeue Back out explicit requeue
djwhatle
force-pushed
the
fix-direct-predicates-requeues
branch
from
a9b0e9a
to
719eb24
Compare
Closes #1011