| Message ID | 20241105154238.3611453-1-jsbronder@cold-front.org |
|---|---|
| State | New |
| Headers | show |
| Series | asyncrpc: timeout attempting to create a TCP connection | expand |
On Tue, Nov 5, 2024 at 8:42 AM Justin Bronder via lists.openembedded.org <jsbronder=cold-front.org@lists.openembedded.org> wrote: > > Timeout when trying to connect to an upstream that may be filtering > connections (instead of waiting endlessly) and raise a ConnectionError > which will allow the caller to note that the client is not connected. > This, for instance, ensures that a firewalled BB_HASHSERVE_UPSTREAM will > trigger warnings. > > Signed-off-by: Justin Bronder <jsbronder@cold-front.org> > --- > lib/bb/asyncrpc/client.py | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/lib/bb/asyncrpc/client.py b/lib/bb/asyncrpc/client.py > index 9be49261c..92e140dfe 100644 > --- a/lib/bb/asyncrpc/client.py > +++ b/lib/bb/asyncrpc/client.py > @@ -68,7 +68,12 @@ class AsyncClient(object): > > async def connect_tcp(self, address, port): > async def connect_sock(): > - reader, writer = await asyncio.open_connection(address, port) > + try: > + reader, writer = await asyncio.wait_for( > + asyncio.open_connection(address, port), > + timeout=5) Use self.timeout instead of "5" so that users can configure this through the normal timeout methods. > + except asyncio.TimeoutError as e: > + raise ConnectionError(f"Timeout connecting to {address}:{port}") from e > return StreamConnection(reader, writer, self.timeout, self.max_chunk) > > self._connect_sock = connect_sock > -- > 2.46.1 > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#16778): https://lists.openembedded.org/g/bitbake-devel/message/16778 > Mute This Topic: https://lists.openembedded.org/mt/109407941/3616693 > Group Owner: bitbake-devel+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [JPEWhacker@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- >
On 05/11/24 15:06 -0700, Joshua Watt wrote: > On Tue, Nov 5, 2024 at 8:42 AM Justin Bronder via > lists.openembedded.org > <jsbronder=cold-front.org@lists.openembedded.org> wrote: > > > > Timeout when trying to connect to an upstream that may be filtering > > connections (instead of waiting endlessly) and raise a ConnectionError > > which will allow the caller to note that the client is not connected. > > This, for instance, ensures that a firewalled BB_HASHSERVE_UPSTREAM will > > trigger warnings. > > > > Signed-off-by: Justin Bronder <jsbronder@cold-front.org> > > --- > > lib/bb/asyncrpc/client.py | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/lib/bb/asyncrpc/client.py b/lib/bb/asyncrpc/client.py > > index 9be49261c..92e140dfe 100644 > > --- a/lib/bb/asyncrpc/client.py > > +++ b/lib/bb/asyncrpc/client.py > > @@ -68,7 +68,12 @@ class AsyncClient(object): > > > > async def connect_tcp(self, address, port): > > async def connect_sock(): > > - reader, writer = await asyncio.open_connection(address, port) > > + try: > > + reader, writer = await asyncio.wait_for( > > + asyncio.open_connection(address, port), > > + timeout=5) > > Use self.timeout instead of "5" so that users can configure this > through the normal timeout methods. I did that initially and you're correct to call me out on this. I should have added more context to my commit message. With a shorter timeout I get the warning 'BB_HASHSERVE_UPSTREAM is not valid, ...' from BBCooker catches the ConnectionError in handlePrServ. That's the behavior I believe is desirable. When I use self.timeout (or longer timeouts in general), bitbake exits with: Timeout while waiting for a reply from the bitbake server (60s at 17:27:09.872219) and bitbake-cookerdaemon.log has BrokenPipeError being raised from python multiprocessing. I started debugging this but then couldn't figure out how a user would actually configure the timeout. At least neither PRAsyncClient or the hashserv client look like they're setting it. So at that point I figured a static, shorter timeout for failing to connect would be reasonable. If that argument doesn't pass muster, I'm happy to continue trying to figure out why using self.timeout isn't getting the warning from BBCooker but I suspect it's due to competing timeouts in separate modules. > > > + except asyncio.TimeoutError as e: > > + raise ConnectionError(f"Timeout connecting to {address}:{port}") from e > > return StreamConnection(reader, writer, self.timeout, self.max_chunk) > > > > self._connect_sock = connect_sock > > -- > > 2.46.1 > > > > > > -=-=-=-=-=-=-=-=-=-=-=- > > Links: You receive all messages sent to this group. > > View/Reply Online (#16778): https://lists.openembedded.org/g/bitbake-devel/message/16778 > > Mute This Topic: https://lists.openembedded.org/mt/109407941/3616693 > > Group Owner: bitbake-devel+owner@lists.openembedded.org > > Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [JPEWhacker@gmail.com] > > -=-=-=-=-=-=-=-=-=-=-=- > >
diff --git a/lib/bb/asyncrpc/client.py b/lib/bb/asyncrpc/client.py index 9be49261c..92e140dfe 100644 --- a/lib/bb/asyncrpc/client.py +++ b/lib/bb/asyncrpc/client.py @@ -68,7 +68,12 @@ class AsyncClient(object): async def connect_tcp(self, address, port): async def connect_sock(): - reader, writer = await asyncio.open_connection(address, port) + try: + reader, writer = await asyncio.wait_for( + asyncio.open_connection(address, port), + timeout=5) + except asyncio.TimeoutError as e: + raise ConnectionError(f"Timeout connecting to {address}:{port}") from e return StreamConnection(reader, writer, self.timeout, self.max_chunk) self._connect_sock = connect_sock
Timeout when trying to connect to an upstream that may be filtering connections (instead of waiting endlessly) and raise a ConnectionError which will allow the caller to note that the client is not connected. This, for instance, ensures that a firewalled BB_HASHSERVE_UPSTREAM will trigger warnings. Signed-off-by: Justin Bronder <jsbronder@cold-front.org> --- lib/bb/asyncrpc/client.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)