Review of the conductor.connection module
While I was reviewing !642 (merged), I listed a few issues I noticed within the connection module. I'm posting those here for further discussion:
-
server discovery:
- The UDP packet is only sent once, even though it could get lost.
- Does it make sense to use discovery if the hostname is already provided? It is currently used to get the TCP port. A default TCP port could be used instead, similar to the existing default UDP port.
-
TCP connection:
- According to this article,
TCP_NODELAY
is sometimes misused. Does it make sense in this context? -
socket.IP_TOS -> 0x10
is not documented.
- According to this article,
-
posix_ipc
:- It is treated as strong requirement in setup.py but optional in the connection module.
- Why not use UDS instead, so TCP is used regardless of the client and the server running on the same machine or not? This would get rid of the dependency and the code would be more maintainable.
-
TCP + posix queue:
- It seems that for local access, both connections are running at the same time. Wouldn't it be more maintainable to manage only one connection? For instance, assuming UDS has replaced the posix queue:
1. connect to TCP 2. get UDS name 3. close TCP 4. connect to UDS 5. transparent local UDS / remote INET use of the socket
- As a side note, does the local performance gain justify this specific code? (see my benchmark here)
- It seems that for local access, both connections are running at the same time. Wouldn't it be more maintainable to manage only one connection? For instance, assuming UDS has replaced the posix queue: