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

Try to fix Elastic connection pooling issues #5760

Merged
merged 3 commits into from Jun 3, 2019

Conversation

@ericholscher
Copy link
Member

@ericholscher ericholscher commented Jun 3, 2019

This creates a new connection on every request,
hopefully working around the issues with the connection pool.

This will go out on 1 web and see if it resolves the issues at all.

This is branched from rel, which seems to have caused some weirdness :/

@ericholscher ericholscher requested a review from Jun 3, 2019
This creates a new connection on every request,
hopefully working around the issues with the connection pool.
This will go out on 1 web and see if it resolves the issues at all.
@ericholscher ericholscher force-pushed the hotfix-elastic-connection-pooling branch from 4f0d64e to f88378f Jun 3, 2019
@ericholscher ericholscher changed the base branch from master to rel Jun 3, 2019
Copy link
Contributor

@davidfischer davidfischer left a comment

The hotfix looks promising based on data in production. However, I can't say I fully understand everything here but it looks ok.

@safwanrahman
Copy link
Member

@safwanrahman safwanrahman commented Jun 3, 2019

@ericholscher When did you pushed it in production?

@ericholscher ericholscher merged commit 5eb9695 into rel Jun 3, 2019
2 checks passed
@delete-merged-branch delete-merged-branch bot deleted the hotfix-elastic-connection-pooling branch Jun 3, 2019
Copy link
Contributor

@agjohnson agjohnson left a comment

I think I understand the solution, though might be a good opportunity to comment how the fix works so we all have more context around the issue later.

# This creates a new connection on every request,
# but actually works :)
log.info('Hacking Elastic indexing to fix connection pooling')
self.using = Elasticsearch(**settings.ELASTICSEARCH_DSL['default'])
Copy link
Contributor

@agjohnson agjohnson Jun 3, 2019

What exactly is the fix here? I'm assuming Elasticsearch is normally instantiated with a class or instance for the connection pooling?

try:
# This is a common case that we should be handling a better way
doc_obj.update(queryset.iterator(), action='delete')
except Exception:
Copy link
Contributor

@agjohnson agjohnson Jun 3, 2019

We should catch a more specific exception here to prevent gobbling up any important exception that might be raised upstream.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants