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

librados: fix unitialized timeout in wait_for_osdmap #24721

Merged
merged 1 commit into from
Oct 25, 2018

Conversation

cbodley
Copy link
Contributor

@cbodley cbodley commented Oct 23, 2018

ceph::timespan is not zero-initialized. use std::optional for an explicit empty state

this is causing RGW=1 vstart.sh -n to sometimes fail in radosgw-admin commands with:

2018-10-23 15:13:26.107 7efe9f50e280 -1 librados: timed out waiting for first osdmap from monitors
couldn't init storage provider

@mogeb
Copy link
Contributor

mogeb commented Oct 23, 2018

Ah, although I did test that explicitly in unit tests but it seems like I still missed it. Looks good, there's an alternative though that is used in other places in the source code where the timespan is initialized to 0: ceph::timespan duration{0}; .

Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley
Copy link
Contributor Author

cbodley commented Oct 23, 2018

thanks, i like that better. updated

Copy link
Contributor

@tchaikov tchaikov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@cbodley
Copy link
Contributor Author

cbodley commented Oct 25, 2018

this passed the rgw suite in http://pulpito.ceph.com/cbodley-2018-10-24_15:36:40-rgw-wip-cbodley-testing-distro-basic-smithi/ (failures are unrelated). does this need to go through any other suites?

@tchaikov tchaikov merged commit 2e6fd44 into ceph:master Oct 25, 2018
@tchaikov
Copy link
Contributor

@cbodley no, i think it's good to merge.

@cbodley cbodley deleted the wip-librados-mono-timeout branch October 25, 2018 15:55
@cbodley
Copy link
Contributor Author

cbodley commented Oct 25, 2018

thanks @tchaikov @mogeb!

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