Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix event loop blocking and deadlocks in batching loop #1332

Open
tsmith023 opened this issue Oct 11, 2024 · 0 comments
Open

Fix event loop blocking and deadlocks in batching loop #1332

tsmith023 opened this issue Oct 11, 2024 · 0 comments
Labels
bug Something isn't working

Comments

@tsmith023
Copy link
Contributor

Due to use of threading.Lock.acquire() inside async def __send_batch, the event loop can be blocked from progressing. This becomes problematic when multiple threads call batch.add_object as lock contention can occur between the many locks involved in synchronising the batching algorithm

In certain scenarios this lock contention leads to a deadlock as the event loop becomes blocked by a call to self.__active_requests_lock.acquire() inside async def __send_batch that stops the event loop from progressing thereby causing self.__active_requests to never increase or decrease in value, since all active requests are stuck in a blocked event loop

The same may occur with the other locks so it would be best to migrate all threading.Locks to asyncio.Locks and to call asyncio.Lock.acquire() within the sidecar event loop from synchronous functions

Once this is dealt with, the batching loop is much faster in sending objects. However, this has unintended consequences for the heuristics in the dynamic batch size calculating background thread. The heuristics involved there will need to be refactored to account for the timing behaviour change between using threading.Lock and asyncio.Lock

Draft PR for reference: #1270

@tsmith023 tsmith023 added the bug Something isn't working label Oct 11, 2024
@dirkkul dirkkul closed this as completed Nov 1, 2024
@dirkkul dirkkul reopened this Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants