[perf] reduce sts updates during sts reconcilation by using equality check #1197
Conversation
| if err != nil { | ||
| if apiErrors.IsNotFound(err) { | ||
| return appsv1.StatefulSet{}, getUpdateCreator.CreateStatefulSet(sts) | ||
| } | ||
| return appsv1.StatefulSet{}, err | ||
| } | ||
| if equality.Semantic.DeepEqual(currSts, sts) { |
There was a problem hiding this comment.
During my local test runs this works fine. This method is preferred since it focuses on semantic equality instead of memory equality.
It might have been enough to check spec and some ownerRefs, but I wanted to be on the safer side.
Alternative:
- create hash of current sts
- save hash as annotation on sts
- check hash of annotation
-> safer but takes more implementation for something that might not be worth the effort
There was a problem hiding this comment.
This method is preferred since it focuses on semantic equality instead of memory equality.
I am not sure if I understand the difference here. Would you happen to have an example?
There was a problem hiding this comment.
Sure.
for example, it matches an empty slice with a nil slice and the same for a map.
In our code where we generate an internal view of a statefulset we create empty maps and slices while the api server returns them as nil (or the other way around). With above function equality is true due to semantic equality.
nammn
requested review from
irajdeep,
priyolahiri,
mircea-cosbuc,
lsierant and
slaskawi
as code owners
|
This PR is being marked stale because it has been open for 60 days with no activity. Please update the PR or ask for a fresh review. |
|
Closing as conflicted WIP work. Please reopen if needed. |
All Submissions:
closes #XXXXin your comment to auto-close the issue that your PR fixes (if such).Context
During our replicaset deployment we do the 2 following things in that order:
Then we wait and check at each reconciliation whether either of above steps are done.
At each reconciliation of [2] we wait and create an internal view how a sts has to look like. There is a possibility that the sts has changed during last reconcile. Because of this we send an update request to our api server to make sure our internal view is applied.
This patch makes sure we only send updates if the internal view differs from the current sts.
Note: Depending on how many member [2] has and how long the reconcile takes we can do a lot of reconciles and therefore a lot of update requests.