Skip to content

[5.28] Cache sockname and peername - #1313

Open
robsdedude wants to merge 3 commits into
5.0from
fix/own-sockname-caching
Open

[5.28] Cache sockname and peername#1313
robsdedude wants to merge 3 commits into
5.0from
fix/own-sockname-caching

Conversation

@robsdedude

@robsdedude robsdedude commented Jun 5, 2026

Copy link
Copy Markdown
Member

Async

Instead of relying on asyncio's caching of these properties, we do it ourselves. The advantage is that asyncio populates the cache on a best-effort basis. This can lead to the values being None if retrieving them causes an OSError.

This is a misalignment between the async and sync driver that this PR aims to remedy.

Sync

While in the async driver we're introducing (custom) caching where implicit caching was already in place, we also introduce caching of these fields in the sync driver for parity (and potentially fewer syscalls).

Fixes: #1310
Part of: DRIVERS-418

Async
-----
Instead of relying on asyncio's caching of these properties, we do it ourselves.
The advantage is that asyncio populates the cache on a best-effort basis. This
can lead to the values being `None` if retrieving them causes an `OSError`.

This is a misalignment between the async and sync driver that this PR aims to
remedy.

Sync
----
While in then async driver we're introducing (custom) caching where implicit
caching was already in place, we also introduce caching of these fields in the
sync driver for parity.
@robsdedude
robsdedude force-pushed the fix/own-sockname-caching branch from b60baef to f1ec1fa Compare June 5, 2026 13:19
@robsdedude
robsdedude marked this pull request as ready for review June 8, 2026 09:03

@AndyHeap-NeoTech AndyHeap-NeoTech left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, just one small comment for clarification.

Comment thread src/neo4j/_async_compat/network/_bolt_socket.py Outdated

@AndyHeap-NeoTech AndyHeap-NeoTech left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM.

@robsdedude

Copy link
Copy Markdown
Member Author

Important

NOT READY FOR MERGE
Waiting for user feedback to confirm/disprove proposed fixed.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants