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

mark_dead() in ConnectionPool always removes the first connection in the pool #1044

Closed
tongwang opened this issue Oct 25, 2019 · 3 comments
Closed

Comments

@tongwang
Copy link

ConnectionPool's mark_dead() method is supposed to remove the dead connection from the connections list:

self.connections.remove(connection)

However the equality comparison implementation __eq__ in the Connection class is incorrect. It always returns True as long as both objects are Connection instances:

def __eq__(self, other):
        if not isinstance(other, Connection):
            raise TypeError(
                "Unsupported equality check for %s and %s" % (self, other)
            )
        return True

This defect manifests itself by removing all good connections until the connections is empty. One dead connection will bring down the whole connection pool.

Proposed solution: removing __eq__ from Connection. Thoughts?

@honzakral
Copy link
Contributor

That is definitely the correct solution, would you be open to sending a PR?

Thank you!

alexk307 pushed a commit to alexk307/elasticsearch-py that referenced this issue Oct 29, 2019
@alexk307
Copy link

Took a stab at it here: #1048

@alexk307
Copy link

I ended up modifying the __eq__ method on the Connection object to just use the __hash__ method for equality comparison. I think we also could have deleted the __eq__ method entirely; either way would work IMHO.

alexk307 pushed a commit to alexk307/elasticsearch-py that referenced this issue Oct 29, 2019
Closes elastic#1044. Fixing bug with mark_dead in connection pool
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

No branches or pull requests

3 participants