Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

libovsdb: use monitor_cond as the monitor method #2627

Merged
merged 1 commit into from
Apr 10, 2023

Conversation

zhangzujian
Copy link
Member

@zhangzujian zhangzujian commented Apr 9, 2023

What type of this PR

  • Bug fixes

Which issue(s) this PR fixes:

By default, libovsdb monitor uses monitor_cond_since as the monitor method. If a client is reconnecting a CondSince monitor that is the only monitor, it uses its LastTransactionID.
This patch is expected to fix #2621 by purging the cache on reconnection.

WHAT

馃 Generated by Copilot at 3a5eca6

This pull request improves the OVN NB client performance and code consistency by using conditional monitoring and sorting monitor options by table name in pkg/ovsdb/client/client.go and pkg/ovs/ovn-nb-suite_test.go.

馃 Generated by Copilot at 3a5eca6

To boost the OVN NB client
We use conditional monitor events
We sort by table name
In test and main
And add a missing import statement

HOW

馃 Generated by Copilot at 3a5eca6

  • Use conditional monitoring feature of libovsdb client to improve performance and scalability of OVN NB client (link, link)
  • Add import of github.com/ovn-org/libovsdb/ovsdb package in pkg/ovsdb/client/client.go (link)
  • Modify NewNbClient function in pkg/ovsdb/client/client.go to use ovsdb.ConditionalMonitorRPC constant and sort monitor options alphabetically by table name (link)
  • Sort monitor options alphabetically by table name in test file pkg/ovs/ovn-nb-suite_test.go for consistency and readability (link)
    • Update newNbClient function in pkg/ovs/ovn-nb-suite_test.go to match the order used in NewNbClient function in pkg/ovsdb/client/client.go (link)

@zhangzujian zhangzujian added bug Something isn't working need backport labels Apr 9, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Apr 9, 2023

  • The commit message is missing, it should provide a clear and concise description of the changes made in the patch.
  • In pkg/ovs/ovn-nb-suite_test.go, the order of tables in monitorOpts has been changed. It's not clear why this change was made and whether it could have any impact on the tests. This should be explained in the commit message or reviewed to ensure that it doesn't introduce any issues.
  • In pkg/ovsdb/client/client.go, the monitorOpts variable has been replaced with a monitor variable that is created using c.NewMonitor(). However, the monitorOpts slice is still used to specify the tables to monitor. This could be confusing and should be cleaned up.
  • The Method field of the monitor variable is set to ConditionalMonitorRPC. It's not clear why this value was chosen and whether it's appropriate for the use case. This should be explained in the commit message or reviewed to ensure that it doesn't introduce any issues.

@zhangzujian zhangzujian marked this pull request as ready for review April 9, 2023 05:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working need backport
Projects
None yet
Development

Successfully merging this pull request may close these issues.

libovsdb: cache inconsistent: row with uuid 9b596796-d378-4047-a47e-86ed44c311fe does not exist
2 participants