Adding batch handling for popping items from RealFIFO #132240
Conversation
|
This issue is currently awaiting triage. If a SIG or subproject determines this is a relevant issue, they will accept it by applying the The Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
6fc1443 to
981a9a1
Compare
981a9a1 to
38a5d0b
Compare
f5373ee to
d77407b
Compare
|
/cc |
9b68011 to
2e4faa5
Compare
|
/cc |
9fd34a1 to
83da6cd
Compare
83da6cd to
4e3cb8c
Compare
4e3cb8c to
6cff2f9
Compare
There was a problem hiding this comment.
This is good performance improvement and FMU it still preserves the ordering of events (we wont' run into https://github.com/kubernetes/kubernetes/pull/127631/files)
| if unique.Has(id) { | ||
| break | ||
| } | ||
| batchSize++ |
There was a problem hiding this comment.
This batchSize would be a useful metric.
There was a problem hiding this comment.
good idea, i think we can separate the metrics changes into another PR so we can focus down on the queue change in this PR
| @@ -165,6 +179,38 @@ type cache struct { | |||
|
|
|||
| var _ Store = &cache{} | |||
|
|
|||
| func (c *cache) Transaction(txns ...Transaction) error { | |||
There was a problem hiding this comment.
IMO it's worth to add some trace log for transaction operation here. e.g. log something if it takes more than 100ms.
There was a problem hiding this comment.
i think 100ms might be too tight for larger batch? added with 500ms
6480cc6 to
6b50997
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: yue9944882 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
15a5469 to
a29f120
Compare
a29f120 to
5872a66
Compare
Signed-off-by: Min Jin <[email protected]>
5872a66 to
20e1fc7
Compare
|
/retest |
|
/cc |
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
/kind feature
/sig api-machinery
Summary
Ref: #130767
The above issue noted a problem that the write lock acquisition from DeltaFIFO's
processDeltascan have much lower throughput due to read lock contention:This leads to an problem that the distribution between read lock and write lock are almost 1:1 in a large scale busy cluster.
This PR enables DeltaFIFO to process items in a batch fashion while preserving the in-queue orders, so it can batch multiple item writes (add/update/delete) within a single write lock acquisition.
Performance Test
By running the following unit test which simulates heavy watch event stream. It constructs a simple controller with an informer built from in-memory fake watch, then we use 100 concurrent routines to flood watch events to the watch channel: