fix(embeddings): bound the connection separately from the read

[bakiburakogun]

Fixes #345

Problem

_get_embedding() passed a single value as the timeout, so nothing bounded establishing the connection — only reading the response. When the TLS handshake to the embedding service stalls (TCP connects, the handshake never completes) the worker waits in recv for the whole request_timeout, which defaults to 1800 s.

It costs more than the one request: files_indexing_thread dispatches a batch across workers and then waits for all of them, so a single stalled handshake holds up the batch and no further queue items are fetched until it returns. In the logs that shows up as every worker of a batch finishing at the same moment, at the timeout, instead of at its own pace.

py-spy on a stalled worker put it inside connect(), in the handshake exchange:

sync_recv_gro (urllib3/contrib/ssa/_gro.py:205)
__exchange_until (urllib3/backend/hface.py:1018)
_post_conn (urllib3/backend/hface.py:712)
connect (urllib3/connection.py:894)
...
post (niquests/api.py:401)
_get_embedding (context_chat_backend/network_em.py:99)

Change

Add HTTP_CONNECT_TIMEOUT next to the existing TCP_CONNECT_TIMEOUT and pass (connect, read) to niquests.post. The read timeout is unchanged, so slow embedding responses are still tolerated exactly as before; only a connection that will not come up now fails quickly, and the retry already implemented in _get_embedding() can land on a healthy connection.

15 s is generous relative to what a working handshake costs — the same endpoint from the same container answers a one-text request in 0.28 s — while being far below the read timeout.

Testing

Deployed on Nextcloud 34.0.3, four backend instances, external embedding service. Same corpus, same nodes, nothing else changed:

	before	after
worker duration	~295–305 s, all finishing together	1.4–85 s
batch interval	~5 min	~1.5 min
indexing rate	5.2 documents/min	60–65 documents/min

Sustained over the following hours at 60–65 documents/min. Before the change the run looked stopped rather than slow, which is what sent us looking for a hang in the first place.

[bakiburakogun]

Please hold this one for a moment — see #345 (comment).

Running with this change I still caught workers stuck for 28 minutes in the HTTP/2 handshake exchange (_post_conn / __exchange_until), which sits after the socket connect and before the read, so neither timeout bounds it. The timeout here is still worth having, but on its own it does not fix what #345 describes and I did not want the PR to claim more than it does.

Forcing HTTP/1.1 for the embedding request removed the stall on our four instances (1 of 4 nodes working before, 4 of 4 after). Happy to extend this PR with that, or to make the protocol configurable instead — whichever shape you prefer.

[kyteinsky]

thanks!
replied to the addition discussion in the issue.

[kyteinsky]

this comment is not required, connect timeout is explanatory enough

[kyteinsky]

this works but would be nice to use a structure here: https://urllib3future.readthedocs.io/en/latest/reference/urllib3.util.html#urllib3.util.Timeout

[bakiburakogun]

Thanks, comment dropped and the timeout is a structure now.

One note on which structure. I started with urllib3.util.Timeout from the page you linked, but niquests dispatches on its own alias:

# niquests/adapters.py
elif isinstance(timeout, TimeoutSauce):
    ...
else:
    timeout = TimeoutSauce(connect=timeout, read=timeout)

and that check does not hold for the module under its urllib3_future name:

>>> niquests.TimeoutConfiguration is niquests.adapters.TimeoutSauce
True
>>> isinstance(urllib3_future.util.Timeout(connect=15, read=30), niquests.adapters.TimeoutSauce)
False

Passing the second one would fall through to the else branch and be treated as a scalar, which would be a quiet regression rather than an error. requirements.txt also pulls real urllib3 in through other packages, so which module import urllib3 resolves to is not something this file should have to depend on.

niquests.TimeoutConfiguration is the same class, is in niquests.__all__, and needs no import beyond the one already in the file, so that is what I used. Happy to switch if you would rather it read as urllib3.util.Timeout.