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/MonClient: random all ranks then pick first_n #13479

Merged
merged 1 commit into from Feb 18, 2017

Conversation

Projects
None yet
3 participants
@LiumxNL
Contributor

LiumxNL commented Feb 17, 2017

Signed-off-by: Mingxin Liu mingxin@xsky.com

@LiumxNL

This comment has been minimized.

Contributor

LiumxNL commented Feb 17, 2017

@tchaikov please review, thanks!

ranks[i] = i;
}
std::random_device rd;
std::mt19937 rng(rd());
std::shuffle(ranks.begin(), ranks.end(), rng);
unsigned n = cct->_conf->mon_client_hunt_parallel;
if (n) {

This comment has been minimized.

@tchaikov

tchaikov Feb 17, 2017

Contributor

could you keep the original block of if (n == 0 || n > monmap.size()), IMHO, it's more explicit and easier to digest

}
vector<unsigned> ranks(n);
for (unsigned i = 0; i < n; i++) {
unsigned m = monmap.size();

This comment has been minimized.

@tchaikov

tchaikov Feb 17, 2017

Contributor

nit, could have a better name than m, like num_mons, also could make this const.

@liewegas

This comment has been minimized.

Member

liewegas commented Feb 17, 2017

I'm hitting this bug from rados suite: http://pulpito.ceph.com/sage-2017-02-17_15:21:53-rados:multimon-wip-sage-testing---basic-smithi

Including this in my test branch!

@tchaikov

lgtm, modulo the nits.

mon/MonClient: random all ranks then pick first_n
Signed-off-by: Mingxin Liu <mingxin@xsky.com>
@LiumxNL

This comment has been minimized.

Contributor

LiumxNL commented Feb 18, 2017

@tchaikov updated.

@liewegas liewegas merged commit 9762dbf into ceph:master Feb 18, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment