mon/OSDMonitor: do not propose on error in prepare_update #50502

batrick · 2023-03-13T16:21:11Z

Fixes: https://tracker.ceph.com/issues/58972

Contribution Guidelines

To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "pacific"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard cephadm
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox
jenkins test windows

github-actions · 2023-05-25T16:33:03Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

github-actions · 2023-05-25T16:33:04Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

ljflores · 2023-05-30T20:03:28Z

jenkins test make check

ljflores · 2023-06-23T17:19:49Z

Hey @batrick, seeing some failures that seem related to this PR. I'm not entirely sure though, so I've asked Yuri to drop this and rebase so I can confirm. In the meantime, can you see if these jobs look related to your changes?

http://pulpito.front.sepia.ceph.com/yuriw-2023-06-22_15:05:06-rados-wip-yuri7-testing-2023-06-12-1220-distro-default-smithi/7311772/
http://pulpito.front.sepia.ceph.com/yuriw-2023-06-22_15:05:06-rados-wip-yuri7-testing-2023-06-12-1220-distro-default-smithi/7311736/
http://pulpito.front.sepia.ceph.com/yuriw-2023-06-22_15:05:06-rados-wip-yuri7-testing-2023-06-12-1220-distro-default-smithi/7311737/
http://pulpito.front.sepia.ceph.com/yuriw-2023-06-22_15:05:06-rados-wip-yuri7-testing-2023-06-12-1220-distro-default-smithi/7311739/
http://pulpito.front.sepia.ceph.com/yuriw-2023-06-22_15:05:06-rados-wip-yuri7-testing-2023-06-12-1220-distro-default-smithi/7311738/
http://pulpito.front.sepia.ceph.com/yuriw-2023-06-22_15:05:06-rados-wip-yuri7-testing-2023-06-12-1220-distro-default-smithi/7311749/
http://pulpito.front.sepia.ceph.com/yuriw-2023-06-22_15:05:06-rados-wip-yuri7-testing-2023-06-12-1220-distro-default-smithi/7311750/
http://pulpito.front.sepia.ceph.com/yuriw-2023-06-22_15:05:06-rados-wip-yuri7-testing-2023-06-12-1220-distro-default-smithi/7311752/

(there are several more not listed on that link- but seemed redundant to list all of them) It seems like a lot, but they all have one thing in common: All the failures seem to happen when creating erasure pools. I'm guessing there is a mon command that creates erasure pools that has a problem with it.

batrick · 2023-06-28T17:54:57Z

src/mon/OSDMonitor.cc

      case -EAGAIN:
 	wait_for_finished_proposal(op, new C_RetryMessage(this, op));
-	return true;
+	return false;


@ljflores @rzarzynski The reason for the failure 7311772 in

#50502 (comment)

is because of this line. The method OSDMonitor::prepare_pool_crush_rule actually changes the crush rules and signals -EAGAIN. It's expected that ::prepare_command_impl will return true; here to trigger a PAXOS proposal. And then, retry the pool creation.

This is really strange to me. Is it actually necessary to split this pool creation into two OSDMap epochs?

2023-06-22T15:48:31.674+0000 7f79267f3700 1 -- v1:172.21.15.182:6789/0 <== client.4347 v1:172.21.15.182:0/3391191032 8 ==== mon_command({"prefix": "osd pool create", "pool": "unique_pool_0", "pg_num": 1, "pgp_num": 1, "pool_type": "erasure", "erasure_code_profile": "backfill_toofull"} v 0) v1 ==== 191+0+0 (unknown 3185792114 0 0) 0x558ec2cf4900 con 0x558ec2f0cc00 2023-06-22T15:48:31.674+0000 7f79267f3700 20 mon.a@0(leader) e1 _ms_dispatch existing session 0x558ec30a2b40 for client.? 2023-06-22T15:48:31.674+0000 7f79267f3700 20 mon.a@0(leader) e1 entity_name client.admin global_id 4347 (new_ok) caps allow * 2023-06-22T15:48:31.674+0000 7f79267f3700 0 mon.a@0(leader) e1 handle_command mon_command({"prefix": "osd pool create", "pool": "unique_pool_0", "pg_num": 1, "pgp_num": 1, "pool_type": "erasure", "erasure_code_profile": "backfill_toofull"} v 0) v1 2023-06-22T15:48:31.674+0000 7f79267f3700 20 MonCap is_capable service=osd command=osd pool create read write addr v1:172.21.15.182:0/3391191032 on cap allow * 2023-06-22T15:48:31.674+0000 7f79267f3700 20 MonCap allow so far , doing grant allow * 2023-06-22T15:48:31.674+0000 7f79267f3700 20 MonCap allow all 2023-06-22T15:48:31.674+0000 7f79267f3700 10 mon.a@0(leader) e1 _allowed_command capable 2023-06-22T15:48:31.674+0000 7f79267f3700 0 log_channel(audit) log [INF] : from='client.? v1:172.21.15.182:0/3391191032' entity='client.admin' cmd=[{"prefix": "osd pool create", "pool": "unique_pool_0", "pg_num": 1, "pgp_num": 1, "pool_type": "erasure", "erasure_code_profile": "backfill_toofull"}]: dispatch 2023-06-22T15:48:31.674+0000 7f79267f3700 1 -- v1:172.21.15.182:6789/0 --> v1:172.21.15.182:6789/0 -- log(1 entries from seq 78 at 2023-06-22T15:48:31.675841+0000) v1 -- 0x558ec3060a80 con 0x558ec1bd1800 2023-06-22T15:48:31.674+0000 7f79267f3700 10 mon.a@0(leader).paxosservice(osdmap 1..13) dispatch 0x558ec2cf4900 mon_command({"prefix": "osd pool create", "pool": "unique_pool_0", "pg_num": 1, "pgp_num": 1, "pool_type": "erasure", "erasure_code_profile": "backfill_toofull"} v 0) v1 from client.4347 v1:172.21.15.182:0/3391191032 con 0x558ec2f0cc00 2023-06-22T15:48:31.674+0000 7f79267f3700 5 mon.a@0(leader).paxos(paxos active c 1..51) is_readable = 1 - now=2023-06-22T15:48:31.675871+0000 lease_expire=2023-06-22T15:48:36.349426+0000 has v0 lc 51 2023-06-22T15:48:31.674+0000 7f79267f3700 10 mon.a@0(leader).osd e13 preprocess_query mon_command({"prefix": "osd pool create", "pool": "unique_pool_0", "pg_num": 1, "pgp_num": 1, "pool_type": "erasure", "erasure_code_profile": "backfill_toofull"} v 0) v1 from client.4347 v1:172.21.15.182:0/3391191032 2023-06-22T15:48:31.674+0000 7f79267f3700 7 mon.a@0(leader).osd e13 prepare_update mon_command({"prefix": "osd pool create", "pool": "unique_pool_0", "pg_num": 1, "pgp_num": 1, "pool_type": "erasure", "erasure_code_profile": "backfill_toofull"} v 0) v1 from client.4347 v1:172.21.15.182:0/3391191032 2023-06-22T15:48:31.674+0000 7f79267f3700 1 mon.a@0(leader).osd e13 implicitly use rule named after the pool: unique_pool_0 2023-06-22T15:48:31.674+0000 7f79267f3700 10 mon.a@0(leader).osd e13 prepare_pool_crush_rule returns -11

from the mon log

I took another look. It doesn't look like there's a good reason except some code would need adjusted to look at the pending incremental OSDMap for the erasure code profiles.

For the purposes of this PR I will undo this change but add a comment protesting the current-state of the code :)

ljflores · 2023-06-28T22:10:06Z

@rzarzynski requesting your review if you have a moment.

rzarzynski · 2023-07-07T13:52:49Z

Review-in-progress.

ljflores · 2023-07-10T23:28:47Z

Rados approved: https://tracker.ceph.com/projects/rados/wiki/MAIN#httpstrellocomcbn3IMWEB1783-wip-yuri7-testing-2023-06-23-1022-old-wip-yuri7-testing-2023-06-12-1220-old-wip-yuri7-testing-2023-06-09-1607

ljflores · 2023-07-10T23:28:52Z

jenkins test api

ljflores · 2023-07-12T18:13:31Z

Rados approved: https://tracker.ceph.com/projects/rados/wiki/MAIN#httpstrellocomcbn3IMWEB1783-wip-yuri7-testing-2023-06-23-1022-old-wip-yuri7-testing-2023-06-12-1220-old-wip-yuri7-testing-2023-06-09-1607

Apologies, this comment was confusing. I had finished that batch, but forgot this one had been dropped. It needs to be added to a new batch.

batrick · 2023-08-03T12:44:18Z

jenkins test api

Fixes: https://tracker.ceph.com/issues/58972 Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

ljflores · 2023-08-25T21:11:57Z

Rados approved: https://tracker.ceph.com/projects/rados/wiki/MAIN#httpstrellocomci87i4GUf1826-wip-yuri10-testing-2023-08-17-1444-old-wip-yuri10-testing-2023-08-15-1601-old-wip-yuri10-testing-2023-08-15-1009

batrick added the core label Mar 13, 2023

github-actions bot added the mon label Mar 13, 2023

batrick mentioned this pull request Mar 13, 2023

mon/MgrMonitor: plug PAXOS for batched MgrMap/OSDMap #50404

Merged

14 tasks

batrick force-pushed the i58972 branch from 52d5702 to 034ba3c Compare March 13, 2023 17:50

batrick force-pushed the i58972 branch 2 times, most recently from f048021 to 863a45d Compare April 6, 2023 12:56

batrick added the needs-review label Apr 6, 2023

batrick marked this pull request as ready for review April 6, 2023 12:56

batrick requested a review from a team as a code owner April 6, 2023 12:56

github-actions bot added the needs-rebase label May 25, 2023

batrick force-pushed the i58972 branch from 863a45d to dfdd627 Compare May 30, 2023 16:42

github-actions bot removed the needs-rebase label May 30, 2023

ljflores added needs-qa and removed needs-qa labels May 30, 2023

batrick force-pushed the i58972 branch from dfdd627 to 0d6d3c9 Compare May 30, 2023 18:43

ljflores approved these changes May 30, 2023

View reviewed changes

ljflores added the needs-qa label May 30, 2023

ljflores added the wip-yuri7-testing label Jun 9, 2023

ljflores removed the needs-review label Jun 20, 2023

yuriw removed the wip-yuri7-testing label Jun 23, 2023

batrick commented Jun 28, 2023

View reviewed changes

ljflores requested a review from rzarzynski June 28, 2023 22:09

batrick force-pushed the i58972 branch from 0d6d3c9 to 649dc2d Compare June 29, 2023 14:16

mon/OSDMonitor: do not propose on error in prepare_update

3974c37

Fixes: https://tracker.ceph.com/issues/58972 Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

batrick force-pushed the i58972 branch from 649dc2d to 3974c37 Compare August 4, 2023 15:08

ljflores added the wip-yuri10-testing label Aug 15, 2023

batrick merged commit 8043b3d into ceph:main Aug 28, 2023
11 checks passed

batrick deleted the i58972 branch August 28, 2023 13:29

batrick mentioned this pull request Aug 28, 2023

reef: mon/OSDMonitor: do not propose on error in prepare_update #53186

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mon/OSDMonitor: do not propose on error in prepare_update #50502

mon/OSDMonitor: do not propose on error in prepare_update #50502

batrick commented Mar 13, 2023 •

edited

github-actions bot commented May 25, 2023

github-actions bot commented May 25, 2023

ljflores commented May 30, 2023

ljflores commented Jun 23, 2023

batrick Jun 28, 2023

batrick Jun 28, 2023

batrick Jun 29, 2023

ljflores commented Jun 28, 2023

rzarzynski commented Jul 7, 2023

ljflores commented Jul 10, 2023

ljflores commented Jul 10, 2023

ljflores commented Jul 12, 2023

batrick commented Aug 3, 2023

ljflores commented Aug 25, 2023

mon/OSDMonitor: do not propose on error in prepare_update #50502

mon/OSDMonitor: do not propose on error in prepare_update #50502

Conversation

batrick commented Mar 13, 2023 • edited

Contribution Guidelines

Checklist

github-actions bot commented May 25, 2023

github-actions bot commented May 25, 2023

ljflores commented May 30, 2023

ljflores commented Jun 23, 2023

batrick Jun 28, 2023

Choose a reason for hiding this comment

batrick Jun 28, 2023

Choose a reason for hiding this comment

batrick Jun 29, 2023

Choose a reason for hiding this comment

ljflores commented Jun 28, 2023

rzarzynski commented Jul 7, 2023

ljflores commented Jul 10, 2023

ljflores commented Jul 10, 2023

ljflores commented Jul 12, 2023

batrick commented Aug 3, 2023

ljflores commented Aug 25, 2023

batrick commented Mar 13, 2023 •

edited