-
Notifications
You must be signed in to change notification settings - Fork 39
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
provide option to set MaxConcurrentReconciles #203
Conversation
Current MaxConcurrentReconciles is 1, which is the default, meaning only one request will be processed at a time. This is not useful when we have 100/1000 CR to reconcile, which can affect the performance. This PR provides a way to set the MaxConcurrentReconciles to the user's desired value so that the user can tune the configuration based on the requirement. Note:- Currently we have performance issue in DR VolRep because of this one, the failover and failback time are slightly affected because of the default value. with this, things will be better and same change need to be done when we add VolRep Reconciler also. Signed-off-by: Madhu Rajanna <[email protected]>
@nixpanic added doc for the command line arguments PTAL |
a14f248
to
7e1c664
Compare
document available command line agruments avaiable for configuration when starting the controller. Signed-off-by: Madhu Rajanna <[email protected]>
7e1c664
to
9be5a13
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes seem fine to me,
Ack, I'll make similar changes to vol-rep controller which will have pr up within next week,
just left a question regarding the default value.
flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") | ||
flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") | ||
flag.BoolVar(&enableLeaderElection, "leader-elect", false, | ||
"Enable leader election for controller manager. "+ | ||
"Enabling this will ensure there is only one active controller manager.") | ||
flag.DurationVar(&reclaimSpaceTimeout, "reclaim-space-timeout", defaultTimeout, "Timeout for reclaimspace operation") | ||
flag.IntVar(&maxConcurrentReconciles, "max-concurrent-reconciles", 100, "Maximum number of concurrent reconciles") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
flag.IntVar(&maxConcurrentReconciles, "max-concurrent-reconciles", 100, "Maximum number of concurrent reconciles") | |
flag.IntVar(&maxConcurrentReconciles, "max-concurrent-reconciles", 100, "Maximum number of concurrent reconciles") |
Do we really need 100
as default value ?
most controllers have just a single loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, having 100 in default doesn't cause any harm, and also, it doesn't make sense to have different configurations per controller. This helps to have concurrent reconciles, which fasten the process when we have more requests.
Added option to set MaxConcurrentReconciles for VolumeReplication which was pending and for all other controllers this is set in csi-addons#203 Signed-off-by: Madhu Rajanna <[email protected]>
Added option to set MaxConcurrentReconciles for VolumeReplication which was pending and for all other controllers this is set in csi-addons#203 Signed-off-by: Madhu Rajanna <[email protected]>
Added option to set MaxConcurrentReconciles for VolumeReplication which was pending and for all other controllers this is set in csi-addons#203 Signed-off-by: Madhu Rajanna <[email protected]>
Added option to set MaxConcurrentReconciles for VolumeReplication which was pending and for all other controllers this is set in csi-addons#203 Signed-off-by: Madhu Rajanna <[email protected]>
Added option to set MaxConcurrentReconciles for VolumeReplication which was pending and for all other controllers this is set in #203 Signed-off-by: Madhu Rajanna <[email protected]>
Added option to set MaxConcurrentReconciles for VolumeReplication which was pending and for all other controllers this is set in red-hat-storage#203 Signed-off-by: Madhu Rajanna <[email protected]>
Syncing latest changes from upstream main for kubernetes-csi-addons
Current MaxConcurrentReconciles is 1, which is the default, meaning only one request will be processed at a time. This is not useful when we have 100/1000 CR to reconcile, which can affect the performance. This PR provides a way to set the MaxConcurrentReconciles to the user's desired value so that the user can tune the configuration based on the requirement.
Note:- Currently we have performance issue in DR VolRep because of this one, the failover and failback time are slightly affected because of the default value. with this, things will be better and same change need to be done when we add VolRep Reconciler also.
Signed-off-by: Madhu Rajanna [email protected]