Skip to content
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

Fix bolt handshake not having a timeout #905

Merged
merged 3 commits into from
Mar 30, 2023

Conversation

robsdedude
Copy link
Member

@robsdedude robsdedude commented Mar 29, 2023

During the bolt version negotiation the socket was not constrained by any timeout. This could, in rare occasions, cause the driver to get stuck until some other mechanism (e.g., socket keep alive) would free it.

The handshake is now limited by the connection_acquisition_timeout.

Introducing tests in neo4j-drivers/testkit#546

During the bolt version negotiation the socket was not constrained by any
timeout. This could, in rare occasions cause the driver to get stuck until some
other mechanism (e.g., socket keep alive) would free it.

The handshake is not limited by the `connection_acquisition_timeout`.
Copy link
Contributor

@bigmontz bigmontz left a comment

Choose a reason for hiding this comment

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

⏲️

@robsdedude robsdedude merged commit 59136a7 into neo4j:5.0 Mar 30, 2023
@robsdedude robsdedude deleted the bolt-handshake-timeout branch March 30, 2023 09:23
Copy link

@AndyHeap-NeoTech AndyHeap-NeoTech left a comment

Choose a reason for hiding this comment

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

With my limited Python this appears to all look good to me. Was initially questioning the need for separate timeouts for TCP and handshake, but on thinking about it I believe you are correct in taking that route.

robsdedude added a commit to robsdedude/neo4j-python-driver that referenced this pull request Apr 11, 2023
robsdedude added a commit that referenced this pull request Apr 13, 2023
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.

3 participants