Change resource sync to use PATCH instead of PUTs #50
Conversation
patrick-east
force-pushed
the
patch-on-sync
branch
from
48e88d9 to
9e9c3fd
Compare
|
/cc @tsandall This handles part of what was talked about in #49 for batching together the sync requests. This does it for each generic sync (ie each resource type being sync'd). If we wanted to do a single patch for all of them I'll need to do some additional refactoring. Something like that might work as a solution to batch updates too, thinking a like generic buffering step in front of the OPA API's to batch requests. |
There was a problem hiding this comment.
Small comment about factoring the patch set generation into a helper and testing that separately. Also, would be good test both namespaced and non-namespaced resources, e.g., Pods and Nodes (respectively).
Did you have a chance to benchmark the change? Would be good to have a rough idea of hte improvement this yields.
patrick-east
changed the title
|
Updates still incoming. I was messing around with a test cluster and see some pretty decent preliminary results as far as performance improvements: Current master: This branch: |
patrick-east
force-pushed
the
patch-on-sync
branch
from
9e9c3fd to
d1dfd00
Compare
patrick-east
changed the title
pkg/data/generic.go
Outdated
| // We don't do a 'remove' patch because doesn't exist we waste all the | ||
| // resources parsing the giant patch payload and have to try again.. Best | ||
| // option currently is to just do two requests. | ||
| err = s.opa.PutData("/", map[string]interface{}{}) |
There was a problem hiding this comment.
I was thinking about this a bit more and now I'm wondering why we can't just set the entire collection at once? If we do a PUT on /v1/data/kubernetes/ it'll overwrite everything in the collection. This may even be more efficient than the PATCH method since the server and store don't have to process a bunch of separate patch operations. WDYT?
There was a problem hiding this comment.
I was also wondering about that. I shied-away from it initially because I was concerned about having to limit the size of the payloads and patches seemed like the most direct way to chunk them up. That being said.. there isn't really any difference between that and just building PUT payloads of different sizes. Let me update quickly to try it out and see if there is much of a difference in performance.
On paper I think you're right though that a single large PUT should be less load on the server as it is today.
When sync'ing resources kube-mgmt would do a delete and then a PUT request for each object. This will aggregate all of the objects into one PUT request. Signed-off-by: Patrick East <[email protected]>
patrick-east
force-pushed
the
patch-on-sync
branch
from
d1dfd00 to
26b1196
Compare
|
Updated to single PUT, speeds appear to be pretty similar to PATCH but as seen in the unit tests the payload is considerably smaller so we're saving some resources on processing JSON on both ends. |
When sync'ing resources kube-mgmt would do a delete and then a PUT
request for each object. This will aggregate all of the objects into
one PUT request.