Skip to content
This repository was archived by the owner on Oct 29, 2024. It is now read-only.

Using client behind AWS NAT #860

Closed
dgnorton opened this issue Oct 28, 2020 · 5 comments · Fixed by #890
Closed

Using client behind AWS NAT #860

dgnorton opened this issue Oct 28, 2020 · 5 comments · Fixed by #890
Assignees
@rhajek @gwossum

Comments

@dgnorton
Copy link

  • InfluxDB version: 1.8.2
  • InfluxDB-python version: 5.3.0
  • Python version: unknown
  • Operating system version: unsure - running on AWS and assume Linux

Using the Python InfluxDB client behind an AWS NAT gateway

The AWS NAT gateway will time out an idle connection after 350 seconds. The InfluxDB Python library uses the Python requests module under the hood to create a session and make all of the InfluxDB API calls using that session. While requests supports keep-alive (https://requests.readthedocs.io/en/latest/user/advanced/#keep-alive) it does not set the TCP socket to use keep alive and send ping probes to keep the connection from appearing to be idle. To work around this issue, the following code enables TCP keep-alives, which starts sending keep-alive probes after the connection has been idle for 60 seconds, sends a probe every 60 seconds and sends a maximum of 15 keep alive probes.

HTTPConnection.default_socket_options = HTTPConnection.default_socket_options + \
                [(socket.SOL_SOCKET, socket.SO_KEEPALIVE, 1),
                (socket.IPPROTO_TCP, socket.TCP_KEEPIDLE, 60),
                (socket.IPPROTO_TCP, socket.TCP_KEEPINTVL, 60),
                (socket.IPPROTO_TCP, socket.TCP_KEEPCNT, 15)]

LOGGER.info('Sending query to InfluxDB: query(%s)' % (query,))
client = InfluxDBClient(host, port, username, password, dbname, ssl, True)
response = client.query(query=query, database=dbname)
LOGGER.info(response)

Can this keep alive functionality be added to the InfluxDB Python client library so it doesn't have to be done by users of the library?
@tarainfluxd
Copy link
Member

Any update on this?

@dgnorton
Copy link
Author

dgnorton commented Jan 5, 2021

@rhajek is this issue on the radar?

@rhajek
Copy link
Contributor

rhajek commented Jan 6, 2021

@dgnorton I will look on this. Keep alive tcp settings can be different on various platforms (ex. socket.TCP_KEEPIDLE is undefined on macOS), this is probably why urllib3 and requests libs do not set it by default and let operating system dafaults.

The same issue we also have in the new client lib influxdb-client-python where we use the same urllib3/requests libs.

Will help you if we make constructor of InfluxDBClient to support custom socket_options like this ?

client = InfluxDBClient('localhost', 8086, 'root', 'root', 'telegraf',
                        socket_options=HTTPConnection.default_socket_options + \
                                       [(socket.SOL_SOCKET, socket.SO_KEEPALIVE, 1),
                                        (socket.IPPROTO_TCP, socket.TCP_KEEPIDLE, 60),
                                        (socket.IPPROTO_TCP, socket.TCP_KEEPINTVL, 60),
                                        (socket.IPPROTO_TCP, socket.TCP_KEEPCNT, 15)])

I'am not sure what tcp socket settings should be hardcoded in client by default, 15x probe every 60s means that connection can be held for 15min. This may be dangerous to enable for all people as a default.

@gwossum
Copy link
Member

gwossum commented Apr 27, 2021

Adding a socket_options optional keyword argument to the InfluxDBClient constructor as shown in #860 (comment) is a good plan. This will allow client code to set socket options that only apply to HTTP connections created by that particular InfluxDBClient object. The current work-around of setting HTTPConnection.default_socket_options works, but would change the socket options for all urllib / requests connections in the application, which could have unintended consequences. An optional keyword parameter to the InfluxDBClient constructor should not cause any issues for existing client code.

@rhajek rhajek mentioned this issue Apr 28, 2021
Merged
2 tasks
@psteinbachs
Copy link

@rhajek Could we please get an update on this? I see there is an open PR that is currently failing checks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@rhajek rhajek

@gwossum gwossum

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: Add custom socket_options rhajek/influxdb-python
5 participants
@dgnorton @rhajek @psteinbachs @gwossum @tarainfluxd