You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by "Jeff Hammerbacher (JIRA)" <ji...@apache.org> on 2010/05/18 04:32:43 UTC

[jira] Created: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Allow the HttpServer to serve forever without a call to Thread.sleep()
----------------------------------------------------------------------

                 Key: AVRO-544
                 URL: https://issues.apache.org/jira/browse/AVRO-544
             Project: Avro
          Issue Type: New Feature
          Components: java
            Reporter: Jeff Hammerbacher


One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Posted by "Jeff Hammerbacher (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jeff Hammerbacher updated AVRO-544:
-----------------------------------

    Attachment: AVRO-544.patch

Hey Patrick,

Thanks for the idea. I did the dead-simplest exposing of {{join()}} and it works for me. Now I can do things like:
{code}
    HttpServer server = new HttpServer(r, 9090);
    server.join();
{code}

instead of:

{code}
    HttpServer server = new HttpServer(r, 9090);
    Thread.sleep(10000);
{code}

Which is sane. I could be naive in how I expose join(), however, so if someone more familiar with Java servers wants to take a look, please do.

Thanks,
Jeff

> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
>                 Key: AVRO-544
>                 URL: https://issues.apache.org/jira/browse/AVRO-544
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Jeff Hammerbacher
>            Assignee: Jeff Hammerbacher
>         Attachments: AVRO-544.patch
>
>
> One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Doug Cutting updated AVRO-544:
------------------------------

          Status: Resolved  (was: Patch Available)
    Hadoop Flags: [Incompatible change, Reviewed]
    Release Note: Changed servers to not be started by constructor, but by new start() method.
      Resolution: Fixed

> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
>                 Key: AVRO-544
>                 URL: https://issues.apache.org/jira/browse/AVRO-544
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Jeff Hammerbacher
>            Assignee: Jeff Hammerbacher
>             Fix For: 1.4.0
>
>         Attachments: AVRO-544-2.patch, AVRO-544-3.patch, AVRO-544-4.patch, AVRO-544.patch, AVRO-544.patch
>
>
> One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Posted by "Jeff Hammerbacher (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12889308#action_12889308 ] 

Jeff Hammerbacher commented on AVRO-544:
----------------------------------------

Hey Doug,

All of the comments above have been addressed. I believe this patch is ready to go in.

Thanks,
Jeff

> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
>                 Key: AVRO-544
>                 URL: https://issues.apache.org/jira/browse/AVRO-544
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Jeff Hammerbacher
>            Assignee: Jeff Hammerbacher
>             Fix For: 1.4.0
>
>         Attachments: AVRO-544-2.patch, AVRO-544-3.patch, AVRO-544-4.patch, AVRO-544.patch
>
>
> One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Posted by "Jeff Hammerbacher (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12885865#action_12885865 ] 

Jeff Hammerbacher commented on AVRO-544:
----------------------------------------

bq. Lastly, why do we use close()? Jetty uses stop(), and that seems like the logical thing for Avro servers to implement as well.

To clarify: I am only talking about choice of name for the function.

> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
>                 Key: AVRO-544
>                 URL: https://issues.apache.org/jira/browse/AVRO-544
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Jeff Hammerbacher
>            Assignee: Jeff Hammerbacher
>             Fix For: 1.4.0
>
>         Attachments: AVRO-544-2.patch, AVRO-544-3.patch, AVRO-544.patch
>
>
> One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Posted by "Jeff Hammerbacher (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jeff Hammerbacher updated AVRO-544:
-----------------------------------

           Status: Patch Available  (was: Open)
    Fix Version/s: 1.4.0

> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
>                 Key: AVRO-544
>                 URL: https://issues.apache.org/jira/browse/AVRO-544
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Jeff Hammerbacher
>            Assignee: Jeff Hammerbacher
>             Fix For: 1.4.0
>
>         Attachments: AVRO-544.patch
>
>
> One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Posted by "Jeff Hammerbacher (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12885397#action_12885397 ] 

Jeff Hammerbacher commented on AVRO-544:
----------------------------------------

For another point of reference, rather than calling {{start()}} in the ctor, {{org.apache.hadoop.http.HttpServer}} exposes {{start()}, {{stop()}}, and {{join()}} as public methods.

I think I like the Hadoop way best: pull {{start()}} out of the ctor and make it a public method. That means existing client code needs to change to call {{start()}} after the ctor, but I think it's worth it.

Thoughts?

> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
>                 Key: AVRO-544
>                 URL: https://issues.apache.org/jira/browse/AVRO-544
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Jeff Hammerbacher
>            Assignee: Jeff Hammerbacher
>             Fix For: 1.4.0
>
>         Attachments: AVRO-544-2.patch, AVRO-544.patch
>
>
> One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Assigned: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Posted by "Jeff Hammerbacher (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jeff Hammerbacher reassigned AVRO-544:
--------------------------------------

    Assignee: Jeff Hammerbacher  (was: Patrick Wendell)

> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
>                 Key: AVRO-544
>                 URL: https://issues.apache.org/jira/browse/AVRO-544
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Jeff Hammerbacher
>            Assignee: Jeff Hammerbacher
>
> One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Posted by "Ken Krugler (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12885405#action_12885405 ] 

Ken Krugler commented on AVRO-544:
----------------------------------

If you're requiring a {{start()}} method call (which I like), then I'm still curious why you think having {{join()}} is important.

> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
>                 Key: AVRO-544
>                 URL: https://issues.apache.org/jira/browse/AVRO-544
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Jeff Hammerbacher
>            Assignee: Jeff Hammerbacher
>             Fix For: 1.4.0
>
>         Attachments: AVRO-544-2.patch, AVRO-544.patch
>
>
> One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Posted by "Ken Krugler (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12885045#action_12885045 ] 

Ken Krugler commented on AVRO-544:
----------------------------------

In all my code where I need to start up a Jetty server, I do the start() and join() together, as in:

    public void start() throws Exception {
        _server.start();
        _server.join();
    }

What's the rational for splitting these up? If you need to have explicit control over starting/stopping the server, then I would have a start() method as per above, otherwise just put both into the constructor.



> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
>                 Key: AVRO-544
>                 URL: https://issues.apache.org/jira/browse/AVRO-544
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Jeff Hammerbacher
>            Assignee: Jeff Hammerbacher
>             Fix For: 1.4.0
>
>         Attachments: AVRO-544-2.patch, AVRO-544.patch
>
>
> One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Posted by "Philip Zeyliger (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12886156#action_12886156 ] 

Philip Zeyliger commented on AVRO-544:
--------------------------------------

Took a look via reviewboard:

The code, besides the catch (Exception e), looks fine.

I do worry that this is a massively backwards-incompatible change.  Anyone who's set up an Avro RPC server using HttpServer now needs to add a line to their code.  I can only think of hacky ways around that.


trunk/lang/java/src/java/org/apache/avro/ipc/HttpServer.java
<http://review.hbase.org/r/281/#comment1391>

   I would add to the java doc that this throws AvroRuntimeException.



trunk/lang/java/src/java/org/apache/avro/ipc/HttpServer.java
<http://review.hbase.org/r/281/#comment1392>

   catch (Exception e) is poor form, typically.  It's usually better to explicitly catch the type of exception that Jetty's start throws.


- Philip


> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
>                 Key: AVRO-544
>                 URL: https://issues.apache.org/jira/browse/AVRO-544
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Jeff Hammerbacher
>            Assignee: Jeff Hammerbacher
>             Fix For: 1.4.0
>
>         Attachments: AVRO-544-2.patch, AVRO-544-3.patch, AVRO-544.patch
>
>
> One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Posted by "Ken Krugler (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12885739#action_12885739 ] 

Ken Krugler commented on AVRO-544:
----------------------------------

bq. Ken, if its important to you to avoid typing both start() and join(), please open a different JIRA for that.

No, I was just curious about having a separate {{start()}} versus {{join()}}, as I hadn't run into that use case.

> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
>                 Key: AVRO-544
>                 URL: https://issues.apache.org/jira/browse/AVRO-544
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Jeff Hammerbacher
>            Assignee: Jeff Hammerbacher
>             Fix For: 1.4.0
>
>         Attachments: AVRO-544-2.patch, AVRO-544.patch
>
>
> One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Issue Comment Edited: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Posted by "Jeff Hammerbacher (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12885864#action_12885864 ] 

Jeff Hammerbacher edited comment on AVRO-544 at 7/7/10 3:27 AM:
----------------------------------------------------------------

Okay, here's a patch that factors start() out of the ctor as well, and changes all of the users of the ctor as well. Note that exposing start() should allow us to make TestStatsPluginAndServlet.java less hacky (e.g. only start one server), but I'll leave that for those who know Java better than me.

Also, I note that DatagramServer and SocketServer both implement the Server interface, so they should probably override start() as well. Given that they are deprecated, and they don't currently use the @Override annotation on close() either, I didn't bother to change them.

Lastly, why do we use close()? Jetty uses stop(), and that seems like the logical thing for Avro servers to implement as well.

      was (Author: hammer):
    Okay, here's a patch that factors start() out of the ctor as well, and changes all of the calling applications. Note that exposing start() should allow us to make TestStatsPluginAndServlet.java less hacky (e.g. only start one server), but I'll leave that for those who know Java better than me.

Also, I note that DatagramServer and SocketServer both implement the Server interface, so they should probably override start() as well. Given that they are deprecated, and they don't currently use the @Override annotation on close() either, I didn't bother to change them.

Lastly, why do we use close()? Jetty uses stop(), and that seems like the logical thing for Avro servers to implement as well.
  
> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
>                 Key: AVRO-544
>                 URL: https://issues.apache.org/jira/browse/AVRO-544
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Jeff Hammerbacher
>            Assignee: Jeff Hammerbacher
>             Fix For: 1.4.0
>
>         Attachments: AVRO-544-2.patch, AVRO-544-3.patch, AVRO-544.patch
>
>
> One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Posted by "Philip Zeyliger (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12886187#action_12886187 ] 

Philip Zeyliger commented on AVRO-544:
--------------------------------------

>From reviewboard:

Jeff Hammerbacher:
{quote}
> On 2010-07-07 16:51:01, Philip Zeyliger wrote:
> > The code, besides the catch (Exception e), looks fine.
> >
> > I do worry that this is a massively backwards-incompatible change.  Anyone who's set up an Avro RPC server using HttpServer now needs to add a line to their code.  I can only think of hacky ways around that.

Well, considering that Avro RPC is under active development, I think now is the time to make massively breaking changes. The discussion on this ticket seems to indicate that the changes are for the better, so I vote for making them now while we still can.

To that end, see also https://issues.apache.org/jira/browse/AVRO-594
{quote}

Scott Carey:
{quote}
ReviewBoard is awesome.  I'll need to figure out how to use it sometime.

Comments inline:
On Jul 7, 2010, at 4:51 PM, Philip Zeyliger wrote:

>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/281/#review320
> -----------------------------------------------------------
>
>
> The code, besides the catch (Exception e), looks fine.
>
> I do worry that this is a massively backwards-incompatible change.  Anyone who's set up an Avro RPC server using HttpServer now needs to add a line to their code.  I can only think of hacky ways around that.
>
Yeah, that is a concern.  This is definitely an improvement though.

>
> trunk/lang/java/src/java/org/apache/avro/ipc/HttpServer.java
> <http://review.hbase.org/r/281/#comment1391>
>
>    I would add to the java doc that this throws AvroRuntimeException.
>

"@throws AvroRuntimeException if the underlying Jetty server throws any exception while starting." ?

>
>
> trunk/lang/java/src/java/org/apache/avro/ipc/HttpServer.java
> <http://review.hbase.org/r/281/#comment1392>
>
>    catch (Exception e) is poor form, typically.  It's usually better to explicitly catch the type of exception that Jetty's start throws.
>

True, but Jetty's start() signature is:

void org.mortbay.component.AbstractLifeCycle.start() throws Exception

So my only caution with this patch is the backwards-incompatible change.  Luckily it is not subtle.  I'm fine with the change but am not a major user of HttpServer.  This does likely imply a change for other Servers down the road too.
{quote}

> On 2010-07-07 16:51:01, Philip Zeyliger wrote:
> > trunk/lang/java/src/java/org/apache/avro/ipc/HttpServer.java, line 65
> > <http://review.hbase.org/r/281/diff/1/?file=2195#file2195line65>
> >
> >     catch (Exception e) is poor form, typically.  It's usually better to explicitly catch the type of exception that Jetty's start throws.

Jetty's start() throws Exception: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/component/LifeCycle.html#start%28%29


> On 2010-07-07 16:51:01, Philip Zeyliger wrote:
> > trunk/lang/java/src/java/org/apache/avro/ipc/HttpServer.java, line 60
> > <http://review.hbase.org/r/281/diff/1/?file=2195#file2195line60>
> >
> >     I would add to the java doc that this throws AvroRuntimeException.

Will do.
{quote}

> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
>                 Key: AVRO-544
>                 URL: https://issues.apache.org/jira/browse/AVRO-544
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Jeff Hammerbacher
>            Assignee: Jeff Hammerbacher
>             Fix For: 1.4.0
>
>         Attachments: AVRO-544-2.patch, AVRO-544-3.patch, AVRO-544-4.patch, AVRO-544.patch
>
>
> One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Posted by "Jeff Hammerbacher (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jeff Hammerbacher updated AVRO-544:
-----------------------------------

    Attachment: AVRO-544-4.patch

Added a @throws comment to the Javadoc for start(). Any other comments? I think this may be ready to commit.

> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
>                 Key: AVRO-544
>                 URL: https://issues.apache.org/jira/browse/AVRO-544
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Jeff Hammerbacher
>            Assignee: Jeff Hammerbacher
>             Fix For: 1.4.0
>
>         Attachments: AVRO-544-2.patch, AVRO-544-3.patch, AVRO-544-4.patch, AVRO-544.patch
>
>
> One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12890044#action_12890044 ] 

Doug Cutting commented on AVRO-544:
-----------------------------------

+1 this looks good to me and passes tests.

Only minor nit is that I'd replace the javadoc comment "Prepares a server to be started on the named port." with "Constructs a server that will run on the named port."

> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
>                 Key: AVRO-544
>                 URL: https://issues.apache.org/jira/browse/AVRO-544
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Jeff Hammerbacher
>            Assignee: Jeff Hammerbacher
>             Fix For: 1.4.0
>
>         Attachments: AVRO-544-2.patch, AVRO-544-3.patch, AVRO-544-4.patch, AVRO-544.patch
>
>
> One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Issue Comment Edited: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Posted by "Jeff Hammerbacher (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12885397#action_12885397 ] 

Jeff Hammerbacher edited comment on AVRO-544 at 7/5/10 11:12 PM:
-----------------------------------------------------------------

For another point of reference, rather than calling {{start()}} in the ctor, {{org.apache.hadoop.http.HttpServer}} exposes {{start()}}, {{stop()}}, and {{join()}} as public methods.

I think I like the Hadoop way best: pull {{start()}} out of the ctor and make it a public method. That means existing client code needs to change to call {{start()}} after the ctor, but I think it's worth it.

Thoughts?

      was (Author: hammer):
    For another point of reference, rather than calling {{start()}} in the ctor, {{org.apache.hadoop.http.HttpServer}} exposes {{start()}, {{stop()}}, and {{join()}} as public methods.

I think I like the Hadoop way best: pull {{start()}} out of the ctor and make it a public method. That means existing client code needs to change to call {{start()}} after the ctor, but I think it's worth it.

Thoughts?
  
> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
>                 Key: AVRO-544
>                 URL: https://issues.apache.org/jira/browse/AVRO-544
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Jeff Hammerbacher
>            Assignee: Jeff Hammerbacher
>             Fix For: 1.4.0
>
>         Attachments: AVRO-544-2.patch, AVRO-544.patch
>
>
> One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Assigned: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Posted by "Patrick Wendell (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Patrick Wendell reassigned AVRO-544:
------------------------------------

    Assignee: Patrick Wendell

> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
>                 Key: AVRO-544
>                 URL: https://issues.apache.org/jira/browse/AVRO-544
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Jeff Hammerbacher
>            Assignee: Patrick Wendell
>
> One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Posted by "Jeff Hammerbacher (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jeff Hammerbacher updated AVRO-544:
-----------------------------------

    Attachment: AVRO-544-2.patch

Adding a Javadoc per Scott's feedback

> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
>                 Key: AVRO-544
>                 URL: https://issues.apache.org/jira/browse/AVRO-544
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Jeff Hammerbacher
>            Assignee: Jeff Hammerbacher
>             Fix For: 1.4.0
>
>         Attachments: AVRO-544-2.patch, AVRO-544.patch
>
>
> One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Posted by "Scott Carey (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12885732#action_12885732 ] 

Scott Carey commented on AVRO-544:
----------------------------------

bq. I think I like the Hadoop way best: pull start() out of the ctor and make it a public method. That means existing client code needs to change to call start() after the ctor, but I think it's worth it.

+1  Oops, I didn't interpret the whole thread here properly the first time through.  A new patch that did this like Hadoop does would be better than the current one IMO.

I think we should probably avoid doing anything other than prep and configuration in the constructor and allow users to control when the server is started.  For example it is often important to only open your ports for requests after some prep-work is done, and it is often useful to construct the object at a different time or place from when you start.

Exposing start(), stop() and join() like Hadoop is the way to go -- it lets users be maximally flexible.  There is a reason why the Jetty API has it like that.

We can consider a convenience method that does both start() and join() in one go (perhaps 'run()'?) but that is trivial and some might call it API clutter.  It saves one line of trivial code, so I'm a little dubious of the value.  
Lets finish this JIRA and discuss that in another JIRA.  Ken, if its important to you to avoid typing both start() and join(), please open a different JIRA for that.


> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
>                 Key: AVRO-544
>                 URL: https://issues.apache.org/jira/browse/AVRO-544
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Jeff Hammerbacher
>            Assignee: Jeff Hammerbacher
>             Fix For: 1.4.0
>
>         Attachments: AVRO-544-2.patch, AVRO-544.patch
>
>
> One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Posted by "Scott Carey (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12885694#action_12885694 ] 

Scott Carey commented on AVRO-544:
----------------------------------

+1 on AVRO-544-2.patch

bq. What's the rational for splitting these up? If you need to have explicit control over starting/stopping the server, then I would have a start() method as per above, otherwise just put both into the constructor.

start() should not block.  The constructor should most definitely not block.  If a user wants to block, there is join(), they should not be forced to block and burn up a thread.

Use cases I've had where I don't want to block are numerous.

Here is an example, two servers with different ports / configurations plus a thread pool with scheduled tasks for custom business logic:

{code}
MyCustomExecutorService pool = ...
HttpServer server1 = ...
HttpServer server2 = ...

pool.submitWithFixedDelay(someTask);
server1.start();
server2.start();

boolean shutdown = false;
while(!shutdown) {
  shutdown = MyCustomShutdownListener.awaitShutdown();
}
server2.stop();
server1.stop();
pool.shutdownNow();
{code}

Also, it is generally better to explicitly start asynchronous activities after construction and configuration of their representative entities -- there are many use cases where construction and starting should be separated.



> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
>                 Key: AVRO-544
>                 URL: https://issues.apache.org/jira/browse/AVRO-544
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Jeff Hammerbacher
>            Assignee: Jeff Hammerbacher
>             Fix For: 1.4.0
>
>         Attachments: AVRO-544-2.patch, AVRO-544.patch
>
>
> One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Posted by "Doug Cutting (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Doug Cutting updated AVRO-544:
------------------------------

    Attachment: AVRO-544.patch

Here's a version that:
 - changes the javadoc as suggested above
 - updates SocketServer, NettyServer & DatagramServer too, plus their tests

> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
>                 Key: AVRO-544
>                 URL: https://issues.apache.org/jira/browse/AVRO-544
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Jeff Hammerbacher
>            Assignee: Jeff Hammerbacher
>             Fix For: 1.4.0
>
>         Attachments: AVRO-544-2.patch, AVRO-544-3.patch, AVRO-544-4.patch, AVRO-544.patch, AVRO-544.patch
>
>
> One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Posted by "Ken Krugler (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12885980#action_12885980 ] 

Ken Krugler commented on AVRO-544:
----------------------------------

I agree that {{stop()}} is the better name.

> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
>                 Key: AVRO-544
>                 URL: https://issues.apache.org/jira/browse/AVRO-544
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Jeff Hammerbacher
>            Assignee: Jeff Hammerbacher
>             Fix For: 1.4.0
>
>         Attachments: AVRO-544-2.patch, AVRO-544-3.patch, AVRO-544.patch
>
>
> One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Posted by "Jeff Hammerbacher (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jeff Hammerbacher updated AVRO-544:
-----------------------------------

    Attachment: AVRO-544-3.patch

Okay, here's a patch that factors start() out of the ctor as well, and changes all of the calling applications. Note that exposing start() should allow us to make TestStatsPluginAndServlet.java less hacky (e.g. only start one server), but I'll leave that for those who know Java better than me.

Also, I note that DatagramServer and SocketServer both implement the Server interface, so they should probably override start() as well. Given that they are deprecated, and they don't currently use the @Override annotation on close() either, I didn't bother to change them.

Lastly, why do we use close()? Jetty uses stop(), and that seems like the logical thing for Avro servers to implement as well.

> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
>                 Key: AVRO-544
>                 URL: https://issues.apache.org/jira/browse/AVRO-544
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Jeff Hammerbacher
>            Assignee: Jeff Hammerbacher
>             Fix For: 1.4.0
>
>         Attachments: AVRO-544-2.patch, AVRO-544-3.patch, AVRO-544.patch
>
>
> One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Posted by "Scott Carey (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12890077#action_12890077 ] 

Scott Carey commented on AVRO-544:
----------------------------------

+1 looks good to me.

> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
>                 Key: AVRO-544
>                 URL: https://issues.apache.org/jira/browse/AVRO-544
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Jeff Hammerbacher
>            Assignee: Jeff Hammerbacher
>             Fix For: 1.4.0
>
>         Attachments: AVRO-544-2.patch, AVRO-544-3.patch, AVRO-544-4.patch, AVRO-544.patch, AVRO-544.patch
>
>
> One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Posted by "Scott Carey (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12885002#action_12885002 ] 

Scott Carey commented on AVRO-544:
----------------------------------

+1 It looks sane to expose Jetty's join() method.  Since this is public we probably want a line of javadoc along the lines of "Will block until the server has stopped or the thread is interrupted."

 If anything more complicated is needed down the road (for example, a timeout while waiting or callback when it finishes) we can add that later.

> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
>                 Key: AVRO-544
>                 URL: https://issues.apache.org/jira/browse/AVRO-544
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Jeff Hammerbacher
>            Assignee: Jeff Hammerbacher
>             Fix For: 1.4.0
>
>         Attachments: AVRO-544.patch
>
>
> One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (AVRO-544) Allow the HttpServer to serve forever without a call to Thread.sleep()

Posted by "Jeff Hammerbacher (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/AVRO-544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12890059#action_12890059 ] 

Jeff Hammerbacher commented on AVRO-544:
----------------------------------------

Looks good. I haven't tested the new changes, but they seem reasonable via eyeballed code review. If the tests pass, go for it.

> Allow the HttpServer to serve forever without a call to Thread.sleep()
> ----------------------------------------------------------------------
>
>                 Key: AVRO-544
>                 URL: https://issues.apache.org/jira/browse/AVRO-544
>             Project: Avro
>          Issue Type: New Feature
>          Components: java
>            Reporter: Jeff Hammerbacher
>            Assignee: Jeff Hammerbacher
>             Fix For: 1.4.0
>
>         Attachments: AVRO-544-2.patch, AVRO-544-3.patch, AVRO-544-4.patch, AVRO-544.patch, AVRO-544.patch
>
>
> One way would be to expose the join() method on the HttpServer: http://jetty.codehaus.org/jetty/jetty-6/apidocs/org/mortbay/jetty/Server.html#join%28%29

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.