[SERVER-43442] Remove blocking call from AsyncWorkScheduler::_targetHostAsync() Created: 23/Sep/19 Updated: 29/Oct/23 Resolved: 09/Apr/20 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 4.7.0 |
| Type: | Task | Priority: | Major - P3 |
| Reporter: | PM Bot | Assignee: | Spencer Brody (Inactive) |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | autogen-todo, servicearch-wfbf-day | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||||||||||
| Sprint: | Service Arch 2019-10-21, Service Arch 2019-11-04, Service Arch 2019-11-18, Service Arch 2019-12-02, Service Arch 2019-12-16, Service Arch 2019-12-30, Service Arch 2020-01-13, Service Arch 2020-01-27, Service Arch 2020-02-10, Service Arch 2020-02-24, Service Arch 2020-03-09, Service Arch 2020-03-23, Service Arch 2020-04-06, Service arch 2020-04-20 | ||||||||||||||||||||
| Participants: | |||||||||||||||||||||
| Description |
|
See here in the transaction_coordinator_futures_util.cpp. |
| Comments |
| Comment by Githook User [ 30/Sep/20 ] |
|
Author: {'name': 'Spencer T Brody', 'email': 'spencer@mongodb.com', 'username': 'stbrody'}Message: This reverts commit 0651f3c7b63130842e85d44fec624b82a33b67a3. |
| Comment by Githook User [ 09/Apr/20 ] |
|
Author: {'name': 'Spencer T Brody', 'email': 'spencer@mongodb.com', 'username': 'stbrody'}Message: Also updates some places relying on implicit conversion from Status to Future |
| Comment by Benjamin Caimano (Inactive) [ 07/Apr/20 ] |
|
spencer, admittedly, I just did a skim that probably wouldn't qualify as fully understanding the code. That said, yeah, the blocking call to get() shouldn't be happening. There is a wrinkle though: findHostWithMaxWait() returns a SemiFuture because it uses a different underlying executor. Normally, we'd use an ExecutorFuture here to transform from HostAndPort to HostAndShard on the local executor. But we apparently have Futures as the AsyncWorkScheduler public API. We've wanted to make unsafeToInlineFuture public for cases like these. I think now is probably the time for this. |
| Comment by Spencer Brody (Inactive) [ 06/Apr/20 ] |
|
The title of this ticket suggests that the return type of AsyncWorkScheduler::_targetHostAsync should be changed from Future<AsyncWorkScheduler::HostAndShard> to SemiFuture<AsyncWorkScheduler::HostAndShard>. My understanding of the difference between Future and SemiFuture, however, is simply that Future allows you to schedule follow-up work on the same Executor, while SemiFuture requires you to specify what Executor you want to run any future work on. Since _targetHostAsync is private, has only 1 caller, and that 1 caller goes on to schedule further work on the same Executor, I feel like the return type doesn't actually need to change here. I think the real issue is that _targetHostAsync makes a blocking call to select which host within the shard should be targeted, rather than doing that async. I think that's the real issue that this ticket is about addressing. ben.caimano, do you agree? If so I'll update the ticket title and description accordingly. |