Skip to content

Fix socket thrashing on disconnect#64

Merged
matburt merged 1 commit intoproject-receptor:masterfrom
matburt:fix_socket_thrashing
Dec 9, 2019
Merged

Fix socket thrashing on disconnect#64
matburt merged 1 commit intoproject-receptor:masterfrom
matburt:fix_socket_thrashing

Conversation

@matburt
Copy link
Copy Markdown
Member

@matburt matburt commented Dec 9, 2019

The underlying transport doesn't seem to realize the connection has
gone away underneath it. This detects the scenario and closes the connection.

async def close(self):
self._closed = True
await self.writer.close()
self.writer.close()
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The writer doesn't support awaiting on close()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread receptor/receptor.py
data = await buf.get()
except Exception:
logger.exception("message_handler")
break
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even with the break this always seems to get concurrent.futures._base.CancelledError

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ERROR 2019-12-09 14:09:21,866 controller receptor message_handler
Traceback (most recent call last):
  File "/c/Users/bsdma/Desktop/ansible/receptor/receptor/receptor.py", line 91, in message_handler
    data = await buf.get()
  File "/c/Users/bsdma/Desktop/ansible/receptor/receptor/messages/envelope.py", line 157, in get
    return await self.q.get()
  File "/usr/lib/python3.6/asyncio/queues.py", line 167, in get
    yield from getter
concurrent.futures._base.CancelledError

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it also seems to be delayed by a few seconds... I'm guessing we're still awaiting further down?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we get a cancellation from the previous await, we should probably just return. Are you saying that the except Exception fails to capture the cancellation?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what it looks like


async def __anext__(self):
bytes_ = await self.reader.read(self.chunk_size)
if not bytes_:
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this bit is actually needed but the empty byte seems to fall through if it's not caught at some other point.

The underlying transport doesn't seem to realize the connection has
gone away underneath it. This detects the scenario and closes the connection.
@matburt matburt force-pushed the fix_socket_thrashing branch from 8bfde2d to d7b9196 Compare December 9, 2019 19:10
Comment thread receptor/buffers/file.py
while self.q.qsize() > 0:
ident = await self.q.get()
data = await self._get_file(ident, handle_only=True, delete=False)
# TODO: This will never work, it's not pure json anymore
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I planned on fixing this in the next bit of work for the buffers.

Comment thread receptor/receptor.py
data = await buf.get()
except Exception:
logger.exception("message_handler")
break
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we get a cancellation from the previous await, we should probably just return. Are you saying that the except Exception fails to capture the cancellation?

@matburt matburt merged commit 98009be into project-receptor:master Dec 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants