Skip to content

Set a timeout on send in connection #1246

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.

Sign up for GitHub

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

Jump to bottom
Closed
tvoinarovskyi opened this issue Oct 8, 2017 · 2 comments
Closed

Set a timeout on send in connection #1246

tvoinarovskyi opened this issue Oct 8, 2017 · 2 comments
Labels
network protocol

Comments

@tvoinarovskyi
Copy link
Collaborator

Spotted while reviewing #1230. The code for sending bytes into the network currently looks like this:

            # In the future we might manage an internal write buffer
            # and send bytes asynchronously. For now, just block
            # sending each request payload
            self._sock.setblocking(True)
            total_sent = 0
            while total_sent < len(data):
                sent_bytes = self._sock.send(data[total_sent:])
                total_sent += sent_bytes
            assert total_sent == len(data)
            if self._sensors:
                self._sensors.bytes_sent.record(total_sent)
            self._sock.setblocking(False)

The problem here is, that if 1 of the nodes goes down only on write (will stop reading bytes on the other end) this will lead to a stop in application code, as the sending Thread will suspend indefinitely. This is not too uncommon in Kubernetics or other virtual environments, that do not perfectly handle socket proxying.
To avoid the issue we have to make the call asynchronous or at least set the timeout based on request_timeout_ms configuration. For example:

            # In the future we might manage an internal write buffer
            # and send bytes asynchronously. For now, just block
            # sending each request payload
            self._sock.settimeout(self.config['request_timeout_ms'] / 1000.0)
            total_sent = 0
            while total_sent < len(data):
                sent_bytes = self._sock.send(data[total_sent:])
                total_sent += sent_bytes
            assert total_sent == len(data)
            if self._sensors:
                self._sensors.bytes_sent.record(total_sent)
            self._sock.setblocking(False)
@tvoinarovskyi
Copy link
Collaborator Author

@dpkp Am I missing something here? We can set timeout every time we use setblocking(False) actually

@dpkp
Copy link
Owner

dpkp commented Oct 8, 2017

Yes -- good catch. I filed #981 for work on non-blocking sends and have been hacking on that locally. Until then, I agree that we should set the socket timeout based on request timeout for blocking sends.

@dpkp dpkp mentioned this issue Oct 25, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
network protocol
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@dpkp @tvoinarovskyi