Re: stomp.rb changes please?

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: stomp.rb changes please?

brianm
Changes applied and 1.0.3 cut.

I want to make a lot of changes to the apis as they are very java-like instead of ruby-like at the moment, but will do that in 1.1 (or someone else is welcome to (Andrew?) if you have available time). It could definately handle getting cleaned up as I did the quick-and-dirty fix to apply one of Andrew' changes.

-Brian

On Jan 3, 2007, at 8:58 AM, Andrew Kuklewicz wrote:

Hello user folks and Brian,

I am doing some work using ActiveMessaging (a13g) and have a few suggested changes for stomp.rb version 1.0.2 that have come up in my testing.
Let me say right up front how much I like the stomp client code, clean and easy to use.
This is just a few things I thought I would pass along to try and make things better.

1) In the unsubscribe method, there is a bug/typo:

Currently it has:

1    def unsubscribe(name, headers = {}, subId=NIL)
2      headers[:destination] = name
3      transmit "UNSUBSCRIBE", headers
4      if @reliable
5        subId = name if subId==NIL
6        @h.delete(subId)
7      end
8    end

Line 6 refers to '@h', should be '@subscriptions' I think - @h does not appear anywhere else in the code.

2) Another minor suggestion/enhancement:
make reconnectDelay configurable on Connection creation.
Since 'socket' is called at the end of the initialize, it has to be set on creation, so a constructor would be lovely.
If not, fine, I can add this in the initialize when Connection gets extended for a13g's stmop adapter.

3) Here's another "problem", or could just be an enhancement.

When you set 'reliable' to true, the expectation is that the Connection will try to reconnect (that is how the 'socket' method seems to be written).
This is true initially.  If activemq is down when you get a connection the first time, it will retry every 5 seconds until connecting.

But if activemq goes down, the receive method starts returning nil, and even if the service comes back, it never tries to reconnect, and never resubscribes.
I see in the Client that this causes a break, basically killing the client, but for a "reliable" connection, why not reuse the socket reconnect logic like on creation?

What I would recommend is to have logic in the connection that if the connection is reliable, then when receive returns a null instead of a message object, interpret this as the connection being lost (b/c the nil indicates an EOF was received).  In which case, reset the connection as on creation of the connection, and on reconnection (assuming activemq comes back up) it will also resubscribe based on the @subscriptions.

I have this working  in the stomp adapter in a13g that extends the Connection object like this:

        def receive
          super_result = super()
          if super_result.nil? && @reliable
            $stderr.print "connection.receive returning EOF as nil - resetting connection.\n"
            @socket = nil
            super_result = super()
          end         
          return super_result
        end

The trick here is to reset the @socket to nil  - this forces the connection to try and reconnect when 'socket' is next called.
Perhaps similar logic could just be put into the Connection::receive method?

So far that is all I have found - otherwise, everything has worked out great.

Thanks for the work,

Andrew Kuklewicz
Senior Web Application Developer
www.prx.org


Reply | Threaded
Open this post in threaded view
|

Re: stomp.rb changes please?

andrewk
Thanks Brian - I see the dirty part - I'll submit something better, time allowing.
- Andrew

... beginsinwonder.com ...


brianm wrote
Changes applied and 1.0.3 cut.

I want to make a lot of changes to the apis as they are very java-
like instead of ruby-like at the moment, but will do that in 1.1 (or  
someone else is welcome to (Andrew?) if you have available time). It  
could definately handle getting cleaned up as I did the quick-and-
dirty fix to apply one of Andrew' changes.

-Brian

On Jan 3, 2007, at 8:58 AM, Andrew Kuklewicz wrote:

> Hello user folks and Brian,
>
> I am doing some work using ActiveMessaging (a13g) and have a few  
> suggested changes for stomp.rb version 1.0.2 that have come up in  
> my testing.
> Let me say right up front how much I like the stomp client code,  
> clean and easy to use.
> This is just a few things I thought I would pass along to try and  
> make things better.
>
> 1) In the unsubscribe method, there is a bug/typo:
>
> Currently it has:
>
> 1    def unsubscribe(name, headers = {}, subId=NIL)
> 2      headers[:destination] = name
> 3      transmit "UNSUBSCRIBE", headers
> 4      if @reliable
> 5        subId = name if subId==NIL
> 6        @h.delete(subId)
> 7      end
> 8    end
>
> Line 6 refers to '@h', should be '@subscriptions' I think - @h does  
> not appear anywhere else in the code.
>
> 2) Another minor suggestion/enhancement:
> make reconnectDelay configurable on Connection creation.
> Since 'socket' is called at the end of the initialize, it has to be  
> set on creation, so a constructor would be lovely.
> If not, fine, I can add this in the initialize when Connection gets  
> extended for a13g's stmop adapter.
>
> 3) Here's another "problem", or could just be an enhancement.
>
> When you set 'reliable' to true, the expectation is that the  
> Connection will try to reconnect (that is how the 'socket' method  
> seems to be written).
> This is true initially.  If activemq is down when you get a  
> connection the first time, it will retry every 5 seconds until  
> connecting.
>
> But if activemq goes down, the receive method starts returning nil,  
> and even if the service comes back, it never tries to reconnect,  
> and never resubscribes.
> I see in the Client that this causes a break, basically killing the  
> client, but for a "reliable" connection, why not reuse the socket  
> reconnect logic like on creation?
>
> What I would recommend is to have logic in the connection that if  
> the connection is reliable, then when receive returns a null  
> instead of a message object, interpret this as the connection being  
> lost (b/c the nil indicates an EOF was received).  In which case,  
> reset the connection as on creation of the connection, and on  
> reconnection (assuming activemq comes back up) it will also  
> resubscribe based on the @subscriptions.
>
> I have this working  in the stomp adapter in a13g that extends the  
> Connection object like this:
>
>         def receive
>           super_result = super()
>           if super_result.nil? && @reliable
>             $stderr.print "connection.receive returning EOF as nil  
> - resetting connection.\n"
>             @socket = nil
>             super_result = super()
>           end
>           return super_result
>         end
>
> The trick here is to reset the @socket to nil  - this forces the  
> connection to try and reconnect when 'socket' is next called.
> Perhaps similar logic could just be put into the  
> Connection::receive method?
>
> So far that is all I have found - otherwise, everything has worked  
> out great.
>
> Thanks for the work,
>
> Andrew Kuklewicz
> Senior Web Application Developer
> www.prx.org
>