Project

General

Profile

Actions

Bug #6442

closed

mon: crush_ops.sh, test.py failures on master

Added by Sage Weil over 10 years ago. Updated over 10 years ago.

Status:
Resolved
Priority:
Urgent
Category:
Monitor
Target version:
-
% Done:

0%

Source:
Q/A
Tags:
Backport:
Regression:
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:
Pull request ID:
Crash signature (v1):
Crash signature (v2):

Description

I see a series of failures that look like the mon just comes up with bad osdmap data after (re)starting a down mon:

ubuntu@teuthology:/a/teuthology-2013-09-27_23:00:04-rados-master-testing-basic-plana/21761
ubuntu@teuthology:/a/teuthology-2013-09-27_23:00:04-rados-master-testing-basic-plana/21695
ubuntu@teuthology:/a/teuthology-2013-09-27_23:00:04-rados-master-testing-basic-plana/21651

haven't dug in, but this definitely smells like a regression.

I suspect that if we look at the started mon we'll see that it has some broken view of the osdmap that is causing the troubles. full logs available for all of the above.


Related issues 1 (0 open1 closed)

Related to Ceph - Bug #6322: mon: osdmon: write 'latest_full' after rebuilding full version from incrementals on update_from_paxos()ResolvedJoao Eduardo Luis09/15/2013

Actions
Actions #1

Updated by Joao Eduardo Luis over 10 years ago

  • Status changed from New to In Progress
Actions #2

Updated by Sage Weil over 10 years ago

  • Status changed from In Progress to Resolved
Actions #3

Updated by Joao Eduardo Luis over 10 years ago

Given the simplistic nature of this one-line patch that fixed a considerably complex issue, I believe some explanation on why and what is deemed desirable for future reference.

To fix #6322 we patched the monitor with 81983bab3630520d6c7ee9b7e4a747bc17b8c5c3. This patch made sure that we would write down the latest full map version to osdmap:full_latest, allowing the monitor to read in the latest available full map on update_from_paxos() instead of rebuilding the osdmap from the oldest available full map plus all the subsequent incremental versions. While this makes perfect sense, it had an unforeseen side effect: this bug.

During normal monitor behavior, when a new version is generated we create an incremental. When said version is committed, we update_from_paxos(), apply said incremental to the in-memory state and create a full version by dumping the in-memory state to an on-disk full map version. We then update osdmap:full_latest with the current osdmap version.

While everything will work out as expected while the monitor is running and applying the incremental versions to its in-memory state, as well as they will if the monitor is started and its on-disk versions allow him to join the quorum, it will however have troubles if it needs to synchronize its way into quorum -- and this is the wtf nature of this bug.

The bug itself was little more than CrushWrapper (the class that wraps around the crushmap implementation and keeps it nice and healthy) not updating its in-memory state from whatever is the on-disk state.

CrushWrapper would update its in-memory state when the CrushWrapper class happened to be first created and initialized with a crushmap. This happens for sure when the monitor is started, the OSDMap class being fresh and its CrushWrapper instance being fresh along with it. OSDMap::apply_incremental() also made sure to create a new instance of CrushWrapper before decoding the new crushmap, after applying the incremental onto its in-memory state.

However, when we synchronized from an existing monitor, we sort of skipped this whole process by simply reading in the osdmap version out of disk and decoding it into memory. As we got our keys, along with the already built full maps, from someone else -- and we already had been started, so our CrushWrapper instance wasn't fresh --, all this amounts to not needing to apply any incremental versions, nor having a fresh CrushWrapper instance that is willing to build the in-memory state out of whatever is the crushmap.

The patch 9b7a2ae329b6a511064dd3d6e549ba61f52cfd21 thus makes sure that we always rebuild the in-memory state whenever we decode a new crushmap. Instead of just loading up the crushmap into memory, we make sure that we also rebuild its in-memory representation. And that's just the one line on the patch. Sage's da man.

Actions

Also available in: Atom PDF