Skip to content

conn: create from socket fd #316

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

Merged
merged 3 commits into from
Nov 13, 2023

Conversation

askalt
Copy link
Contributor

@askalt askalt commented Oct 31, 2023

This patch adds the ability to create Tarantool connection using an existing socket fd.

I didn't forget about:

  • Tests
  • Changelog

Closes #304

@askalt askalt force-pushed the askalt/gh-304-socket-fd-connection branch 6 times, most recently from f033280 to df0eec0 Compare November 1, 2023 09:38
@askalt askalt marked this pull request as ready for review November 1, 2023 09:50
@askalt askalt marked this pull request as draft November 1, 2023 11:02
@askalt askalt force-pushed the askalt/gh-304-socket-fd-connection branch from df0eec0 to 5b9f1cc Compare November 1, 2023 16:50
@askalt askalt force-pushed the askalt/gh-304-socket-fd-connection branch 12 times, most recently from c77681b to 8aaece6 Compare November 2, 2023 13:49
@askalt askalt marked this pull request as ready for review November 2, 2023 17:06
@askalt
Copy link
Contributor Author

askalt commented Nov 2, 2023

There will be one more test (with Tarantool made sockets and child process), but in general it is already ready for the review.

@askalt askalt force-pushed the askalt/gh-304-socket-fd-connection branch 2 times, most recently from 7d6454e to 39815c1 Compare November 3, 2023 13:03
Copy link
Contributor

@oleg-jukovec oleg-jukovec left a comment

Choose a reason for hiding this comment

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

  1. select must have respect the timeout setting for non-blocking sockets.
  2. We need to think what we could to do if fd the socket is blocking and the timeout is set. The only way I see is to turn the socket into non-blocking mode.

@askalt askalt force-pushed the askalt/gh-304-socket-fd-connection branch from 39815c1 to 02d7bc2 Compare November 3, 2023 18:58
@askalt askalt force-pushed the askalt/gh-304-socket-fd-connection branch from 02d7bc2 to 5712143 Compare November 3, 2023 20:18
@askalt askalt force-pushed the askalt/gh-304-socket-fd-connection branch from 5712143 to 6b00156 Compare November 4, 2023 06:44
Copy link
Contributor

@oleg-jukovec oleg-jukovec left a comment

Choose a reason for hiding this comment

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

Thank you for the patch!

@askalt askalt force-pushed the askalt/gh-304-socket-fd-connection branch 5 times, most recently from d470f82 to 42e7f85 Compare November 8, 2023 07:02
@askalt askalt force-pushed the askalt/gh-304-socket-fd-connection branch 2 times, most recently from 195b3c9 to 543e17d Compare November 8, 2023 10:16
Copy link
Member

@DifferentialOrange DifferentialOrange left a comment

Choose a reason for hiding this comment

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

Sorry for the long delay, I was a bit busy. Let's fix the last description and I'll merge it.

Comment on lines 818 to 820
raise ConfigurationError("need to specify both host and port, "
"port only in the case on Unix sockets, "
"or socket_fd in the case of socket fd")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
raise ConfigurationError("need to specify both host and port, "
"port only in the case on Unix sockets, "
"or socket_fd in the case of socket fd")
raise ConfigurationError("need to specify host/port, "
"port (in case of Unix sockets)"
"or socket_fd")

This patch adds the ability to create Tarantool connection
using an existing socket fd.

To achieve this, several changes have been made to work with
non-blocking sockets, as `socket.socketpair` creates such [1].

The authentication [2] might have already occured when we establish
such a connection. If that's the case, there is no need to pass
'user' argument.

On success, connect takes ownership of the `socket_fd`.

1. tarantool/tarantool#8944
2. https://www.tarantool.io/en/doc/latest/dev_guide/internals/iproto/authentication/

Closes #304
@askalt askalt force-pushed the askalt/gh-304-socket-fd-connection branch from 543e17d to 8da5df2 Compare November 10, 2023 15:50
@DifferentialOrange DifferentialOrange merged commit 2983de8 into master Nov 13, 2023
@DifferentialOrange DifferentialOrange deleted the askalt/gh-304-socket-fd-connection branch November 13, 2023 07:39
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.

Create connection from socket fd
3 participants