[WIP] Refactor legacy informer/handler framework #2372
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The legacy event driven informer/handler framework has flaws with
performance. The regular "informer" object is not multi-threaded and
handles events without a queue. The federated "queued informer" sends
events to queues where worker threads pull items off and process them.
In either informer version, the processing of objects is paused when a
new handler is added to the informer. When a new handler is added, all
existing objects in the cluster are processed as ADDs. This is extremely
slow with UDN and adding new handlers on the fly as UDNs are created.
Additionally, the previous model for queued informer did not actually
stop the shared informer from enqueuing objects directly. It only paused
the workers, which indirectly would eventually stop the shared informer
from enqueuing after the channel buffer (size 10) was full.
To address this, the following has been changed:
1. Classic informer type object is removed, all legacy informers now
use a queued informer. These queued informers only use 1 worker
thread.
2. The queued informer now only enqueues events on initial add. The
queue size has been adjusted to 10k to account for buffering initial
add objects. Can tweak this later.
3. Locking has been modified. The informer lock is now used to control
event flow into the queues. In order to enqueue events from the
informer cache, a read lock must be obtained. When a handler is
added the informer lock is temporarily taken, to stop the informer
cache so that initial objects can be enqueued. Additionally a new
handlerMutex is introduced to control adding/removing handlers on
an queued informer object.
4. Since now all objects are enqueued when a handler is added and all
handlers will iterate over these already processed objects, a map
has been added to the handler. This processedInitialAdd map allows
the handler to determine if it has already processed the object or
not. In the case of existing handlers, they will ignore adds for
objects they already procesed. For newly added handlers, they will
ignore updates and deletes received before they have first added
the object.
Signed-off-by: Tim Rozet <trozet@redhat.com>
(cherry picked from commit 27998123011b1eb6cb056e9a307a62c8eea20026)
trozet
changed the title
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
/test qe-perfscale-aws-ovn-small-udn-density-l3 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: trozet 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 |
openshift-ci
bot
added
the
approved
|
/test qe-perfscale-aws-ovn-small-udn-density-l3 |
|
@trozet: The following tests failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The legacy event driven informer/handler framework has flaws with performance. The regular "informer" object is not multi-threaded and handles events without a queue. The federated "queued informer" sends events to queues where worker threads pull items off and process them. In either informer version, the processing of objects is paused when a new handler is added to the informer. When a new handler is added, all existing objects in the cluster are processed as ADDs. This is extremely slow with UDN and adding new handlers on the fly as UDNs are created. Additionally, the previous model for queued informer did not actually stop the shared informer from enqueuing objects directly. It only paused the workers, which indirectly would eventually stop the shared informer from enqueuing after the channel buffer (size 10) was full.
To address this, the following has been changed:
Signed-off-by: Tim Rozet trozet@redhat.com
(cherry picked from commit 27998123011b1eb6cb056e9a307a62c8eea20026)
📑 Description
Fixes #
Additional Information for reviewers
✅ Checks
How to verify it