make resyncPeriod configurable #1013
Conversation
|
|
||
| "k8s.io/api/core/v1" | ||
| ) | ||
|
|
||
| const DefaultResyncPeriod = 12 * time.Hour |
There was a problem hiding this comment.
What should be the optimal default resync period? Currently, it is set to 30 sec
There was a problem hiding this comment.
In k8s, the default resync period is 12 hour. tf-operator is event driven to reach the desired state, so we do not really need the resync mechanism. If the resync period is short, it may reduce the processing speed of tf-operator, so we can make it longer like k8s.
There was a problem hiding this comment.
The resync is strongly suggested by jeremy. But I think we do not really need it. 12h LGTM. If there are some problems caused by the long period, it should be somewhere has bugs in the implementation.
There was a problem hiding this comment.
Agree. But since we are moving to v1, we need to test well to see if there are any hidden bugs.
|
/cc @richardsliu |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: richardsliu The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
For now, the default value for
resyncPeriodis 30s and it is too short and we do not rely on the resync of reflector to sync our tfjob, So make the resyncPeriod configurable and update the default value to 12 hourThis change is