Skip to content

Commit

Permalink
Auto merge of #2517 - micbou:remove-healthy-check, r=Valloric
Browse files Browse the repository at this point in the history
[READY] Remove healthy check

This PR removes the code that, if the server is not yet healthy, tries in a separate thread to send the same request at different intervals until it gets a response or reaches 5 failed attempts.

Main issue with that code is that it blocks Vim if a synchronous request (`semantic_completion_available`, `run_completer_command`, etc.)  is sent while the server is not up. See issue #2071.

Other issues are that it's hard to understand how this code behaves (it combines threads and retries), it becomes useless once the server is healthy (because `SERVER_HEALTHY` is cached and never reset), it depends on arbitrary parameters (why 5 retries with a delay of 0.5s multiplied by 1.5 at each attempt?), and it has a package dependency `retries`.

Now that we catch all server exceptions (PR #2453) and define a very short connection timeout (PR #2514), we can just drop this code.

This change gives a huge improvement in startup time on Windows (don't worry, more improvements are coming for all platforms). The reason is that `requests` (probably `urllib3`) tries for ~1s to check if the server is healthy on Windows while it's almost instantaneous on other platforms.
<table>
  <tr>
    <th rowspan="2">Platform</th>
    <th colspan="2">First run (ms)</th>
    <th colspan="2">Subsequent runs (ms)</th>
  </tr>
  <tr>
    <td>Before</td>
    <td>After</td>
    <td>Before</td>
    <td>After</td>
  </tr>
  <tr>
    <td>Ubuntu 16.04 64-bit</td>
    <td>228</td>
    <td>248</td>
    <td>176</td>
    <td>151</td>
  </tr>
  <tr>
    <td>macOS 10.12</td>
    <td>433</td>
    <td>417</td>
    <td>263</td>
    <td>263</td>
  </tr>
  <tr>
    <td>Windows 10 64-bit</td>
    <td>1861</td>
    <td>844</td>
    <td>814</td>
    <td>292</td>
  </tr>
</table>

These results were obtained by running the `prof.py` script from [this branch](https://github.com/micbou/YouCompleteMe/tree/profiling-startup). The difference between first run and subsequent runs is Python bytecode generation (`*.pyc` files).

Fixes #2071.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/valloric/youcompleteme/2517)
<!-- Reviewable:end -->
  • Loading branch information
homu committed Jan 28, 2017
2 parents 410a4b6 + 10aba99 commit f686734
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 142 deletions.
77 changes: 14 additions & 63 deletions python/ycm/client/base_request.py
Expand Up @@ -30,7 +30,6 @@
import json
from future.utils import native
from base64 import b64decode, b64encode
from retries import retries
from requests_futures.sessions import FuturesSession
from ycm.unsafe_thread_pool_executor import UnsafeThreadPoolExecutor
from ycm import vimsupport
Expand Down Expand Up @@ -102,43 +101,20 @@ def _TalkToHandlerAsync( data,
handler,
method,
timeout = _READ_TIMEOUT_SEC ):
def SendRequest( data, handler, method, timeout ):
request_uri = _BuildUri( handler )
if method == 'POST':
sent_data = _ToUtf8Json( data )
return BaseRequest.session.post(
request_uri,
data = sent_data,
headers = BaseRequest._ExtraHeaders( method,
request_uri,
sent_data ),
timeout = ( _CONNECT_TIMEOUT_SEC, timeout ) )
if method == 'GET':
return BaseRequest.session.get(
request_uri,
headers = BaseRequest._ExtraHeaders( method, request_uri ),
timeout = ( _CONNECT_TIMEOUT_SEC, timeout ) )

@retries( 5, delay = 0.5, backoff = 1.5 )
def DelayedSendRequest( data, handler, method ):
request_uri = _BuildUri( handler )
if method == 'POST':
sent_data = _ToUtf8Json( data )
return requests.post(
request_uri,
data = sent_data,
headers = BaseRequest._ExtraHeaders( method,
request_uri,
sent_data ) )
if method == 'GET':
return requests.get(
request_uri,
headers = BaseRequest._ExtraHeaders( method, request_uri ) )

if not _CheckServerIsHealthyWithCache():
return _EXECUTOR.submit( DelayedSendRequest, data, handler, method )

return SendRequest( data, handler, method, timeout )
request_uri = _BuildUri( handler )
if method == 'POST':
sent_data = _ToUtf8Json( data )
return BaseRequest.session.post(
request_uri,
data = sent_data,
headers = BaseRequest._ExtraHeaders( method,
request_uri,
sent_data ),
timeout = ( _CONNECT_TIMEOUT_SEC, timeout ) )
return BaseRequest.session.get(
request_uri,
headers = BaseRequest._ExtraHeaders( method, request_uri ),
timeout = ( _CONNECT_TIMEOUT_SEC, timeout ) )


@staticmethod
Expand Down Expand Up @@ -271,31 +247,6 @@ def _BuildUri( handler ):
handler ) ) )


SERVER_HEALTHY = False


def _CheckServerIsHealthyWithCache():
global SERVER_HEALTHY

def _ServerIsHealthy():
request_uri = _BuildUri( 'healthy' )
response = requests.get( request_uri,
headers = BaseRequest._ExtraHeaders(
'GET', request_uri, bytes( b'' ) ) )
_ValidateResponseObject( response )
response.raise_for_status()
return response.json()

if SERVER_HEALTHY:
return True

try:
SERVER_HEALTHY = _ServerIsHealthy()
return SERVER_HEALTHY
except:
return False


def MakeServerException( data ):
if data[ 'exception' ][ 'TYPE' ] == UnknownExtraConf.__name__:
return UnknownExtraConf( data[ 'exception' ][ 'extra_conf_file' ] )
Expand Down
79 changes: 0 additions & 79 deletions third_party/retries/retries.py

This file was deleted.

0 comments on commit f686734

Please sign in to comment.