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

Hanging dataloader #41

Open
chaffeqa opened this issue Sep 11, 2017 · 8 comments
Open

Hanging dataloader #41

chaffeqa opened this issue Sep 11, 2017 · 8 comments

Comments

@chaffeqa
Copy link

We have experianced issues with v2.0.1 where a promise(s) would not be resolved, and the graphene request would hang forever. I wanted to get a totally hopeful response that this may be a known issue, and that it is solved in cb9bbff

@syrusakbary
Copy link
Owner

Hi @chaffeqa,
Thanks for the response, that's something that happened before as you pointed, and should be easy to fix.

Could be possible to have a quick overview of your installed packages (version of graphene, promises, graphene-*, ...).
A github repo or a code sample that could help us to reproduce the issue will be also helpful!

@chaffeqa
Copy link
Author

# graphql tool
##############################
# graphene
# - Current version: https://github.com/graphql-python/graphene/commit/078230ad4906465a0757df7ca490c2990421d7d6
# - See missing updates: https://github.com/graphql-python/graphene/compare/078230ad4906465a0757df7ca490c2990421d7d6...graphql-python:master
graphene==1.4.1
# graphene-django
# - Current version: https://github.com/graphql-python/graphene-django/commit/cec1a8448048ce55a2844df63b295b8d9d5000d7
# - See missing updates: https://github.com/graphql-python/graphene-django/compare/cec1a8448048ce55a2844df63b295b8d9d5000d7...graphql-python:master
git+https://github.com/graphql-python/graphene-django.git@cec1a8448048ce55a2844df63b295b8d9d5000d7#egg=graphene-django
# - Current version: https://github.com/graphql-python/graphql-core/commit/bf9e156b791f349ab91f113e3dee0731331ad59f
# - See missing updates: https://github.com/graphql-python/graphql-core/compare/bf9e156b791f349ab91f113e3dee0731331ad59f...graphql-python:master
git+https://github.com/graphql-python/graphql-core.git@bf9e156b791f349ab91f113e3dee0731331ad59f#egg=graphql-core

Unfortunately I don't have a good bit of sample code, the integration was pretty intense though, lots of records, large batches, but I'm fairly confident that for every key we would try to load, we would return either a record or None in the correct array position

@syrusakbary
Copy link
Owner

Version of promise?

@chaffeqa
Copy link
Author

chaffeqa commented Sep 11, 2017

it seemed to resolve to 2.0.1

Requirement already satisfied: promise>=2.0 in /usr/local/var/pyenv/.../src/promise (from graphene==1.4.1->-r requirements/../requirements.txt (line 60))

# fresh install:
Collecting promise>=2.0 (from graphene==1.4.1->-r requirements/../requirements.txt (line 60))
  Using cached promise-2.0.2.tar.gz

@syrusakbary
Copy link
Owner

This issue is only caused by the promises package.

I believe the issue was fixed in 2.0.2 with this commit: 57d2058.

You can try to see if with 2.0.2 this issue is still happening.
If that's the case, might be worth to try: pip install "promise>=2.1.dev".

Please keep me in the loop with the results :)

@chaffeqa
Copy link
Author

chaffeqa commented Sep 11, 2017

Actually we ran into that issue, but only with the clear(key).prime(key, record) usage.

However that doesn't seem to address the issue of "hanging requests" due to unfulfilled promises (unless I'm missing something deeper in the code that it effects)

The main reason for writing this is because I'm curious whether this commit may deal with the bug we were having. I didn't dive in deep enough to see what case it addresses, but we definitely didnt have it in our codebase

@chaffeqa
Copy link
Author

Thanks for the info! We can confirm 2.0.2 fixes the issue we saw.

I have a further question though, I did some diffing to see the changes (FYI I think the git tags and pypi versions are pretty confusing right now, but understandable since you have so many projects to keep in sync here 😃 ):

I wanted to ask about how you feel 2.1.x (master) changes from 2.0.2 are as:

  • What issues you think they address
  • Stability

@chaffeqa
Copy link
Author

I just sent you an email containing a stacktrace referencing a bug we encountered on 2.0.2, we are trying now on 2.1.dev20170724043809and will update if we see it there too. We are hoping that the change cleans up the bug.

From my understanding, the callback chain is losing context somewhere, and ends up calling context_instance._exit() inside a callback, but that function has lost its context, and self._exited throws a AttributeError since it doesn't exist (or has been GC'd?).

Either way, I wish I had more time to isolate the issue and make a test, but at this point this is the best I can do. Hope its helpful, and even more that the issue is fixed with your moving away from Context 🎉

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

2 participants