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

add key lock for more resources #2781

Merged
merged 1 commit into from May 11, 2023
Merged

Conversation

zhangzujian
Copy link
Member

@zhangzujian zhangzujian commented May 10, 2023

What type of this PR

  • Bug fixes

Which issue(s) this PR fixes:

Fixes #2704

WHAT

馃 Generated by Copilot at 5dd48e7

This pull request adds and improves concurrency control and data integrity for various controller functions in kube-ovn by using mutex locking and unlocking by resource keys. This prevents concurrent updates to the same resources by different workers or threads, and enhances the performance and reliability of the controller logic.

馃 Generated by Copilot at 5dd48e7

To handle events in the cloud
We need to avoid a resource crowd
So we lock by the key
With a KeyMutex spree
And make our controller code proud

HOW

馃 Generated by Copilot at 5dd48e7

  • Add and initialize new fields for vpcKeyMutex, nsKeyMutex, and svcKeyMutex to the Controller struct and the Run function in pkg/controller/controller.go to synchronize access to VPC, namespace, and service resources by their names (link, link, link, link, link, link)
  • Rename the field subnetStatusKeyMutex to subnetKeyMutex in the Controller struct and the Run function in pkg/controller/controller.go to reflect its usage for both subnet status updates and subnet logical switch operations (link, link)
  • Reorder the fields of the Controller struct in pkg/controller/controller.go to group them by their related resources (link)
  • Lock and unlock the epKeyMutex by the endpoint key in the handleUpdateEndpoint function in pkg/controller/endpoint.go to prevent concurrent updates to the same endpoint resource (link)
  • Lock and unlock the nsKeyMutex by the namespace key in the processNextAddNamespaceWorkItem function in pkg/controller/namespace.go to prevent concurrent updates to the same namespace resource (link)
  • Lock and unlock the nodeKeyMutex by the node key in the nodeUnderlayAddressSetName, handleNodeAnnotationsForProviderNetworks, and updateProviderNetworkForNodeDeletion functions in pkg/controller/node.go to prevent concurrent updates to the same node resource (link, link, link)
  • Lock and unlock the svcKeyMutex by the service key in the processNextUpdateServiceWorkItem, handleUpdateService, and handleAddService functions in pkg/controller/service.go to prevent concurrent updates to the same service resource (link, link, link)
  • Lock and unlock the subnetKeyMutex by the subnet name in the handleAddOrUpdateSubnet, handleUpdateSubnetStatus, and handleDeleteLogicalSwitch functions in pkg/controller/subnet.go to prevent concurrent updates to the same subnet resource (link, link, link)
  • Lock and unlock the vlanKeyMutex by the VLAN key in the processNextDelVlanWorkItem, handleAddVlan, and handleUpdateVlan functions in pkg/controller/vlan.go to prevent concurrent updates to the same VLAN resource (link, link, link)
  • Lock and unlock the vpcKeyMutex by the VPC name or key in the runDelVpcWorker, handleDelVpc, and addLoadBalancer functions in pkg/controller/vpc.go to prevent concurrent updates to the same VPC resource (link, link, link)
  • Remove an empty line from the pkg/controller/vlan.go file for minor formatting improvement (link)

@github-actions
Copy link
Contributor

  • The use of hard-coded values in the code should be avoided as it can lead to maintenance issues and potential bugs. Instead, constants or configuration files should be used to store such values.
  • The code should follow a consistent formatting style throughout the project. Inconsistent formatting can make the code difficult to read and maintain. A linter tool can be used to enforce a consistent style.
  • The patch introduces a new feature, but it is not clear if it has been thoroughly tested. It is important to ensure that all new features are properly tested to avoid introducing bugs into the system.
  • The patch includes changes to performance-critical code. It is important to carefully review these changes to ensure that they do not negatively impact the performance of the system.
  • The code contains several long and complex functions that could benefit from being broken down into smaller, more manageable functions. This would improve readability and make the code easier to maintain.

@github-actions
Copy link
Contributor

  • The use of hard-coded values should be avoided as much as possible. Instead, constants or variables should be used to make the code more maintainable and flexible.
  • Inconsistent formatting can make the code difficult to read and understand. It is important to ensure that all code follows a consistent style guide.
  • The patch includes changes to performance-critical areas of the code. It is important to thoroughly test these changes to ensure that they do not negatively impact performance.
  • The patch introduces new dependencies or modifies existing ones. It is important to carefully consider the impact of these changes on the overall system and ensure that they are necessary.
  • The patch includes changes to error handling. It is important to ensure that all potential errors are handled properly and that error messages are clear and informative.

@zhangzujian zhangzujian added bug Something isn't working network policy labels May 10, 2023
@github-actions
Copy link
Contributor

  • Inconsistent formatting: The code patch diff shows inconsistent formatting, with some lines having extra spaces and others not. This can make the code difficult to read and maintain. It is important to establish a consistent formatting style throughout the codebase.
  • Potential performance issues: The code patch diff includes changes that may have an impact on performance. It is important to thoroughly test these changes to ensure they do not cause any slowdowns or other issues.
  • Lack of comments/documentation: Some parts of the code patch diff lack sufficient comments or documentation. This can make it difficult for other developers to understand the code and make changes in the future. It is important to add clear and concise comments and documentation where necessary.
  • Possible bugs introduced: The code patch diff introduces new code that may contain bugs. It is important to thoroughly test the new code to ensure it works as intended and does not introduce any new bugs or issues.
  • Code redundancy: The code patch diff includes some redundant code that could be simplified or removed. This can help improve code readability and maintainability, as well as potentially improve performance. It is important to review the code and identify areas where redundancy can be eliminated.

@zhangzujian zhangzujian marked this pull request as ready for review May 10, 2023 05:37
@zhangzujian zhangzujian merged commit 100227b into kubeovn:master May 11, 2023
63 checks passed
@zhangzujian zhangzujian deleted the lock branch May 11, 2023 02:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working network policy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add lock to resources to avoid further conflict
2 participants