Retry Deepcell in case of bad gateway (or similar) response errors #605

alex-l-kong · 2022-06-17T22:36:09Z

What is the purpose of this PR?

Closes #602, which indicates an error in decoding the JSON in case the Deepcell API returns a response indicating an error.

How did you implement your changes

Utilize the Retry module in requests and mount it to a requests.Session, which we use to invoke the post method to Deepcell.

We also add a try/except block around this statement in case none of the retries work and the resulting json cannot be decoded anyways.

Remaining issues

I'm a bit in the dark here because I've not encountered this error personally. @ngreenwald let me know if this resolves the issue, or if it will need additional work.

…returns an invalid response

alex-l-kong · 2022-06-18T05:13:56Z

@ngreenwald whoops, I forgot to request the review earlier. I wasn't able to replicate the bad gateway/json decode error on my end, let me know if you run into issues with the Retry module on your end.

… halted

alex-l-kong · 2022-06-18T05:32:20Z

@ngreenwald since we make a similar API calls later to our Redis database, I can implement the same Retry logic there if it works for Deepcell on your end.

ngreenwald · 2022-06-19T17:07:06Z

Just tried it, looks the error isn't being caught

JSONDecodeError                           Traceback (most recent call last)
/usr/local/lib/python3.6/site-packages/requests/models.py in json(self, **kwargs)
    909         try:
--> 910             return complexjson.loads(self.text, **kwargs)
    911         except JSONDecodeError as e:

/usr/local/lib/python3.6/json/__init__.py in loads(s, encoding, cls, object_hook, parse_float, parse_int, parse_constant, object_pairs_hook, **kw)
    353             parse_constant is None and object_pairs_hook is None and not kw):
--> 354         return _default_decoder.decode(s)
    355     if cls is None:

/usr/local/lib/python3.6/json/decoder.py in decode(self, s, _w)
    338         """
--> 339         obj, end = self.raw_decode(s, idx=_w(s, 0).end())
    340         end = _w(s, end).end()

/usr/local/lib/python3.6/json/decoder.py in raw_decode(self, s, idx)
    356         except StopIteration as err:
--> 357             raise JSONDecodeError("Expecting value", s, err.value) from None
    358         return obj, end

JSONDecodeError: Expecting value: line 1 column 1 (char 0)

During handling of the above exception, another exception occurred:

JSONDecodeError                           Traceback (most recent call last)
<ipython-input-9-c0157cd31fa5> in <module>
----> 1 deepcell_service_utils.create_deepcell_output(deepcell_input_dir, deepcell_output_dir, fovs=fovs, scale=rescale_factor, zip_size=10)

/usr/local/lib/python3.6/site-packages/ark/utils/deepcell_service_utils.py in create_deepcell_output(deepcell_input_dir, deepcell_output_dir, fovs, suffix, host, job_type, scale, timeout, zip_size, parallel)
    147             executor.shutdown(wait=True)
    148     else:
--> 149         list(map(_zip_run_extract, fov_groups, range(len(fov_groups))))
    150 
    151 

/usr/local/lib/python3.6/site-packages/ark/utils/deepcell_service_utils.py in _zip_run_extract(fov_group, group_index)
    117         print('Uploading files to DeepCell server.')
    118         status = run_deepcell_direct(
--> 119             zip_path, deepcell_output_dir, host, job_type, scale, timeout
    120         )
    121 

/usr/local/lib/python3.6/site-packages/ark/utils/deepcell_service_utils.py in run_deepcell_direct(input_dir, output_dir, host, job_type, scale, timeout, num_retries)
    244             json={
    245                 'hash': predict_hash,
--> 246                 'key': ["status", "progress", "output_url", "reason", "failures"]
    247             }
    248         ).json()

/usr/local/lib/python3.6/site-packages/requests/models.py in json(self, **kwargs)
    915                 raise RequestsJSONDecodeError(e.message)
    916             else:
--> 917                 raise RequestsJSONDecodeError(e.msg, e.doc, e.pos)
    918 
    919     @property

JSONDecodeError: [Errno Expecting value] <html>
<head><title>502 Bad Gateway</title></head>
<body>
<center><h1>502 Bad Gateway</h1></center>
<hr><center>nginx/1.17.10</center>
</body>
</html>
: 01

alex-l-kong · 2022-06-19T20:34:51Z

@ngreenwald in that case, seems likely that the main endpoint doesn't actually throw a 502 error, but simply returns an HTML object that indicates it encountered one (which can't be processed as JSON). I've explicitly added a check for JSONDecodeError, let me know if it works now.

ngreenwald

Looks like this did the trick. Now we need to deal with zip files that never got processed. There should be a status update that gets printed for the user with with the name of the zip file that wasn't processed if the retry times out.

I think having the 5 retries in place should do the trick, but if it doesn't, we'll need a record of what happened so that we can add functionality to retry for those zip files that failed.

I also added some print statements for debugging, you can remove those.

ark/utils/deepcell_service_utils.py

alex-l-kong · 2022-06-29T21:29:18Z

Looks like this did the trick. Now we need to deal with zip files that never got processed. There should be a status update that gets printed for the user with with the name of the zip file that wasn't processed if the retry times out.

I think having the 5 retries in place should do the trick, but if it doesn't, we'll need a record of what happened so that we can add functionality to retry for those zip files that failed.

I also added some print statements for debugging, you can remove those.

Here's how I'm thinking the process will work:

while True:
    # make calls in parallel
    if parallel:
        with ThreadPoolExecutor() as executor:
            executor.map(_zip_run_extract, fov_groups, range(len(fov_groups)))
            executor.shutdown(wait=True)
    else:
        list(map(_zip_run_extract, fov_groups, range(len(fov_groups))))

    # run check for zip file created for each `fov_group`
    # if they all exist, break
    # if not, then set fov_groups equal to the ones that remain, and stay in the loop

I can also add another status variable that will prevent this from getting stuck in long cycle in case of a catastrophic error on Deepcell's end, but based on your findings using the retry module, I doubt this loop will need to run more than a few times at most.

ngreenwald · 2022-06-29T22:32:38Z

Yeah, I agree that's the long-term solution. However, for now I think we should just print a message when this error occurs. Switching the default batch_size to 5, and retrying on failures, seems to have resolved all the problems for me.

Once other users report that they're running into the same issue, we can make the change, but it will require some refactoring of how the function works, since right now the output zip file isn't uniquely named. So we'll need to make the name matches with the input zip name, and also make sure that when the process is restarted the old zip files don't give a false positive for which file needs to get uploaded.

Can you open an issue describing it?

alex-l-kong · 2022-06-29T23:02:03Z

Yeah, I agree that's the long-term solution. However, for now I think we should just print a message when this error occurs. Switching the default batch_size to 5, and retrying on failures, seems to have resolved all the problems for me.

Once other users report that they're running into the same issue, we can make the change, but it will require some refactoring of how the function works, since right now the output zip file isn't uniquely named. So we'll need to make the name matches with the input zip name, and also make sure that when the process is restarted the old zip files don't give a false positive for which file needs to get uploaded.

Can you open an issue describing it?

Sounds good, I've just opened the issue.

I think the now-modified print statements we have do a pretty good job of notifying the user, I'll also include an additional print out of the FOVs that couldn't be processed with normal retry logic under the check for if status != 0.

…rk-analysis into deepcell_json_decode

Add retry option to run_deepcell_direct in case a call to Deepcell …

c802054

…returns an invalid response

alex-l-kong requested a review from ngreenwald June 18, 2022 05:12

alex-l-kong added 2 commits June 17, 2022 22:15

Ensure function returns if it cannot decode the JSON

08f9ab4

Ensure if Deepcell cannot connect then create_deepcell_output is also…

3dff0f0

… halted

Catch error if Deepcell returns a malformed JSON, indicating error

0b60f9b

alex-l-kong and others added 3 commits June 19, 2022 13:52

Add retry on JSONDecodeError functionality to Deepcell API call

6418d57

Merge branch 'master' into deepcell_json_decode

fc439fe

add print statements

e049ff3

ngreenwald requested changes Jun 29, 2022

View reviewed changes

ark/utils/deepcell_service_utils.py Show resolved Hide resolved

ark/utils/deepcell_service_utils.py Outdated Show resolved Hide resolved

ark/utils/deepcell_service_utils.py Outdated Show resolved Hide resolved

Address first round of code review comments

06f2abe

Merge branch 'master' into deepcell_json_decode

c5c4010

alex-l-kong added 2 commits June 29, 2022 16:14

Add printout to identify problematic FOVs during the call to Deepcell

99264dd

Merge branch 'deepcell_json_decode' of https://github.com/angelolab/a…

4941546

…rk-analysis into deepcell_json_decode

alex-l-kong requested a review from ngreenwald June 29, 2022 23:25

ngreenwald approved these changes Jun 30, 2022

View reviewed changes

ngreenwald merged commit ad6ccfc into master Jun 30, 2022

ngreenwald deleted the deepcell_json_decode branch June 30, 2022 17:46

srivarra mentioned this pull request Jul 1, 2022

Removal of force_ints in all load functions. #609

Merged

srivarra added the bug Something isn't working label Aug 16, 2022

alex-l-kong mentioned this pull request Sep 28, 2022

Retry missing fov_group zip files in case of DeepCell response error #628

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Retry Deepcell in case of bad gateway (or similar) response errors #605

Retry Deepcell in case of bad gateway (or similar) response errors #605

alex-l-kong commented Jun 17, 2022

alex-l-kong commented Jun 18, 2022

alex-l-kong commented Jun 18, 2022

ngreenwald commented Jun 19, 2022

alex-l-kong commented Jun 19, 2022 •

edited

Loading

ngreenwald left a comment

alex-l-kong commented Jun 29, 2022

ngreenwald commented Jun 29, 2022 •

edited

Loading

alex-l-kong commented Jun 29, 2022

Retry Deepcell in case of bad gateway (or similar) response errors #605

Retry Deepcell in case of bad gateway (or similar) response errors #605

Conversation

alex-l-kong commented Jun 17, 2022

alex-l-kong commented Jun 18, 2022

alex-l-kong commented Jun 18, 2022

ngreenwald commented Jun 19, 2022

alex-l-kong commented Jun 19, 2022 • edited Loading

ngreenwald left a comment

Choose a reason for hiding this comment

alex-l-kong commented Jun 29, 2022

ngreenwald commented Jun 29, 2022 • edited Loading

alex-l-kong commented Jun 29, 2022

alex-l-kong commented Jun 19, 2022 •

edited

Loading

ngreenwald commented Jun 29, 2022 •

edited

Loading