-
Notifications
You must be signed in to change notification settings - Fork 0
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
Intent Parser returning intermittent HTTP 500 Error #202
Comments
In GitLab by @MWeston on May 26, 2020, 17:36 I was able to pull the following traces from the logs. It looks like the container eventually crashes at the end. [INFO ] 2020-05-26 21:31:20,990 intent_parser_server.py line:174 Traceback (most recent call last): [INFO ] 2020-05-26 21:31:20,991 intent_parser_server.py line:174 Traceback (most recent call last): File "/usr/src/intent_parser/server/intent_parser_server.py", line 166, in handle_client_connection |
In GitLab by @MWeston on May 26, 2020, 17:40 @tramyn this may be relevant: https://github.com/googleapis/google-api-python-client/blob/master/docs/thread_safety.md |
In GitLab by @MWeston on May 28, 2020, 11:42 mentioned in merge request cp-request!438 |
In GitLab by @tramyn on May 28, 2020, 14:44 mentioned in merge request !115 |
In GitLab by @tramyn on May 28, 2020, 14:44 created merge request !115 to address this issue |
In GitLab by @tramyn on Jun 10, 2020, 14:57 @MWeston, Can you provide me reasons on why this is classified as a high priority issue? Right now, I see this issue as a medium to low priority. I am hesitant to address this issue in 2.6 because a lot of code changes were made to support parsing control tables and linking URIs correctly. I want to minimize the type of issues that could occur with this new release so that they are all related to parsing tables. I understand that there is a workaround solution and that's why I think it is safe to push this issue to a later milestone release. |
In GitLab by @MWeston on Jun 10, 2020, 16:15 @tramyn sure. Your change pushed this issue back to 2.8 - this is two releases away (we're currently at 2.6, 2.7 is next). I am requesting this be looked at in 2.7 next before we do any new feature requests. Alternatively something like a minor fix release of "2.6.1". This is a small change, but it's important. Currently, any call to the intent-parser can fail because we're calling the Google API Python Client from a multi-threaded environment. As documented, this is not supported by the underlying use of the httplib2 library and can non-deterministically fail. Any client currently using the intent parser (of which we are just one example) has to defensively code around this non-determinism. Even then, there's no guarantee we'll get correct behavior. The choices of the clients are:
We could say: "Yes, if we make enough calls to the IP, the probability of this happening approaches 0" but this is not desirable from the perspective of:
This change should have no effect on any of the actual parsing output. As documented above, the suggested solutions are to 1) build an accessor http service for the google api accessor used by each thread or 2) re-use a single http instance in the accessor initialization. |
In GitLab by @tramyn on Jun 18, 2020, 01:14 @MWeston the above solution is producing a google credential issue that intent parser will need to address. This credential issue occurs when fetching information from the SD2 Dictionary Maintainer. I'm not positive if the above solution will resolve this issue but I'll know more after I spend some time isolating each google APIs to run with its own credential file. |
@mwes I looked into this issue and it turns out that IP was maxing out it's given quota for fetching information from the SBOL Spreadsheet. To resolve this, I had to filter through IP to ensure that functions accessing the spreadsheet should use IP's cached version of the spreadsheet and not directly calling the API. As it turns out, one function had this issue and to confirm it further, this function is what we see in the error report (mapping common names to Transcriptic UID). This function has been updated. Now, one task remain. When IP has reached its quota limit for caching information from the spreadsheet, IP will need to safely catch this HTTP 500 error and bypass caching the spreadsheet until the next scheduled call. |
Great, thanks @tramyn |
This should be fixed in 746ae7d. |
In GitLab by @MWeston on May 26, 2020, 17:25
@dbryce noticed some issues parsing an experiment request:
http://intentparser.sd2e.org/document_request?1fFcxyJyheMrzSsVoSsO6v7qHJKFf_0heIFtqEur02cg
Trying to reproduce, I built out a minimal example:
Running this gives inconsistent results. Sometimes we'll get the JSON returned, and sometimes we'll get an HTTP 500.
Running more than one instance of the above seems to increase the likelihood of the error, but even then it's not consistent/guaranteed.
@tramyn can you look into this? I'm not sure what's going on, but it's non-deterministic. Maybe a possible state or race condition issue related to load?
The text was updated successfully, but these errors were encountered: