os/bluestore: invoke _prepare_ondisk_format_super as the last op #34616
Conversation
|
This looks plausible to me at a quick glance. Are we okay splitting up the format number update from the actual disk changes transaction, given that it could crash in between? Will pull this into today's cephfs integration run to check it fixes those, anyway. |
Well, this might be the case if crash happens during the upgrade from 1 to 2, and one decides to revert back to Ceph release with version 1. |
ifed01
force-pushed
the
wip-ifed-fix-upgrade-super
branch
from
5f9b5ab
to
3689255
Compare
|
This is working for the upgrade tests in the FS suite |
src/os/bluestore/BlueStore.cc
Outdated
| ceph_assert(r == 0); | ||
|
|
||
| ondisk_format = 4; | ||
| } else { |
There was a problem hiding this comment.
But I'm confused about this, if somebody's on bluestore v1 they have to upgrade more than once? I'm not sure what major releases this corresponds to but it makes me nervous.
There was a problem hiding this comment.
Yeah, you're right. Fixed.
ifed01
force-pushed
the
wip-ifed-fix-upgrade-super
branch
2 times, most recently
from
bb81de5
to
dea3a34
Compare
src/os/bluestore/BlueStore.cc
Outdated
| @@ -11084,6 +11084,9 @@ int BlueStore::_upgrade_super() | |||
| t->rmkey(PREFIX_SUPER, "min_min_alloc_size"); | |||
| } | |||
| ondisk_format = 2; | |||
|
|
|||
| // make sure upgrade is performed within a single transaction | |||
| // hence preparing new super record here other than at the end | |||
| _prepare_ondisk_format_super(t); | |||
There was a problem hiding this comment.
Well now I'm confused because AFAIK we just can't ever invoke _prepare_ondisk_format_super() until ondisk_format==4 (for now, the latest version anyway).
So either we have to fix whatever that requires, or else we have to do all the disk format changes in sequence and only invoke _prepare_ondisk_format_super() at the end.
I don't really know which pieces of what are important here so I can't help with those invariants unfortunately.
There was a problem hiding this comment.
Not sure I understand what's wrong with this version except being a bit sub-optimal. But here is another turn.
Fixes: https://tracker.ceph.com/issues/45133 Signed-off-by: Igor Fedotov <ifedotov@suse.com>
ifed01
force-pushed
the
wip-ifed-fix-upgrade-super
branch
from
dea3a34
to
e792e19
Compare
Fixes: https://tracker.ceph.com/issues/45133
Signed-off-by: Igor Fedotov ifedotov@suse.com
Checklist
Show available Jenkins commands
jenkins retest this pleasejenkins test classic perfjenkins test crimson perfjenkins test signedjenkins test make checkjenkins test make check arm64jenkins test submodulesjenkins test dashboardjenkins test dashboard backendjenkins test docsjenkins render docsjenkins test ceph-volume alljenkins test ceph-volume tox