Skip to content
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/PGMap: add up_primary pg number field for pg-dump cmd #13451

Merged
merged 1 commit into from May 2, 2017

Conversation

Projects
None yet
4 participants
@xiexingguo
Copy link
Member

xiexingguo commented Feb 16, 2017

Check db5989b for a better aligned demo result output.

Signed-off-by: xie xingguo xie.xingguo@zte.com.cn

@xiexingguo xiexingguo force-pushed the xiexingguo:wip-add-up-primary-sum branch from bc3054e to db5989b Feb 16, 2017

@xiexingguo xiexingguo added the mon label Feb 16, 2017

@xiexingguo xiexingguo requested a review from tchaikov Feb 16, 2017

@xiexingguo

This comment has been minimized.

Copy link
Member Author

xiexingguo commented Feb 20, 2017

@tchaikov Ping

@xiexingguo

This comment has been minimized.

Copy link
Member Author

xiexingguo commented Mar 31, 2017

@tchaikov BTW, can you check this one too? Thanks.

@tchaikov
Copy link
Contributor

tchaikov left a comment

i am just not sure if we want "UP_PRIMARY_PG_SUM" in the output from user's perspective. it's not obvious to me why we need it. maybe you can elaborate it a little bit in your commit comment?

@liewegas and @jecluis what do you think?

int get_num_up_primary_pg_by_osd(int osd) const {
assert(osd >= 0);
int num = 0;
for (ceph::unordered_map<pg_t, pg_stat_t>::const_iterator i =

This comment has been minimized.

Copy link
@tchaikov

tchaikov Mar 31, 2017

Contributor

could use range based for loop.

@tchaikov tchaikov removed their assignment Mar 31, 2017

@xiexingguo

This comment has been minimized.

Copy link
Member Author

xiexingguo commented Mar 31, 2017

i am just not sure if we want "UP_PRIMARY_PG_SUM" in the output from user's perspective. it's not obvious to me why we need it. maybe you can elaborate it a little bit in your commit comment?

We want the Primary distribution on each osd for performance estimation, especially for pure read.
E.g., we'd like to confirm the load banlance variations are not caused by unevenly distributed Primarys during osds...

@@ -956,6 +956,7 @@ void PGMap::dump_osd_stats(ostream& ss) const
tab.define_column("TOTAL", TextTable::LEFT, TextTable::RIGHT);
tab.define_column("HB_PEERS", TextTable::LEFT, TextTable::RIGHT);
tab.define_column("PG_SUM", TextTable::LEFT, TextTable::RIGHT);
tab.define_column("UP_PRIMARY_PG_SUM", TextTable::LEFT, TextTable::RIGHT);

This comment has been minimized.

Copy link
@liewegas

liewegas Apr 27, 2017

Member

PRIMARY_PG_SUM is sufficient

@xiexingguo xiexingguo force-pushed the xiexingguo:wip-add-up-primary-sum branch from db5989b to 3414fb7 Apr 28, 2017

@xiexingguo

This comment has been minimized.

Copy link
Member Author

xiexingguo commented Apr 28, 2017

for (const auto &p : pg_stat) {
if (p.second.up_primary == osd)
++num;
}

This comment has been minimized.

Copy link
@liewegas

liewegas Apr 28, 2017

Member

On second thought, we should probably not calculate this each time. For a large number of PGs this will be extremely slow. Let's make a map<int,int> num_primary_pg_by_osd PGMap member to go along with pg_by_osd? It can just be a count probably (not a full set<>).

This comment has been minimized.

Copy link
@xiexingguo

xiexingguo Apr 28, 2017

Author Member

Sure. Updated.

@xiexingguo xiexingguo force-pushed the xiexingguo:wip-add-up-primary-sum branch from 3414fb7 to 781bd5f Apr 28, 2017

mon/PGMap: add up_primary pg number field for "pg dump osds" cmd
As follows:
[root@tecs131 ~]# ceph pg dump osds
OSD_STAT USED  AVAIL  TOTAL  HB_PEERS                                                     PG_SUM UP_PRIMARY_PG_SUM
9         118G   822G   941G    [0,1,2,4,5,7,8,10,11,12,13,15,16,17,20,21,22,23,25,27,29]    299               101
8         122G   819G   941G     [3,4,6,7,9,10,11,14,16,18,19,20,22,23,24,25,26,27,28,29]    311               107
7         124G   817G   941G        [0,1,2,3,5,6,8,9,10,12,13,14,15,17,18,19,21,24,26,28]    314               102
6         119G   821G   941G       [0,1,2,4,5,7,8,11,12,13,15,16,17,20,21,22,23,25,27,29]    311               103
5         127G   813G   941G     [3,4,6,7,9,10,11,14,16,18,19,20,22,23,24,25,26,27,28,29]    333               113
4         118G   822G   941G        [0,1,2,3,5,6,8,9,10,12,13,14,15,17,18,19,21,24,26,28]    304               127
3         124G   816G   941G       [0,1,2,4,5,7,8,11,12,13,15,16,17,20,21,22,23,25,27,29]    324               106
29        111G   829G   941G        [0,1,2,3,5,6,8,9,10,12,13,14,15,17,18,19,21,24,26,28]    282               100
17        115G   825G   941G     [3,4,6,7,9,10,11,14,16,18,19,20,22,23,24,25,26,27,28,29]    291               101
16        122G   819G   941G        [0,1,2,3,5,6,8,9,10,12,13,14,15,17,18,19,21,24,26,28]    321               104
15        105G   836G   941G     [3,4,6,7,9,10,11,14,16,18,19,20,22,23,24,25,26,27,28,29]    273                77
14        122G   819G   941G       [0,1,2,4,5,7,8,11,12,13,15,16,17,20,21,22,23,25,27,29]    316               120
13        120G   821G   941G  [3,4,6,7,9,10,11,12,14,16,18,19,20,22,23,24,25,26,27,28,29]    305                98
10        111G   829G   941G     [0,1,2,4,5,7,8,9,11,12,13,15,16,17,20,21,22,23,25,27,29]    272                67
12        126G   814G   941G  [3,4,6,7,9,10,11,13,14,16,18,19,20,22,23,24,25,26,27,28,29]    321                97
1         127G   813G   941G [0,2,3,4,6,7,9,10,11,14,16,18,19,20,22,23,24,25,26,27,28,29]    326               104

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>

@yuriw yuriw merged commit 279050e into ceph:master May 2, 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

@xiexingguo xiexingguo deleted the xiexingguo:wip-add-up-primary-sum branch May 3, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.