Code review comment for lp:~marcustomlinson/unity-scopes-api/disable_timeouts_in_debug_mode

Revision history for this message
Marcus Tomlinson (marcustomlinson) wrote :

> 102 - auto future = mw_base()->twoway_pool()->submit([&] { return
> this->invoke_twoway_(request_builder, -1, 30000); });
> 103 + auto future = mw_base()->twoway_pool()->submit([&] { return
> this->invoke_twoway_(request_builder, -1, -1); });
>
> This change looks a bit suspicious, is this intended? In full context it is:
>
> if (!debug_mode_)
> {
> capnp::MallocMessageBuilder request_builder;
> make_request_(request_builder, "debug_mode");
>
> auto future = mw_base()->twoway_pool()->submit([&] { return
> this->invoke_twoway_(request_builder, -1, -1); });
>
> Which, if I understand it correctly, means we don't apply timeout when running
> normally (i.e. not in debug mode)?

So the timeout we're disabling here is the locate() call that happens implicitly when making any request. It goes:
    1. reg->locate(scope) - first ensure the scope is running
    2. scope->debug_mode() - make the actual request

The issue there is if a scope actually is in debug mode and it takes long to start up, that implicit locate() call will time out prematurely.

In the situation where a scope is not in debug mode but starts up slowly (E.g. has a breakpoint in start()), the registry's own "Process.Timeout" will kick in (which is configured according to the "DebugMode" state of each scope config as they are added). Meaning, that implicit locate() call will actually still timeout due to the registry's "Process.Timeout" constraint when a regular scope takes too long to start.

Make sense?

« Back to merge proposal