refactor(arbitrator): Simplify arbitration participant lock #11525
Conversation
facebook-github-bot
added
the
CLA Signed
✅ Deploy Preview for meta-velox canceled.
|
tanjialiang
force-pushed
the
simplify_timelock
branch
from
e849490 to
4aba9a7
Compare
There was a problem hiding this comment.
@tanjialiang LGTM and some comments to extend existing participant APIs. thanks!
| @@ -320,7 +320,7 @@ uint64_t ArbitrationParticipant::shrinkLocked(bool reclaimAll) { | |||
|
|
|||
| uint64_t ArbitrationParticipant::abort( | |||
| const std::exception_ptr& error) noexcept { | |||
| ArbitrationOperationTimedLock l(reclaimMutex_); | |||
| std::lock_guard<std::timed_mutex> l(reclaimMutex_); | |||
There was a problem hiding this comment.
Can we extend to take time for abort? Thanks!
There was a problem hiding this comment.
abort path is used by global arbitration. We currently don't have timeout information to pass for global arbitration because it does not belong to any requestor. The timeout protection is performed already when waiting for global arbitration.
tanjialiang
force-pushed
the
simplify_timelock
branch
from
4aba9a7 to
91e4f38
Compare
|
@tanjialiang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
tanjialiang
force-pushed
the
simplify_timelock
branch
from
91e4f38 to
05f1648
Compare
|
@tanjialiang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
1 similar comment
|
@tanjialiang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
tanjialiang
force-pushed
the
simplify_timelock
branch
from
05f1648 to
67764d4
Compare
|
@tanjialiang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
tanjialiang
force-pushed
the
simplify_timelock
branch
from
67764d4 to
3322005
Compare
|
@tanjialiang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
tanjialiang
changed the title
tanjialiang
force-pushed
the
simplify_timelock
branch
2 times, most recently
from
5b5ef46 to
2579426
Compare
|
@tanjialiang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
tanjialiang
force-pushed
the
simplify_timelock
branch
from
2579426 to
ce4ffd4
Compare
|
@tanjialiang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
tanjialiang
changed the title
tanjialiang
force-pushed
the
simplify_timelock
branch
from
ce4ffd4 to
0bfcb7f
Compare
tanjialiang
changed the title
|
@tanjialiang has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@tanjialiang merged this pull request in 3265e79. |
|
Conbench analyzed the 1 benchmark run on commit There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
…incubator#11525) Summary: Simplify the ArbitrationTimedLock and MemoryArbitrationContext. Do not carry ArbitrationOperation in thread_local context anymore. ArbitrationTimedLock only deals with timed lock and custom arbitration throwing logic. Pull Request resolved: facebookincubator#11525 Reviewed By: xiaoxmeng Differential Revision: D65899323 Pulled By: tanjialiang fbshipit-source-id: 3907603132f6d4b70a443359887466ec82a69a65
Simplify the ArbitrationTimedLock and MemoryArbitrationContext. Do not carry ArbitrationOperation in thread_local context anymore. ArbitrationTimedLock only deals with timed lock and custom arbitration throwing logic.