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

Feat: remove loop reduction & filter unnecessary apprev update #5598

Merged
merged 3 commits into from
Mar 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion charts/vela-core/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ helm install --create-namespace -n vela-system kubevela kubevela/vela-core --wai
| Name | Description | Value |
| ------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------- |
| `optimize.cachedGvks` | Optimize types of resources to be cached. | `""` |
| `optimize.controllerReconcileLoopReduction` | Optimize ApplicationController reconcile by reducing the number of loops to reconcile application. | `false` |
| `optimize.markWithProb` | Optimize ResourceTracker GC by only run mark with probability. Side effect: outdated ResourceTracker might not be able to be removed immediately. | `0.1` |
| `optimize.disableComponentRevision` | Optimize componentRevision by disabling the creation and gc | `true` |
| `optimize.disableApplicationRevision` | Optimize ApplicationRevision by disabling the creation and gc. | `false` |
Expand Down
3 changes: 0 additions & 3 deletions charts/vela-core/templates/kubevela-controller.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -247,9 +247,6 @@ spec:
{{ if ne .Values.optimize.cachedGvks "" }}
- "--optimize-cached-gvks={{ .Values.optimize.cachedGvks }}"
{{ end }}
{{ if .Values.optimize.controllerReconcileLoopReduction }}
- "--optimize-controller-reconcile-loop-reduction"
{{ end }}
{{ if .Values.optimize.markWithProb }}
- "--optimize-mark-with-prob={{ .Values.optimize.markWithProb }}"
{{ end }}
Expand Down
2 changes: 0 additions & 2 deletions charts/vela-core/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ healthCheck:

## @section KubeVela controller optimization parameters
##@param optimize.cachedGvks Optimize types of resources to be cached.
##@param optimize.controllerReconcileLoopReduction Optimize ApplicationController reconcile by reducing the number of loops to reconcile application.
##@param optimize.markWithProb Optimize ResourceTracker GC by only run mark with probability. Side effect: outdated ResourceTracker might not be able to be removed immediately.
##@param optimize.disableComponentRevision Optimize componentRevision by disabling the creation and gc
##@param optimize.disableApplicationRevision Optimize ApplicationRevision by disabling the creation and gc.
Expand All @@ -95,7 +94,6 @@ healthCheck:
##@param optimize.enableResourceTrackerDeleteOnlyTrigger Optimize resourcetracker by only trigger reconcile when resourcetracker is deleted.
optimize:
cachedGvks: ""
controllerReconcileLoopReduction: false
markWithProb: 0.1
disableComponentRevision: true
disableApplicationRevision: false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,6 @@ const (
)

var (
// EnableReconcileLoopReduction optimize application reconcile loop by fusing phase transition
EnableReconcileLoopReduction = false
// EnableResourceTrackerDeleteOnlyTrigger optimize ResourceTracker mutate event trigger by only receiving deleting events
EnableResourceTrackerDeleteOnlyTrigger = true
)
Expand Down Expand Up @@ -250,11 +248,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
if workflowInstance.Status.EndTime.IsZero() {
r.doWorkflowFinish(logCtx, app, handler, workflowState)
}
if !EnableReconcileLoopReduction {
if result, err := r.gcResourceTrackers(logCtx, handler, common.ApplicationRunning, false, isUpdate); err != nil {
return result, err
}
}
case workflowv1alpha1.WorkflowStateSkipped:
return r.result(nil).requeue(executor.GetBackoffWaitTime()).ret()
default:
Expand Down Expand Up @@ -376,8 +369,7 @@ func (r *Reconciler) handleFinalizers(ctx monitorContext.Context, app *v1beta1.A
defer subCtx.Commit("finish add finalizers")
meta.AddFinalizer(app, oam.FinalizerResourceTracker)
subCtx.Info("Register new finalizer for application", "finalizer", oam.FinalizerResourceTracker)
endReconcile := !EnableReconcileLoopReduction
return r.result(errors.Wrap(r.Client.Update(ctx, app), errUpdateApplicationFinalizer)).end(endReconcile)
return r.result(errors.Wrap(r.Client.Update(ctx, app), errUpdateApplicationFinalizer)).end(true)
}
} else {
if slices.Contains(app.GetFinalizers(), oam.FinalizerResourceTracker) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -856,6 +856,11 @@ func (h *AppHandler) FinalizeAndApplyAppRevision(ctx context.Context) error {
}
return err
}
if apiequality.Semantic.DeepEqual(gotAppRev.Spec, appRev.Spec) &&
apiequality.Semantic.DeepEqual(gotAppRev.GetLabels(), appRev.GetLabels()) &&
apiequality.Semantic.DeepEqual(gotAppRev.GetAnnotations(), appRev.GetAnnotations()) {
return nil
}
appRev.ResourceVersion = gotAppRev.ResourceVersion

// Set compression types (if enabled)
Expand Down
3 changes: 0 additions & 3 deletions pkg/controller/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,6 @@ func AddOptimizeFlags(fs *pflag.FlagSet) {
fs.StringVar(&velaclient.CachedGVKs, "optimize-cached-gvks", "", "Types of resources to be cached. For example, --optimize-cached-gvks=Deployment.v1.apps,Job.v1.batch . If you have dedicated resources to be managed in your system, you can turn it on to improve performance. NOTE: this optimization only works for single-cluster.")
fs.BoolVar(&cache.OptimizeListOp, "optimize-list-op", true, "Optimize ResourceTracker & ApplicationRevision list op by adding index. This will increase the use of memory and accelerate the list operation of ResourceTracker. Default to enable it. If you want to reduce the memory use of KubeVela, you can switch it off.")

// optimize controller reconcile loop
fs.BoolVar(&application.EnableReconcileLoopReduction, "optimize-controller-reconcile-loop-reduction", false, "Optimize ApplicationController reconcile by reducing the number of loops to reconcile application. In detail, reconciles after finalizer patching and workflow finished will not return immediately but will continue running. If you do not care about the occasional re-run of workflow, you can switch it on to further improve KubeVela controller performance.")

// optimize functions
fs.Float64Var(&resourcekeeper.MarkWithProbability, "optimize-mark-with-prob", 0.1, "Optimize ResourceTracker GC by only run mark with probability. Side effect: outdated ResourceTracker might not be able to be removed immediately. Default to 0.1. If you want to cleanup outdated resource for keepLegacyResource mode immediately, set it to 1.0 to disable this optimization.")
fs.BoolVar(&application.DisableAllComponentRevision, "optimize-disable-component-revision", false, "Optimize ComponentRevision by disabling the creation and gc. Side effect: rollout cannot be used. If you don't use rollout trait, you can switch it on to reduce the storage and improve performance.")
Expand Down
14 changes: 9 additions & 5 deletions test/e2e-multicluster-test/multicluster_standalone_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,10 +280,14 @@ var _ = Describe("Test multicluster standalone scenario", func() {
))

By("Rollback application")
_, err = execCommand("workflow", "suspend", "busybox", "-n", namespace)
Expect(err).Should(Succeed())
_, err = execCommand("workflow", "rollback", "busybox", "-n", namespace)
Expect(err).Should(Succeed())
Eventually(func(g Gomega) {
_, err = execCommand("workflow", "suspend", "busybox", "-n", namespace)
g.Expect(err).Should(Succeed())
}).WithTimeout(10 * time.Second).Should(Succeed())
Eventually(func(g Gomega) {
_, err = execCommand("workflow", "rollback", "busybox", "-n", namespace)
g.Expect(err).Should(Succeed())
}).WithTimeout(10 * time.Second).Should(Succeed())

Eventually(func(g Gomega) {
g.Expect(k8sClient.Get(hubCtx, appKey, app)).Should(Succeed())
Expand All @@ -296,7 +300,7 @@ var _ = Describe("Test multicluster standalone scenario", func() {
revs, err := application.GetSortedAppRevisions(hubCtx, k8sClient, app.Name, namespace)
g.Expect(err).Should(Succeed())
g.Expect(len(revs)).Should(Equal(1))
})
}).WithTimeout(10 * time.Second).Should(Succeed())
})

It("Test large application parallel apply and delete", func() {
Expand Down