Mailing-List: contact users-help@qpid.apache.org; run by ezmlm
Precedence: bulk
Reply-To: users@qpid.apache.org
Received-SPF: pass (nike.apache.org: domain of gsim@redhat.com designates
 209.132.183.28 as permitted sender)
Message-ID: <53E8E587.209@redhat.com>
Date: Mon, 11 Aug 2014 16:47:19 +0100
From: Gordon Sim <gsim@redhat.com>
Organization: Red Hat UK Ltd, Registered in England and Wales under Company
 Registration No. 3798903, Directors: Michael Cunningham (USA), Matt Parsons
 (USA), Charlie Peters (USA), Michael O'Neill (Ireland)
User-Agent: Mozilla/5.0 (X11; Linux x86_64;
 rv:17.0) Gecko/20130625 Thunderbird/17.0.7
MIME-Version: 1.0
To: users@qpid.apache.org
Subject: Re: 'client' APIs again
References: <53DA1FFC.5040306@redhat.com>
 <1407765677.30949.33.camel@localhost>
In-Reply-To: <1407765677.30949.33.camel@localhost>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

Thanks very much for the comments, Alan, some responses inline...

On 08/11/2014 03:01 PM, Alan Conway wrote:
> Naming: I hate things called *_tools, *_utils etc. We should find a more
> descriptive name. Not foo or fred or whatchymacallit or dingdong or
> doofer either. Ideally these classes might all just end up in the proton
> package, or maybe as a subpackage like proton.events or proton.reactor.

I personally think they should (generally) be kept distinct from the 
current proton package to make it clear that they are additive and 
optional. A subpackage would be ok, I just didn't want to mix this with 
dependent edits to the proton tree itself to begin with. It may even be 
that not all the additional pieces belong in the same package.

> Runtime is a bit vague as a name as well though I haven't got a better
> idea.

Yes, this was unpopular with other I spoke to also. I reverted to my 
original name - Container - which isn't ideal either. Any suggestions 
from other gratefully considered.

>
> Get rid of Runtime.DEFAULT. Developers who want a global instance should
> make their own and be responsible for its scope:
>
>      DEFAULT = Runtime()
>
> We definitely should not provide a global in the library (I have
> suffered horribly in the past from this mistake. Seems harmless at first
> but creates nightmarish startup & shutdown ordering issues.)

Yes, I was coming to that conclusion myself...

> I'd modify the Runtime constructor a little:
>
> class Runtime:
>      def __init__(self, handlers=None):
>          if handlers is None: handlers = [FlowController(10)]
>          self.handlers = handlers
> That make it possible to be clear about whether you want default
> handlers Runtime() or no handlers Runtime([])

Good idea, I'll do that and it's a nicer solution to the defaults than a 
global.

> Example ordering: I would put the direct example first. People want to
> see something work right away, it is annoying if the first thing you
> have to do is set up some extra software. I think the broker example
> second would be fine, even if it is a line shorter.

I do see your point and I'll chew this over a bit.

> Waiting for subscriptions with direct connections: one of the things
> that bit me hard and often with proton is waiting for things to be
> ready. In particular if you create a receiver & sender on separate
> connections and send immediately you have a race condition where the
> message will sometimes be lost because the sender sent it before the
> subscription was actually active. This is quite infuriating to debug.

Yes (unless of course you use some sort of 'broker' that could perhaps 
'queue up' messages it received until there was a subscriber to which 
they could be forwarded!)

> You dodge the issue by using the same connection for sender & receiver
> which is not the case for any but the most trivial example.
> I suggest you face this head on:

That's a fair point. I've received other feedback that the examples were 
not 'reactive' enough - too much done before the event loop is entered, 
not enough done within the loop itself - which fits in with this.

I've already made some changes that address the lack of reactivity by 
creating the sender and receiver only when the connection is ready and 
sending only when credit has been issued. I could perhaps modify this 
further to have the sender created only once the receiver is ready.

The request-response example shows that already of course, since the 
receiver's address, generated by the broker, is required before sending 
the first message.

One of the goals is to emphasise that simple things can be done simply.

In my first attempt I fell in to the trap of trying to make the first 
example so simple that it lost any real illustrative value. However it 
is 'hello world' and I don't want to overcomplicate it either, 
especially if the issues could be reasonably dealt with in subsequent 
examples... just need to find the right balance and all the feedback 
helps so thanks again!

> the direct example should show how to
> create a receiver on one connection, wait till it is active, then send
> on a different connection.

The direct example only actually creates one link (it's a sender from 
the "client's" perspective and a receiver from the "server's"). I quite 
like that aspect of it so I think your comment is perhaps more 
applicable to the non-direct i.e. brokered case.

> Reconnect: needs more connection lifecycle events (connected,
> heartbeats) but I see you are planning to address this.

Yes. So far I've been making some updates based on feedback received 
(examples are all now updated as checked in, though the tutorial text is 
still lagging behind a little). However my next step was to look at 
timer driven events (e.g. for heartbeats or reconnect backoff) etc, and 
some more detailed examples on approaches to reliable messaging.

> Messages and threads: once received an app will want to dispatch
> messages based on address and other properties and allocate processing
> to threads in various ways. This is (should be) orthogonal to the API
> you are designing here but we need to make sure we have adequate thread
> safety and don't create limitations in the API that make things
> difficult.

I agree that more thought and examples on how to interact with other 
threads is important. However at the same time, I want to avoid trying 
to make everything threadsafe.

> E.g. the app may want to process messages from a single
> proton connection (i.e. a single event stream) in multiple threads and
> send reply messages back to the same connection from arbitrary threads.

The proton connection object is not threadsafe, and I don't think that 
should change.

Providing utilities to allow message processing on different threads and 
have interaction with a given connection coordinated is certainly 
something I agree should be explored.

> We should also make it possible for a single thread pool to be used to
> handle proton and application events.
>
> MessagingContext: I found this a bit confusing. I got the impression
> that its an abstraction over either a session or a connection, but I am
> not too clear what purpose it serves.

Yes, it's (mostly) to allow users to ignore sessions unless they 
explicitly want to control them. (It also provided a convenient place to 
locate some utility code for creating links in the common cases which is 
quite verbose with the engine api as it is).


---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org