Skip to content

Remove mutex locks from interceptors on method calls#276

Merged
chimanjain merged 15 commits into
mainfrom
defect-resolve
Aug 30, 2024
Merged

Remove mutex locks from interceptors on method calls#276
chimanjain merged 15 commits into
mainfrom
defect-resolve

Conversation

@chimanjain

@chimanjain chimanjain commented Aug 23, 2024

Copy link
Copy Markdown
Contributor

Description

Improve performance by removing mutex calls from interceptors on method calls. Reducing unnecessary synchronization can decrease overhead and enhance system efficiency.

GitHub Issues

List the GitHub issues impacted by this PR:

GitHub Issue #
https://github.com/dell/csm/issues/1438

Checklist:

  • I have performed a self-review of my own code to ensure there are no formatting, vetting, linting, or security issues
  • I have verified that new and existing unit tests pass locally with my changes
  • I have not allowed coverage numbers to degenerate
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • Backward compatibility is not broken

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Please also list any relevant details for your test configuration

  • Run unit test
  • Sanity Testing
  • Successful image build
  • Successful testing of RWX, RWO, ROX Access modes on 3 node cluster and 11 node cluster.
    • I have done PV creation, pod mounting and pod deletion and PV deletion consecutively for the above configurations.
  • Successful installation via csm-operator and helm charts.
  • Tested quota feature

Screenshot 2024-08-23 171608
Screenshot 2024-08-23 170352

@prablr79

Copy link
Copy Markdown
Contributor

@chimanjain please ensure all Checklist verified and ensure all test results attached in the ticket for detail review of this PR. Thank you!

@nitesh3108

Copy link
Copy Markdown
Contributor

Have you considered a ROX volume scenario where same volume is mounted against multiple nodes.

@chimanjain

Copy link
Copy Markdown
Contributor Author

Have you considered a ROX volume scenario where same volume is mounted against multiple nodes.

Yes, it was tested against RWX, RWO, ROX access mode

@shefali-malhotra shefali-malhotra left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully you have verified setting of quota limit parameters in volume with these changes. If not, please verify and update the test results in PR.

@chimanjain

Copy link
Copy Markdown
Contributor Author

Verified the quota feature by creating the PVC with quota and pod mounting and then pod deletion and PVC deletion
Screenshot 2024-08-29 181516
Screenshot 2024-08-29 183449

@shefali-malhotra shefali-malhotra left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM . Considering sufficient regression testing has already been performed with these changes.

@chimanjain chimanjain merged commit 0290535 into main Aug 30, 2024
@chimanjain chimanjain deleted the defect-resolve branch August 30, 2024 03:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants