Fixes synchronicity between transactions/updates #254
Merged
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
|
/assign @dave-tucker |
|
testing in upstream ovnkube: ovn-org/ovn-kubernetes#2599 |
Pull Request Test Coverage Report for Build 1381829607
💛 - Coveralls |
dave-tucker
requested changes
Oct 22, 2021
|
@trozet there appear to still be panics in |
|
from that log, it's clear that we got an |
jcaamano
reviewed
Oct 25, 2021
jcaamano
reviewed
Oct 25, 2021
trozet
force-pushed
the
fix_handling_cache
branch
from
6db1815
to
33bbf40
Compare
The changes to use a buffered channel to handle updates broken the synchronization ovsdb depends on with txns. This is because OVSDB protocol specifies an update will be sent before a txn reply. We depend on this to ensure our cache is valid when we complete a txn. This patch reverts the changes from cda509c and while the monitor is being setup, defers updates to a buffer. After the monitor setup is complete, the defered updates will be processed in order. Afterwards, the db cacheMutex is unlocked so that subsequent updates maybe processed normally. Signed-off-by: Tim Rozet <trozet@redhat.com>
trozet
force-pushed
the
fix_handling_cache
branch
from
33bbf40
to
d11896c
Compare
jcaamano
reviewed
Oct 25, 2021
Previously failures to populate the cache would result in panic. This should be avoided where possible and error handling should take place to reconnect and rebuild the cache if the client and server become out of sync. This change fixes the remaining panics to return errors, and errors are sent to signal a reconnect and cache flush. Additionally, errors are propagated up to the RPC handler and error responses may be sent. Currently these responses may race with client disconnect, but at least its a step in the right direction. Signed-off-by: Tim Rozet <trozet@redhat.com>
dave-tucker
approved these changes
Oct 25, 2021
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 changes to use a buffered channel to handle updates broken the
synchronization ovsdb depends on with txns. This is because OVSDB
protocol specifies an update will be sent before a txn reply. We depend
on this to ensure our cache is valid when we complete a txn.
This patch reverts the changes from
cda509c and while the monitor is being
setup, defers updates to a buffer. After the monitor setup is complete,
the defered updates will be processed in order. Afterwards, the db
cacheMutex is unlocked so that subsequent updates maybe processed
normally.
Signed-off-by: Tim Rozet trozet@redhat.com