Avoid unnecessary reclaim on non-reclaimable query #9737
Conversation
facebook-github-bot
added
the
CLA Signed
✅ Deploy Preview for meta-velox canceled.
|
|
@xiaoxmeng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
xiaoxmeng
force-pushed
the
reclaim
branch
2 times, most recently
from
779e919
to
2fecf99
Compare
|
@xiaoxmeng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
1 similar comment
|
@xiaoxmeng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
| @@ -181,9 +181,10 @@ void SharedArbitrator::getCandidateStats( | |||
| op->candidates.clear(); | |||
| op->candidates.reserve(op->candidatePools.size()); | |||
| for (const auto& pool : op->candidatePools) { | |||
| const bool selfCandidate = op->requestRoot == pool.get(); | |||
There was a problem hiding this comment.
s/selfCandidate/isSelfReclaim/ to have the consistent name across methods.
| int64_t SharedArbitrator::maxReclaimableCapacity(const MemoryPool& pool) const { | ||
| int64_t SharedArbitrator::maxReclaimableCapacity( | ||
| const MemoryPool& pool, | ||
| bool isSelfReclaim) const { | ||
| // Checks if a query memory pool has finished processing or not. If it has |
There was a problem hiding this comment.
Can we update the comments here as well?
There was a problem hiding this comment.
Thanks for the changes. One more thing maybe as a follow up to do. The arbitrator is growing more complicated with a bunch of special treatment. Do you think we shall document details on the arbitrator's mechanisms, modes, reservation pools etc. on top of SharedArbitrator class as an overview?
|
@xiaoxmeng merged this pull request in ad25e87. |
|
Conbench analyzed the 1 benchmark run on commit There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
…#9737) Summary: Avoid unnecessary reclaim on non-reclaimable query which might spent time on waiting for query task to pause. Also allows arbitration to reclaim reserved capacity from the request pool itself. Pull Request resolved: facebookincubator#9737 Reviewed By: tanjialiang Differential Revision: D57055391 Pulled By: xiaoxmeng fbshipit-source-id: 34fd61a343e445c2e5a4e228f3ce54d630c65f24
…#9737) Summary: Avoid unnecessary reclaim on non-reclaimable query which might spent time on waiting for query task to pause. Also allows arbitration to reclaim reserved capacity from the request pool itself. Pull Request resolved: facebookincubator#9737 Reviewed By: tanjialiang Differential Revision: D57055391 Pulled By: xiaoxmeng fbshipit-source-id: 34fd61a343e445c2e5a4e228f3ce54d630c65f24
Avoid unnecessary reclaim on non-reclaimable query which might spent time on waiting
for query task to pause.
Also allows arbitration to reclaim reserved capacity from the request pool itself.