Hey @JoshSalomon I see the problem with the Signed-off-by failure. A few of your commits are missing the doc: prefix. The format of all your commit titles, for this PR, should be doc: ________.

Here is the error message:

raise AssertionError("\n".join([
                "The title/s of following commit/s is/are not started with 'doc', but they only touch files under 'doc/'. Please make sure the commit titles",
                "are started with 'doc'. See the 'Submitting Patches' guide:",
                "https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#commit-title",

If you fix the commits that are missing a prefix, you should be good.

@JoshSalomon also, not sure if this will trigger the Signed-off-by test, but you might want to reconfigure your name to be "Josh Salomon" instead of your email, like so:

Signed-off-by: Josh Salomon <josh.salomon@gmail.com>

That is generally the convention for contributions.

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

@JoshSalomon also, not sure if this will trigger the Signed-off-by test, but you might want to reconfigure your name to be "Josh Salomon" instead of your email, like so:

Signed-off-by: Josh Salomon <josh.salomon@gmail.com>

That is generally the convention for contributions.

I know and I changed it - what is left is commit before the change

jenkins test signed

@JoshSalomon the Signed-off-by check looks good! I see that now the branch needs a rebase. Let me know if you need any help resolving the conflicts.

Also, are the new commits from other people meant to be in this PR? Or were they unintentionally added after the force push? If it's the latter, let me know if you need help removing the irrelevant commits.

@JoshSalomon I see now that you actually can remove/add labels on drafts. If you want to do that, hit the gear icon next to "Labels" in the upper right corner, and you should be able to edit as you need.

Hey @JoshSalomon,

Your PR looks okay to me. If you ever see this in the make check details:

 Identified problems

  | CTest Failure1 tests failed out of 258  Total Test time (real) = 1304.34 sec  The following tests FAILED: 	 32 - run-rbd-unit-tests-61.sh (Failed)

Know that it is a known failure, and that usually a retest will solve the issue. You are not making any modifications to rbd, so it's clear that this failure is unrelated.

Same goes for docs. For whatever reason, there are failures going on right now in docs, but your modifications to the docs passed before, so they are likely unrelated. If you ever are curious about why the docs are failing, you can click on "Details" next to "Required" and inspect the error message. But I believe in your case, it's unrelated.

Edit:
If you ever want to retest your PR, leave a comment that says jenkins retest this please. This comment will tell jenkins to retrigger the tests.

Here is a list of all available jenkins commands, which you can trigger in the same way as above-- by leaving a comment. This list is available on all PRs when you click on the dropdown menu that says "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 retest this please

jenkins test api

jenkins test docs

jenkins render docs

@JoshSalomon are you ready to take this out of draft form? If so, you can select "Ready for review" next to "This pull request is still a work in progress."

Also, let me know when you are ready for reviews. You can add me as a reviewer, and also it might be a good idea to add @neha-ojha and @jdurgin.

jenkins render docs

This is the error message from the render docs rest:

Command "python setup.py egg_info" failed with error code 1 in /tmp/pip-ffhm0q7k-build/

Doesn't seem to have anything to do with your doc commits, however, we should try to get the test to pass to confirm that.

Newest teuthology run:
http://pulpito.front.sepia.ceph.com/lflores-2021-12-21_07:21:04-rados-wip-primary-balancer-distro-default-smithi/

It's still not done yet, but I'll add a summary of failures when the jobs complete.

@JoshSalomon also we will want to squash commits that relate to the same concept. We can go over this in our meeting. :)

@JoshSalomon how is the commit squashing going? Let me know if you need help

Hello, I wonder in what way do you design the primary balance feature without data movement? It seems that there are no relative code in commits. I think it's almost impossible to achieve this through upmap feature (especially for ec pool). So I'm really curious about your idea.

Hello, I wonder in what way do you design the primary balance feature without data movement? It seems that there are no relative code in commits. I think it's almost impossible to achieve this through upmap feature (especially for ec pool). So I'm really curious about your idea.

I will let @ljflores explain how we change primaries without data movement - in any case this will be in a new PR and not in this one. This PR is just a first in a list of PRs that will improve current upmap balancer and will add primary balancing. (I thought we can make a single PR but I was wrong)
Regarding EC, primary balancer has limited value in EC pools (almost no value when working in fast mode) and currently we plan on implementing primary balancer (actually workload balancer is a better name) for replicated pools only. In the future we may add support for EC pools without the fast mode, but I am not sure it is worth the effort (since the value is limited in most cases). In any case I believe it requires a separate implementation (a different algorithm) than for replicated pools.

Hello, I wonder in what way do you design the primary balance feature without data movement? It seems that there are no relative code in commits. I think it's almost impossible to achieve this through upmap feature (especially for ec pool). So I'm really curious about your idea.

I will let @ljflores explain how we change primaries without data movement - in any case this will be in a new PR and not in this one. This PR is just a first in a list of PRs that will improve current upmap balancer and will add primary balancing. (I thought we can make a single PR but I was wrong) Regarding EC, primary balancer has limited value in EC pools (almost no value when working in fast mode) and currently we plan on implementing primary balancer (actually workload balancer is a better name) for replicated pools only. In the future we may add support for EC pools without the fast mode, but I am not sure it is worth the effort (since the value is limited in most cases). In any case I believe it requires a separate implementation (a different algorithm) than for replicated pools.

Can we please update the PR title to reflect the primary aim of this PR?

Hello, I wonder in what way do you design the primary balance feature without data movement? It seems that there are no relative code in commits. I think it's almost impossible to achieve this through upmap feature (especially for ec pool). So I'm really curious about your idea.

I will let @ljflores explain how we change primaries without data movement - in any case this will be in a new PR and not in this one. This PR is just a first in a list of PRs that will improve current upmap balancer and will add primary balancing. (I thought we can make a single PR but I was wrong) Regarding EC, primary balancer has limited value in EC pools (almost no value when working in fast mode) and currently we plan on implementing primary balancer (actually workload balancer is a better name) for replicated pools only. In the future we may add support for EC pools without the fast mode, but I am not sure it is worth the effort (since the value is limited in most cases). In any case I believe it requires a separate implementation (a different algorithm) than for replicated pools.

Thanks for your reply. I have two questions. As I'm new to ceph these questions may be emm... a little bit ridiculous...
What is the fast mode? Is it a feature ceph currently support? I didn't find a doc or blog describe it. Is it a cache layer?
Why workload balancer has limited value in EC pools? Due to the algorithm design or something else? With a large cluster that may set k+m a big number (probably > 10 or 20), there are only several primary pgs on each osd. In such a case the number of primary pg is significantly unbalanced. Maybe the bottleneck is not disk but network and cpu (calc during recover).

Hello, I wonder in what way do you design the primary balance feature without data movement? It seems that there are no relative code in commits. I think it's almost impossible to achieve this through upmap feature (especially for ec pool). So I'm really curious about your idea.

I will let @ljflores explain how we change primaries without data movement - in any case this will be in a new PR and not in this one. This PR is just a first in a list of PRs that will improve current upmap balancer and will add primary balancing. (I thought we can make a single PR but I was wrong) Regarding EC, primary balancer has limited value in EC pools (almost no value when working in fast mode) and currently we plan on implementing primary balancer (actually workload balancer is a better name) for replicated pools only. In the future we may add support for EC pools without the fast mode, but I am not sure it is worth the effort (since the value is limited in most cases). In any case I believe it requires a separate implementation (a different algorithm) than for replicated pools.

Can we please update the PR title to reflect the primary aim of this PR?

Done - changed the title and the description.

Hello, I wonder in what way do you design the primary balance feature without data movement? It seems that there are no relative code in commits. I think it's almost impossible to achieve this through upmap feature (especially for ec pool). So I'm really curious about your idea.

I will let @ljflores explain how we change primaries without data movement - in any case this will be in a new PR and not in this one. This PR is just a first in a list of PRs that will improve current upmap balancer and will add primary balancing. (I thought we can make a single PR but I was wrong) Regarding EC, primary balancer has limited value in EC pools (almost no value when working in fast mode) and currently we plan on implementing primary balancer (actually workload balancer is a better name) for replicated pools only. In the future we may add support for EC pools without the fast mode, but I am not sure it is worth the effort (since the value is limited in most cases). In any case I believe it requires a separate implementation (a different algorithm) than for replicated pools.

Thanks for your reply. I have two questions. As I'm new to ceph these questions may be emm... a little bit ridiculous... What is the fast mode? Is it a feature ceph currently support? I didn't find a doc or blog describe it. Is it a cache layer? Why workload balancer has limited value in EC pools? Due to the algorithm design or something else? With a large cluster that may set k+m a big number (probably > 10 or 20), there are only several primary pgs on each osd. In such a case the number of primary pg is significantly unbalanced. Maybe the bottleneck is not disk but network and cpu (calc during recover).

For fast mode search fast_read in https://docs.ceph.com/en/latest/rados/operations/pools/
In EC the read request is sent to multiple OSDs not only to the primary (and in fast_mode to all) so it is not that important how the primaries are distributed, but how the Ks in all the PGs are distributed. Therefore the implementation of the workload balancer is different. Additionally in EC you need to move data because balancing means almost by definition changing roles between the K and the N (unlike replica case) so the cost of balancing is much higher. I am not claiming that EC does not need workload balancing, I say it is a different type of balancer, the cost is higher and I think that in many cases the problem is smaller since the number of the OSDS to be balanced (K * #PGs) is higher and the statistical distribution is better. (experiments show that unbalanced primaries impacts the system less when you have more PGs in the pool)

Hello, I wonder in what way do you design the primary balance feature without data movement? It seems that there are no relative code in commits. I think it's almost impossible to achieve this through upmap feature (especially for ec pool). So I'm really curious about your idea.

I will let @ljflores explain how we change primaries without data movement - in any case this will be in a new PR and not in this one. This PR is just a first in a list of PRs that will improve current upmap balancer and will add primary balancing. (I thought we can make a single PR but I was wrong) Regarding EC, primary balancer has limited value in EC pools (almost no value when working in fast mode) and currently we plan on implementing primary balancer (actually workload balancer is a better name) for replicated pools only. In the future we may add support for EC pools without the fast mode, but I am not sure it is worth the effort (since the value is limited in most cases). In any case I believe it requires a separate implementation (a different algorithm) than for replicated pools.

Thanks for your reply. I have two questions. As I'm new to ceph these questions may be emm... a little bit ridiculous... What is the fast mode? Is it a feature ceph currently support? I didn't find a doc or blog describe it. Is it a cache layer? Why workload balancer has limited value in EC pools? Due to the algorithm design or something else? With a large cluster that may set k+m a big number (probably > 10 or 20), there are only several primary pgs on each osd. In such a case the number of primary pg is significantly unbalanced. Maybe the bottleneck is not disk but network and cpu (calc during recover).

For fast mode search fast_read in https://docs.ceph.com/en/latest/rados/operations/pools/ In EC the read request is sent to multiple OSDs not only to the primary (and in fast_mode to all) so it is not that important how the primaries are distributed, but how the Ks in all the PGs are distributed. Therefore the implementation of the workload balancer is different. Additionally in EC you need to move data because balancing means almost by definition changing roles between the K and the N (unlike replica case) so the cost of balancing is much higher. I am not claiming that EC does not need workload balancing, I say it is a different type of balancer, the cost is higher and I think that in many cases the problem is smaller since the number of the OSDS to be balanced (K * #PGs) is higher and the statistical distribution is better. (experiments show that unbalanced primaries impacts the system less when you have more PGs in the pool)

Okay, I get it! Thanks a lot!
Just to say that although sub read requests will be sent to multiple OSDs, the sender (also decoder) of these sub read requests is the primary OSD itself, am I right? The client's read request still only sends to primary if CEPH_OSD_FLAG_BALANCE_READS is not set.

Okay, I get it! Thanks a lot! Just to say that although sub read requests will be sent to multiple OSDs, the sender (also decoder) of these sub read requests is the primary OSD itself, am I right? The client's read request still only sends to primary if CEPH_OSD_FLAG_BALANCE_READS is not set.

You are right

@JoshSalomon @ljflores Let's target the refactor commit for Quincy and hold off on merging
(create a separate PR) the developer documentation for primary balancing until the feature is implemented and ready for backport, to avoid any kind of confusion.

@JoshSalomon @neha-ojha

The commit in this PR now contains all of the refactor changes for Quincy (changes that were previously spread across commits from src/osd/OSDMap.h/cc and src/tools/osdmaptoolcc). #44565 contains the doc commit, which can be merged after the Quincy feature freeze this Friday.

http://pulpito.front.sepia.ceph.com/yuriw-2022-01-12_00:42:29-rados-wip-yuri7-testing-2022-01-11-1457-distro-default-smithi

Unrelated failures tracked in

• https://tracker.ceph.com/issues/53807
• https://tracker.ceph.com/issues/53843

@xiexingguo if you have a chance to review this, it would be great to have a +1.