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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix bind changed not handled to 1.1 #14243

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

zhangxu19830126
Copy link
Contributor

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

issue #14241

What this PR does / why we need it:

fix bind changed not handled

@matrix-meow matrix-meow added the size/S Denotes a PR that changes [10,99] lines label Jan 17, 2024
@mergify mergify bot added the kind/bug Something isn't working label Jan 17, 2024
@matrix-meow
Copy link
Contributor

@zhangxu19830126 Thanks for your contributions!

Review:

Title: fix bind changed not handled to 1.1

Body: The pull request is a bug fix for issue #14241. It fixes an issue where bind changes were not being handled correctly.

Changes:

  1. In pkg/lockservice/service_remote.go, the code has been modified to handle stale bind changes. The old bind lock table is now removed when a stale bind is found. This is done by calling the removeWithFilter method on the s.tables object.
  2. In pkg/lockservice/service_remote_test.go, an assertion has been added to check if the lock table is nil after a bind change.
  3. In pkg/pb/lock/lock.go, a new method Equal has been added to the LockTable struct. This method checks if two LockTable objects have the same table and version.

Overall, the changes in this pull request address the issue of stale bind changes not being handled correctly. The code modifications seem to be correct and should fix the problem. However, there are a few suggestions for improvement:

  1. Security: It's important to ensure that the removal of the old bind lock table is done securely. This means verifying the identity of the requester before removing the table. Consider adding authentication or authorization checks to ensure that only authorized users can remove the table.

  2. Code Optimization: The code in pkg/lockservice/service_remote.go can be optimized by using a more efficient data structure for storing the lock tables. Currently, the code iterates over all lock tables to find the one that needs to be removed. This can be slow if there are a large number of lock tables. Consider using a data structure like a map or a hash table to improve the lookup time.

  3. Test Coverage: It would be beneficial to add more test cases to cover different scenarios and edge cases related to bind changes. This will help ensure that the code handles all possible scenarios correctly.

  4. Documentation: It would be helpful to add comments or documentation explaining the purpose and behavior of the Equal method in pkg/pb/lock/lock.go. This will make it easier for other developers to understand and use the code.

Addressing these suggestions will help improve the security, performance, and maintainability of the codebase.

@mergify mergify bot merged commit 44716eb into matrixorigin:1.1-dev Jan 17, 2024
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working size/S Denotes a PR that changes [10,99] lines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants