* 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.