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

refactor(admin): prevent the large lock from mrmanager #554

Merged
merged 1 commit into from
Oct 4, 2023

Conversation

ijsong
Copy link
Member

@ijsong ijsong commented Jul 31, 2023

What this PR does

This PR changes the role of internal/admin/mrmanager.(*mrManager).mu. The large lock in the mrmanager made the mrmanager run sequentially. However, from now, it only guards the dirty flag for the cluster metadata. Since the metadata repository works concurrently, adopting a large lock on the client side is unnecessary.

@ijsong ijsong force-pushed the common_repfactor_from_uint_to_int branch from 6702ac2 to fd9e4d7 Compare July 31, 2023 12:45
@ijsong ijsong force-pushed the mr_no_large_lock branch from bd7df02 to 59cbb21 Compare July 31, 2023 12:45
@ijsong ijsong self-assigned this Jul 31, 2023
@ijsong ijsong force-pushed the common_repfactor_from_uint_to_int branch from fd9e4d7 to 6a9eb6f Compare September 11, 2023 11:16
@ijsong ijsong force-pushed the common_repfactor_from_uint_to_int branch from 6a9eb6f to 833c9aa Compare September 11, 2023 12:49
@codecov-commenter
Copy link

codecov-commenter commented Sep 11, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (2480922) 61.20% compared to head (efe2d44) 61.12%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #554      +/-   ##
==========================================
- Coverage   61.20%   61.12%   -0.09%     
==========================================
  Files         144      144              
  Lines       19241    19241              
==========================================
- Hits        11777    11761      -16     
- Misses       6876     6888      +12     
- Partials      588      592       +4     

see 7 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This PR changes the role of `internal/admin/mrmanager.(*mrManager).mu`. The large lock in the
mrmanager made the mrmanager run sequentially. However, from now, it only guards the dirty flag for
the cluster metadata. Since the metadata repository works concurrently, adopting a large lock on the
client side is unnecessary.
@ijsong
Copy link
Member Author

ijsong commented Oct 4, 2023

Merge Activity

  • Oct 4, 5:10 AM: @ijsong started a stack merge that includes this pull request via Graphite.
  • Oct 4, 5:10 AM: @ijsong merged this pull request with Graphite.

@ijsong ijsong merged commit 52257ff into main Oct 4, 2023
@ijsong ijsong deleted the mr_no_large_lock branch October 4, 2023 09:10
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.

2 participants