* mon: forwarding user commands
@ 2015-01-18 9:28 Mykola Golub
2015-01-18 18:33 ` Sage Weil
0 siblings, 1 reply; 7+ messages in thread
From: Mykola Golub @ 2015-01-18 9:28 UTC (permalink / raw)
To: ceph-devel
Hi Ceph,
Right now, for not a monitor leader, if a received command is not
supported locally, but is supported by the leader, it is forwarded to
the leader.
For the recently added "ceph tell mon.x version" it gives a confusing
behavior: if the mon.x is not a leader and does not support "version"
command yet, but the leader does, the user will receive the version of
the leader, and can't be actually sure about a non leader version.
I have a patch that fixes this by checking if the received "version"
command is forwarded and returning the error in this case:
https://github.com/trociny/ceph/commit/98f835357e378b1c5f05b32ba90a8b8537ba1ad8
But may be we need a more general solution? We might face a similar
issue in future, when adding a new command, which is not expected to
be forwarded to the leader (like injectargs).
--
Mykola Golub
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: mon: forwarding user commands
2015-01-18 9:28 mon: forwarding user commands Mykola Golub
@ 2015-01-18 18:33 ` Sage Weil
2015-01-19 7:02 ` Mykola Golub
0 siblings, 1 reply; 7+ messages in thread
From: Sage Weil @ 2015-01-18 18:33 UTC (permalink / raw)
To: Mykola Golub; +Cc: ceph-devel
On Sun, 18 Jan 2015, Mykola Golub wrote:
> Hi Ceph,
>
> Right now, for not a monitor leader, if a received command is not
> supported locally, but is supported by the leader, it is forwarded to
> the leader.
>
> For the recently added "ceph tell mon.x version" it gives a confusing
> behavior: if the mon.x is not a leader and does not support "version"
> command yet, but the leader does, the user will receive the version of
> the leader, and can't be actually sure about a non leader version.
>
> I have a patch that fixes this by checking if the received "version"
> command is forwarded and returning the error in this case:
>
> https://github.com/trociny/ceph/commit/98f835357e378b1c5f05b32ba90a8b8537ba1ad8
>
> But may be we need a more general solution? We might face a similar
> issue in future, when adding a new command, which is not expected to
> be forwarded to the leader (like injectargs).
Yeah.. that works for 'version' but not any other 'tell mon ...' command.
I think the way to do it properly going forward is to add a flags field to
MMonCommand, with a flag NOFORWARD.
That won't help in the version case, so we can combine it with your
patch...
sage
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: mon: forwarding user commands
2015-01-18 18:33 ` Sage Weil
@ 2015-01-19 7:02 ` Mykola Golub
2015-01-19 14:57 ` Gregory Farnum
0 siblings, 1 reply; 7+ messages in thread
From: Mykola Golub @ 2015-01-19 7:02 UTC (permalink / raw)
To: Sage Weil; +Cc: ceph-devel
On Sun, Jan 18, 2015 at 10:33:05AM -0800, Sage Weil wrote:
> On Sun, 18 Jan 2015, Mykola Golub wrote:
> > Hi Ceph,
> >
> > Right now, for not a monitor leader, if a received command is not
> > supported locally, but is supported by the leader, it is forwarded to
> > the leader.
> >
> > For the recently added "ceph tell mon.x version" it gives a confusing
> > behavior: if the mon.x is not a leader and does not support "version"
> > command yet, but the leader does, the user will receive the version of
> > the leader, and can't be actually sure about a non leader version.
> >
> > I have a patch that fixes this by checking if the received "version"
> > command is forwarded and returning the error in this case:
> >
> > https://github.com/trociny/ceph/commit/98f835357e378b1c5f05b32ba90a8b8537ba1ad8
> >
> > But may be we need a more general solution? We might face a similar
> > issue in future, when adding a new command, which is not expected to
> > be forwarded to the leader (like injectargs).
>
> Yeah.. that works for 'version' but not any other 'tell mon ...' command.
>
> I think the way to do it properly going forward is to add a flags field to
> MMonCommand, with a flag NOFORWARD.
>
> That won't help in the version case, so we can combine it with your
> patch...
Do you mean
1) statically adding the NOFORWARD flag to commands definitions in
MonCommands.h (and to the struct MonCommand in Monitor.h), and then
checking for the flag in Monitor::handle_command(), before forwarding
to the leader, or
2) extending librados API, RadosClient::mon_command(), so a user could
set the flag for any sent command she don't want to be forwarded, and
using this flag for 'ceph tell mon.x ...' commands?
I think you mean (2), but just to be sure, before coding this...
--
Mykola Golub
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: mon: forwarding user commands
2015-01-19 7:02 ` Mykola Golub
@ 2015-01-19 14:57 ` Gregory Farnum
2015-01-19 15:17 ` Sage Weil
0 siblings, 1 reply; 7+ messages in thread
From: Gregory Farnum @ 2015-01-19 14:57 UTC (permalink / raw)
To: Mykola Golub; +Cc: Sage Weil, ceph-devel
On Sun, Jan 18, 2015 at 11:02 PM, Mykola Golub <mgolub@mirantis.com> wrote:
> On Sun, Jan 18, 2015 at 10:33:05AM -0800, Sage Weil wrote:
>> On Sun, 18 Jan 2015, Mykola Golub wrote:
>> > Hi Ceph,
>> >
>> > Right now, for not a monitor leader, if a received command is not
>> > supported locally, but is supported by the leader, it is forwarded to
>> > the leader.
>> >
>> > For the recently added "ceph tell mon.x version" it gives a confusing
>> > behavior: if the mon.x is not a leader and does not support "version"
>> > command yet, but the leader does, the user will receive the version of
>> > the leader, and can't be actually sure about a non leader version.
>> >
>> > I have a patch that fixes this by checking if the received "version"
>> > command is forwarded and returning the error in this case:
>> >
>> > https://github.com/trociny/ceph/commit/98f835357e378b1c5f05b32ba90a8b8537ba1ad8
>> >
>> > But may be we need a more general solution? We might face a similar
>> > issue in future, when adding a new command, which is not expected to
>> > be forwarded to the leader (like injectargs).
>>
>> Yeah.. that works for 'version' but not any other 'tell mon ...' command.
>>
>> I think the way to do it properly going forward is to add a flags field to
>> MMonCommand, with a flag NOFORWARD.
>>
>> That won't help in the version case, so we can combine it with your
>> patch...
>
> Do you mean
>
> 1) statically adding the NOFORWARD flag to commands definitions in
> MonCommands.h (and to the struct MonCommand in Monitor.h), and then
> checking for the flag in Monitor::handle_command(), before forwarding
> to the leader, or
>
> 2) extending librados API, RadosClient::mon_command(), so a user could
> set the flag for any sent command she don't want to be forwarded, and
> using this flag for 'ceph tell mon.x ...' commands?
>
> I think you mean (2), but just to be sure, before coding this...
I think it would be (1), but you'd have to check it both before
forwarding and after receipt of a forward. That way we can determine
whenever we add commands if it should be forwarded or not, instead of
letting users get it wrong?
-Greg
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: mon: forwarding user commands
2015-01-19 14:57 ` Gregory Farnum
@ 2015-01-19 15:17 ` Sage Weil
2015-01-27 8:31 ` Mykola Golub
0 siblings, 1 reply; 7+ messages in thread
From: Sage Weil @ 2015-01-19 15:17 UTC (permalink / raw)
To: Gregory Farnum; +Cc: Mykola Golub, ceph-devel
On Mon, 19 Jan 2015, Gregory Farnum wrote:
> On Sun, Jan 18, 2015 at 11:02 PM, Mykola Golub <mgolub@mirantis.com> wrote:
> > On Sun, Jan 18, 2015 at 10:33:05AM -0800, Sage Weil wrote:
> >> On Sun, 18 Jan 2015, Mykola Golub wrote:
> >> > Hi Ceph,
> >> >
> >> > Right now, for not a monitor leader, if a received command is not
> >> > supported locally, but is supported by the leader, it is forwarded to
> >> > the leader.
> >> >
> >> > For the recently added "ceph tell mon.x version" it gives a confusing
> >> > behavior: if the mon.x is not a leader and does not support "version"
> >> > command yet, but the leader does, the user will receive the version of
> >> > the leader, and can't be actually sure about a non leader version.
> >> >
> >> > I have a patch that fixes this by checking if the received "version"
> >> > command is forwarded and returning the error in this case:
> >> >
> >> > https://github.com/trociny/ceph/commit/98f835357e378b1c5f05b32ba90a8b8537ba1ad8
> >> >
> >> > But may be we need a more general solution? We might face a similar
> >> > issue in future, when adding a new command, which is not expected to
> >> > be forwarded to the leader (like injectargs).
> >>
> >> Yeah.. that works for 'version' but not any other 'tell mon ...' command.
> >>
> >> I think the way to do it properly going forward is to add a flags field to
> >> MMonCommand, with a flag NOFORWARD.
> >>
> >> That won't help in the version case, so we can combine it with your
> >> patch...
> >
> > Do you mean
> >
> > 1) statically adding the NOFORWARD flag to commands definitions in
> > MonCommands.h (and to the struct MonCommand in Monitor.h), and then
> > checking for the flag in Monitor::handle_command(), before forwarding
> > to the leader, or
Hmm, this is a cleaner way to approach your change that specifically
applies to the new 'version' command.
> > 2) extending librados API, RadosClient::mon_command(), so a user could
> > set the flag for any sent command she don't want to be forwarded, and
> > using this flag for 'ceph tell mon.x ...' commands?
..but I think we also want this, so that *any* 'ceph tell mon.whatever
...' command can be flagged to not get forwarded.
sage
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: mon: forwarding user commands
2015-01-19 15:17 ` Sage Weil
@ 2015-01-27 8:31 ` Mykola Golub
2015-01-27 16:20 ` Sage Weil
0 siblings, 1 reply; 7+ messages in thread
From: Mykola Golub @ 2015-01-27 8:31 UTC (permalink / raw)
To: Sage Weil; +Cc: Gregory Farnum, ceph-devel
On Mon, Jan 19, 2015 at 07:17:58AM -0800, Sage Weil wrote:
> On Mon, 19 Jan 2015, Gregory Farnum wrote:
> > On Sun, Jan 18, 2015 at 11:02 PM, Mykola Golub <mgolub@mirantis.com> wrote:
> > > On Sun, Jan 18, 2015 at 10:33:05AM -0800, Sage Weil wrote:
> > >> On Sun, 18 Jan 2015, Mykola Golub wrote:
> > >> > Hi Ceph,
> > >> >
> > >> > Right now, for not a monitor leader, if a received command is not
> > >> > supported locally, but is supported by the leader, it is forwarded to
> > >> > the leader.
> > >> >
> > >> > For the recently added "ceph tell mon.x version" it gives a confusing
> > >> > behavior: if the mon.x is not a leader and does not support "version"
> > >> > command yet, but the leader does, the user will receive the version of
> > >> > the leader, and can't be actually sure about a non leader version.
> > >> >
> > >> > I have a patch that fixes this by checking if the received "version"
> > >> > command is forwarded and returning the error in this case:
> > >> >
> > >> > https://github.com/trociny/ceph/commit/98f835357e378b1c5f05b32ba90a8b8537ba1ad8
> > >> >
> > >> > But may be we need a more general solution? We might face a similar
> > >> > issue in future, when adding a new command, which is not expected to
> > >> > be forwarded to the leader (like injectargs).
> > >>
> > >> Yeah.. that works for 'version' but not any other 'tell mon ...' command.
> > >>
> > >> I think the way to do it properly going forward is to add a flags field to
> > >> MMonCommand, with a flag NOFORWARD.
> > >>
> > >> That won't help in the version case, so we can combine it with your
> > >> patch...
> > >
> > > Do you mean
> > >
> > > 1) statically adding the NOFORWARD flag to commands definitions in
> > > MonCommands.h (and to the struct MonCommand in Monitor.h), and then
> > > checking for the flag in Monitor::handle_command(), before forwarding
> > > to the leader, or
>
> Hmm, this is a cleaner way to approach your change that specifically
> applies to the new 'version' command.
>
> > > 2) extending librados API, RadosClient::mon_command(), so a user could
> > > set the flag for any sent command she don't want to be forwarded, and
> > > using this flag for 'ceph tell mon.x ...' commands?
>
> ..but I think we also want this, so that *any* 'ceph tell mon.whatever
> ...' command can be flagged to not get forwarded.
I have created a pull request that implements both:
https://github.com/ceph/ceph/pull/3496
The (2) looks not very useful for now -- I suppose currently the only
command that suffers from the describe problem is "version" and it has
"noforward" hard coded. This might change in future though.
--
Mykola Golub
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: mon: forwarding user commands
2015-01-27 8:31 ` Mykola Golub
@ 2015-01-27 16:20 ` Sage Weil
0 siblings, 0 replies; 7+ messages in thread
From: Sage Weil @ 2015-01-27 16:20 UTC (permalink / raw)
To: Mykola Golub; +Cc: Gregory Farnum, ceph-devel
On Tue, 27 Jan 2015, Mykola Golub wrote:
> On Mon, Jan 19, 2015 at 07:17:58AM -0800, Sage Weil wrote:
> > On Mon, 19 Jan 2015, Gregory Farnum wrote:
> > > On Sun, Jan 18, 2015 at 11:02 PM, Mykola Golub <mgolub@mirantis.com> wrote:
> > > > On Sun, Jan 18, 2015 at 10:33:05AM -0800, Sage Weil wrote:
> > > >> On Sun, 18 Jan 2015, Mykola Golub wrote:
> > > >> > Hi Ceph,
> > > >> >
> > > >> > Right now, for not a monitor leader, if a received command is not
> > > >> > supported locally, but is supported by the leader, it is forwarded to
> > > >> > the leader.
> > > >> >
> > > >> > For the recently added "ceph tell mon.x version" it gives a confusing
> > > >> > behavior: if the mon.x is not a leader and does not support "version"
> > > >> > command yet, but the leader does, the user will receive the version of
> > > >> > the leader, and can't be actually sure about a non leader version.
> > > >> >
> > > >> > I have a patch that fixes this by checking if the received "version"
> > > >> > command is forwarded and returning the error in this case:
> > > >> >
> > > >> > https://github.com/trociny/ceph/commit/98f835357e378b1c5f05b32ba90a8b8537ba1ad8
> > > >> >
> > > >> > But may be we need a more general solution? We might face a similar
> > > >> > issue in future, when adding a new command, which is not expected to
> > > >> > be forwarded to the leader (like injectargs).
> > > >>
> > > >> Yeah.. that works for 'version' but not any other 'tell mon ...' command.
> > > >>
> > > >> I think the way to do it properly going forward is to add a flags field to
> > > >> MMonCommand, with a flag NOFORWARD.
> > > >>
> > > >> That won't help in the version case, so we can combine it with your
> > > >> patch...
> > > >
> > > > Do you mean
> > > >
> > > > 1) statically adding the NOFORWARD flag to commands definitions in
> > > > MonCommands.h (and to the struct MonCommand in Monitor.h), and then
> > > > checking for the flag in Monitor::handle_command(), before forwarding
> > > > to the leader, or
> >
> > Hmm, this is a cleaner way to approach your change that specifically
> > applies to the new 'version' command.
> >
> > > > 2) extending librados API, RadosClient::mon_command(), so a user could
> > > > set the flag for any sent command she don't want to be forwarded, and
> > > > using this flag for 'ceph tell mon.x ...' commands?
> >
> > ..but I think we also want this, so that *any* 'ceph tell mon.whatever
> > ...' command can be flagged to not get forwarded.
>
> I have created a pull request that implements both:
>
> https://github.com/ceph/ceph/pull/3496
>
> The (2) looks not very useful for now -- I suppose currently the only
> command that suffers from the describe problem is "version" and it has
> "noforward" hard coded. This might change in future though.
Other commands where this would be useful are 'injectargs' and
'mon_status', 'compact', and 'heap'.
Made a few other comments on the pull request!
Thanks-
sage
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-01-27 16:20 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-18 9:28 mon: forwarding user commands Mykola Golub
2015-01-18 18:33 ` Sage Weil
2015-01-19 7:02 ` Mykola Golub
2015-01-19 14:57 ` Gregory Farnum
2015-01-19 15:17 ` Sage Weil
2015-01-27 8:31 ` Mykola Golub
2015-01-27 16:20 ` Sage Weil
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.