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/PGMonitor: batch filter pg states; add sanity check #9394

Merged
merged 5 commits into from Jul 4, 2016

Conversation

xiexingguo
Copy link
Member

No description provided.

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
@xiexingguo
Copy link
Member Author

@tchaikov Some mon-related minor fixes, could you kindly help to review? Thanks:-)

@tchaikov tchaikov self-assigned this May 31, 2016
return;
}

MStatfsReply *reply;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could move this line down to where reply is used for the first time.

propose = true;
assert(pg_map.last_osdmap_epoch < epoch);
pending_inc.osdmap_epoch = epoch;
(void)map_pg_creates();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check_osd_map() is the only caller of these functions. maybe we can change them so that they return void instead?

@xiexingguo
Copy link
Member Author

@tchaikov Done and thanks.

@@ -1754,24 +1754,17 @@ void PGMap::generate_test_instances(list<PGMap*>& o)
}
}

void PGMap::get_filtered_pg_stats(const string& state, int64_t poolid, int64_t osdid,
void PGMap::get_filtered_pg_stats(__u32 state, int64_t poolid, int64_t osdid,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

__u32 is a linux specific type, maybe we can try to use uint32_t instead? i see the underlying type of pg_state_t::state is __u32. but we can be more portable by using these ANSI types.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. And @tchaikov , I see you are busy with cmake, thank you for your time:-)

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
We'll assure pg_map.last_osdmap_epoch is smaller than osdmap's epoch
at the function entry, so we are doomed to update pending_inc.osdmap_epoch,
which means the "propose" variables will be always set to true.

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
We use "backfill_wait" everywhere except this one.

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
@@ -824,7 +824,7 @@ std::string pg_state_string(int state)
oss << "repair+";
if ((state & PG_STATE_BACKFILL_WAIT) &&
!(state &PG_STATE_BACKFILL))
oss << "wait_backfill+";
oss << "backfill_wait+";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could update the doc/dev/osd_internals/recovery_reservation.rst accordingly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Thank you for your reminder.


/**
* recalculate creating pg mappings
*
* @return true if we updated pending_inc
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you update the comment accordingly?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I don't understand what do you mean. It does not return anything now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, sorry, i thought you missed this.

@tchaikov
Copy link
Contributor

tchaikov commented Jun 6, 2016

lgtm

@tchaikov tchaikov merged commit 8b433e1 into ceph:master Jul 4, 2016
@xiexingguo
Copy link
Member Author

@tchaikov Thanks for the merge.

@xiexingguo xiexingguo deleted the xxg-wip-batchpgstates branch November 16, 2016 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants