Skip to content

Ensure that grequestx continuously make progress #6991

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

Merged
merged 1 commit into from
Sep 18, 2019

Conversation

devreal
Copy link
Contributor

@devreal devreal commented Sep 18, 2019

The handling of the grequest extensions is missing the release of the in_progress guard, causing requests not being polled after the first invocation.

Signed-off-by: Joseph Schuchart [email protected]

@bosilca
Copy link
Member

bosilca commented Sep 18, 2019

Nice catch.

@jsquyres jsquyres added the bug label Sep 18, 2019
@jsquyres
Copy link
Member

Does this mean that grequests were, in general, broken? (i.e., never being advanced)

@bosilca
Copy link
Member

bosilca commented Sep 18, 2019

yup.

@jsquyres jsquyres added the NEWS label Sep 18, 2019
@jsquyres
Copy link
Member

This is NEWS-worthy, then...

@jsquyres
Copy link
Member

@devreal Can you cherry-pick to the appropriate branches?

@jsquyres jsquyres merged commit cc586d8 into open-mpi:master Sep 18, 2019
@devreal
Copy link
Contributor Author

devreal commented Sep 18, 2019

@jsquyres :

Does this mean that grequests were, in general, broken? (i.e., never being advanced)

I think only the extended grequests (see #24) were affected, the ones that come with a progress function. that are not in the MPI standard. AFAICS they are used internally by ROMIO but are not exposed to the user (unlike MPICH). The regular grequests should not be affected.

I will cherry-pick to the release branches.

@jsquyres
Copy link
Member

Oh, if there's no way for the user to be exposed to this bug, then it's not worth cherry-picking.

Specifically: are we using the extended grequests functionality in ROMIO?

@bosilca
Copy link
Member

bosilca commented Sep 18, 2019

indeed, the normal grequest are having their query function called as expected. I wonder how is ROMIO working if we don't progress their requests as expected ? If this functionality used in the current code ?

@devreal
Copy link
Contributor Author

devreal commented Sep 19, 2019

@bosilca Grepping through the romio code it seems that extended grequests are only used when a working aio library is found, for example here. Not sure how often this configuration is used.

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

Successfully merging this pull request may close these issues.

3 participants