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 add check only concern our self cluster command #9976

Merged
merged 1 commit into from
Feb 7, 2017

Conversation

songbaisen
Copy link

@songbaisen songbaisen commented Jun 28, 2016

mon: PGMonitor add check only concern our self cluster command

we only concern the command send to our self cluster

Signed-off-by:song baisen song.baisen@zte.com.cn

@ghost
Copy link

ghost commented Nov 21, 2016

jenkins test this please (no logs)

@ghost ghost added core bug-fix labels Nov 23, 2016
@ghost
Copy link

ghost commented Nov 23, 2016

please explain the bug you are trying to fix in the commit message. Note that I do not understand the meaning of the title of the commit message: it needs to be reworded for clarity.

Copy link
Member

@jecluis jecluis left a comment

Choose a reason for hiding this comment

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

I understand what you are trying to achieve with this patch: making sure the PGMonitor will only handle messages that target this cluster, especially in environments where there may be other ceph clusters around.

However, I am curious: how does one incur in such an issue? Unless the admin is somehow sharing ceph.conf across multiple clusters, or allowing clients from another cluster to contact this cluster, you should never hit a condition where this happens. If it does, please do let us know and how to reproduce.

In any case, it would be nice if you were to propose a few more patches for all the consumers of PaxosServiceMessage's that have an fsid, not just for those from the PGMonitor.

@@ -699,6 +699,11 @@ bool PGMonitor::preprocess_pg_stats(MonOpRequestRef op)
return true;
}

if (stats->fsid != mon->monmap->fsid) {
dout(0) << "preprocess_pg_stats on fsid " << stats->fsid << " != " << mon->monmap->fsid << dendl;
Copy link
Member

Choose a reason for hiding this comment

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

it would be nice to also print out the message that triggered this. Say,

dout(0) << __func__ << " drop message on fsid " << stats->fsid << " != " << mon->monmap->fsid << " for " << *stats << dendl;

or something. Same for the other parts where you add this check.

Copy link
Author

Choose a reason for hiding this comment

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

@jecluis Thank you. I will change this later.

 we only concern the command send to our self cluster

 Signed-off-by:song baisen <song.baisen@zte.com.cn>
@songbaisen
Copy link
Author

@jecluis Done.

@yuriw
Copy link
Contributor

yuriw commented Feb 7, 2017

@songbaisen I see no bug number associated with this PR

@liewegas liewegas changed the title mon: PGMonitor add check only concern our self cluster command mon: PGMonitor add check only concern our self cluster command Feb 7, 2017
@liewegas liewegas merged commit eafbe44 into ceph:master Feb 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants