Skip to content

Conversation

@vobinics
Copy link

@vobinics vobinics commented Jun 5, 2020

These changes do not violate the old code, but complement it
Unix socket connection is 30% faster than TCP
Now, if there is a file along the path /tmp/memcached.sock, then a unix connection is selected
I think this change will help other developers who need a Unix connection.

@aio-libs aio-libs deleted a comment from CLAassistant Jan 14, 2022
@Dreamsorcerer
Copy link
Member

Could you merge master and also add information to the documentation about this option?

class MemcachePool:

def __init__(self, host, port, *, minsize, maxsize, loop=None):
def __init__(self, host, port, *, minsize, maxsize, loop=None, unix=None):
Copy link
Member

Choose a reason for hiding this comment

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

I think this should accept host/port or unix. Currently a unix connection requires the host/port even though it is unused.

Copy link
Member

Choose a reason for hiding this comment

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

Or, we could try unix first, and then fallback to host/port if the connection fails. In which case, we should support host/port as None to disable this fallback behaviour and always get an error if the unix connection fails.

@Dreamsorcerer
Copy link
Member

We also need a new test.

So, if you'd like to finish off this PR and get it merged, we need:

  • Merge master.
  • Add a test.
  • Update documentation.
  • Allow using unix without host/port.

Sorry for the delay in reviewing.

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.

2 participants