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
mon/OSDMonitor: encode full osdmaps with features all OSDs can understand #11284
Conversation
We should discuss automatically setting the flags - it's silly to add extra steps for users to jump through for every upgrade. Perhaps some heuristic like 'after all up osds are kraken for 24h, require the kraken feature bit'. |
Yeah. It's a bit tricky because we need to avoid situations where all the un-upgraded osds crash or are stopped and only upgraded osds are up. It could be gated on the cluster being healthy, perhaps... or all in osds being up. |
@@ -1220,13 +1220,25 @@ void OSDMonitor::encode_pending(MonitorDBStore::TransactionRef t) | |||
} | |||
} | |||
|
|||
// determine appropriate features | |||
uint64_t features = mon->quorum_features; | |||
if (!newmap.has_flag(CEPH_ODSMAP_REQUIRE_JEWEL)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/ODS/OSD/
If the JEWEL or KRAKEN flags aren't set, encode the full map without those features. This ensure that older OSDs in the cluster will be able to correctly encode the full map with a matching CRC. At least, that is true as long as the encoding changes are guarded by those feature bits. That appears to be true currently, and we plan to ensure that it is true in the future as well. Signed-off-by: Sage Weil <sage@redhat.com>
5d34061
to
37c8d90
Compare
lgtm. @liewegas we need run this thru at least one upgrade test, and probably you could include tchaikov@31233d6 in your PR? and try to remove 'failed to encode map' from (at least some of) the upgrade tests. i will take care of this once this change is merged. |
do we want to backport this change to jewel? if yes, might need to connect it to http://tracker.ceph.com/issues/17386 . |
Yeah, I think we should backport.
|
@liewegas how's test going? |
trying to get it through upgrade/jewel-x
|
We want to prompt users to set these flags as soon as their upgrades complete. Signed-off-by: Sage Weil <sage@redhat.com>
37c8d90
to
12e5083
Compare
Lots of checks look for the jewel flag; setting the kraken flag should also set this one. Signed-off-by: Sage Weil <sage@redhat.com>
Also prompt users to set the require_*_osds flags once tey finish their upgrades.