qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ken Giusti <kgiu...@redhat.com>
Subject Re: svn commit: r1052086 - in /qpid/trunk/qpid: extras/qmf/src/py/qmf/console.py tools/src/py/qpid-config tools/src/py/qpid-printevents tools/src/py/qpid-stat tools/src/py/qpid-tool
Date Wed, 05 Jan 2011 15:50:04 GMT
Hi Gordon,

Jonathan discussed some of these issues with me before working on this change.  Some of the
changes here are due to the current state of the underlying python qmf implementation (console.py)
used by these command line tools.

Couple of things to keep in mind re: the qmf implementation used by these tools:

console.py provides two usage models, which are reflected by the behavior of command line
tools.  console.py can be used for a synchronous command-response tool (like qpid-route, qpid-config),
or it can be used as a passive monitor (qpid-tool, qpid-printevents).  

Connection failures when using qmf in the synchronous mode are reported back immediately to
the application, which deals with them appropriately. No issue there.

However, there's an ambiguity with the passive approach: what should happen when the connection
to the broker fails?   The nature of the qpid-printevents/qpid-tool allows them to be run
in lieu of a broker being available.  A user can run the tool indefinitely, allowing brokers
to come and go over time.

Having a connection attempt fail due to broker not being available, or going away, isn't really
a hard error in this model.  qmf merely notifies the application that the broker has disconnected,
and keeps retrying the connection.  However, that's a problem when the failure may involve
a misconfiguration - say invalid credentials - as qmf will retry indefinitely until a human
intervenes to fix the misconfiguration.

Part of what these changes attempt to alert the user to a hard connection failure rather than
a transient broker availability issue when qmf is used in passive mode.

Which brings us to logging in qmf:  There currently isn't any logging, which makes it hard
for a user to determine the underlying cause of a failure to connect with a broker.  Part
of these changes are an attempt to provide more visibility in this area.

-K

----- Original Message -----
> On 01/04/2011 10:26 PM, Jonathan Robie wrote:
> > On 01/04/2011 03:47 PM, Gordon Sim wrote:
> >
> >>> URL: http://svn.apache.org/viewvc?rev=1052086&view=rev
> >>> Log:
> >>> Added logging to QMF console connections.
> >>>
> >>> Warning if a broker can not be found, error if SASL authentication
> >>> fails or other connection errors when connecting to an existing
> >>> broker. Default log level is ERROR.
> >>
> >> What is the usual default for the log level (i.e. what is the
> >> default
> >> for other uses of the log module)? It seems a little odd to set the
> >> log
> >> level to error on most tools, perhaps that's just me...
> >
> > It's easy to change the default.
> 
> From the commit it looks like each tool sets the 'default'
> separately(?)
> 
> > I'm not sure what the default log level should be for QMF console
> > applications - what would you suggest?
> 
> What is the usual default (i.e. for other users of the qpid.log
> module)?
> What other log statements are there for 'qpid.qmf' (I couldn't find
> any)?
> 
> >>> qpid-printevents allows the log level to be set.
> >>
> >> What about the other tools? Is it not useful for them to also be
> >> able to
> >> set the logging level? (As the topic of consistency came up
> >> recently,
> >> should the -v/--verbose option in qpid-route by rationalised with
> >> this?)
> >
> > It would be easy to add the log level option to the other tools.
> >
> > When I raised the topic of consistency in options recently, you
> > suggested we should start by determining which tools we need and
> > designing the right tool set. I'd like to avoid making changes that
> > are
> > not backwards compatible until we actually do that redesign.
> >
> > Ideally, I'd probably just use logging levels in qpid-route. But for
> > now, I'm reluctant to eliminate -v if there are scripts that might
> > use it.
> 
> That is certainly an important consideration and I agree that we would
> not want to start eliminating options without due consideration. I
> think
> similar consideration before adding any options - particularly any
> that
> may increase the feeling of inconsistency - is also important.
> 
> I guess the real issue is that I don't fully understand the particular
> choices made in this commit or what motivated them.
> 
> What was your thinking around a lower log level (i.e. warn rather than
> error) for the case where 'a broker can not be found'? And what
> exactly
> do you mean by that? I seem to only get the warning level used where
> the
> hostname can't even be resolved; connection refused and no-route to
> host
> are both error level statements. This seems pretty odd to me.
> 
> The changes to qpid-printevents seem to be addressing a different
> issue,
> in fact two further separate issues.
> 
> The first is a new option that as I understand it disables the
> built-in
> connection retry (and in an ideal world would have been made via a
> distinct commit). While not opposed to the change in principle, it
> would
> be good to be clear on the motivation.
> 
> The second is an option to alter the logging level for the qpid.qmf
> logger. What's the motivation for this? Are there any log statements
> for
> that logger other than the one added by this commit? The tool already
> has some sort of logging style output that this option doesn't affect.
> 
> I suspect this commit was motivated by the fact that qpid-printevents
> appears just to hang if it can't establish the connection to begin
> with
> (if it is disconnected, that is already logged, as is subsequent
> reconnection).
> 
> >>> It also allows the user to specify that a connection is required,
> >>> in
> >>> which case it terminates if a connection can not be established.
> >>
> >> I'm not mad on the name of this new option (i.e.
> >> require-connection). It
> >> seems like allow-reconnect or exit-on-failure would communicate a
> >> little
> >> more on the purpose of the option.
> >
> > It's more like exit-on-connection-failure, but that's rather
> > verbose.
> >
> > To me, exit-on-failure fails to communicate that this has to do with
> > connections,
> 
> Alternatives in that case might be
> exit-on-disconnect/exit-if-not-connected/exit-unless-connected...
> however it actually also affects handling of other types of error,
> e.g.
> acl failures, so I'm not sure that exit-on-failure is even wrong.
> 
> > and allow-reconnect is not the same as exiting if a
> > connection can not be established.
> 
> The allow-reconnect option would indicate that it is ok to try to
> periodically reconnect; if not then the only choice would be to exit.
> Reversing the sense, disable-reconnect would also be reasonable in my
> view, and in fact I think that would probably be my preference.
> 
> > I'm not convinced that either of
> > these really does a better job of explaining the purpose.
> 
> I'm fairly convinced that I don't like require-connection.
> 
> The tool *always* requires a connection else it cannot do anything.
> The
> question is whether it is allowed to remain running in the
> disconnected
> state while periodically trying to re-establish a connection or
> whether
> it should immediately exit.
> 
> ---------------------------------------------------------------------
> Apache Qpid - AMQP Messaging Implementation
> Project: http://qpid.apache.org
> Use/Interact: mailto:dev-subscribe@qpid.apache.org

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org


Mime
View raw message