Details

    • Type: New Feature New Feature
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: Future
    • Component/s: HttpClient
    • Labels:
      None

      Description

      I'm currently working on a patch (wrote most of it on a cross-country flight) that will adapt the size of a per-route connection pool based on the interactions we see from that particular host. There's a sample implementation that does TCP-style additive increase/multiplicative decrease (AIMD) adaptation of the per-route pool where successful requests allow probing for more connections, but socket timeouts, connection timeouts, and 503s all result in backoffs.

      I'm hoping to hook this up for a demo to show multiple clients hitting a server with a fixed capacity where we can kill one client and the others then increase their pool sizes to take advantage of the unused server capacity. We can then restart the client and see things rebalance again. This would enable folks to use HttpClient e.g. in an application server cluster setting, where we wouldn't have to precompute or adjust the connection pool sizes as we add/remove nodes from the cluster (whether intentionally or via failures).

      Once I get that proof of concept working I'll post a patch for review. Roughly the patch hooks into AbstractHttpClient to look either for an HttpResponse or to catch an Exception, then hands those events off to another object to decide whether to backoff or not. In turn, we dynamically manage a ConnPerRouteBean to adjust the maxPerRoute to allow for the pool to grow or shrink naturally with TSSCM. Default implementations are all backwards compatible and don't change behavior.

      Thoughts?

        Activity

        Hide
        Oleg Kalnichevski added a comment -

        Fair enough.

        Oleg

        Show
        Oleg Kalnichevski added a comment - Fair enough. Oleg
        Hide
        Jon Moore added a comment -

        @Oleg: I think the main thing I was waiting to do was to provide some documentation around how it works and how one would use it, but the code is basically done. I was leaving this open until I could come back around for the docs (which I am planning to do, still).

        Show
        Jon Moore added a comment - @Oleg: I think the main thing I was waiting to do was to provide some documentation around how it works and how one would use it, but the code is basically done. I was leaving this open until I could come back around for the docs (which I am planning to do, still).
        Hide
        Oleg Kalnichevski added a comment -

        Jon, is there anything that blocks resolution of this issue? Are planning to do more work on the adaptive connection pool sizing in the future?

        Oleg

        Show
        Oleg Kalnichevski added a comment - Jon, is there anything that blocks resolution of this issue? Are planning to do more work on the adaptive connection pool sizing in the future? Oleg
        Hide
        Jon Moore added a comment -

        Ok, I modified AbstractHttpClient so we are not catching Throwables anymore, although I'll still allow the ConnectionBackoffStrategy to be passed Throwables.

        I think I'll proceed to merge down to the trunk and kill the branch, since I think I've managed to isolate this work off in its own little area with a minimal seam and with defaults that preserve the current behavior. I need to do some testing with actual connection pools to convince myself this is reasonable, but then otherwise I think the main thing needed would be documentation to describe how to set up and use this (and possibly the addition of some other convenience constructors, which we can talk about later).

        Show
        Jon Moore added a comment - Ok, I modified AbstractHttpClient so we are not catching Throwables anymore, although I'll still allow the ConnectionBackoffStrategy to be passed Throwables. I think I'll proceed to merge down to the trunk and kill the branch, since I think I've managed to isolate this work off in its own little area with a minimal seam and with defaults that preserve the current behavior. I need to do some testing with actual connection pools to convince myself this is reasonable, but then otherwise I think the main thing needed would be documentation to describe how to set up and use this (and possibly the addition of some other convenience constructors, which we can talk about later).
        Hide
        Oleg Kalnichevski added a comment -

        Hi Jon

        Looks very promising. Great stuff! Feel free to merge the changes down to the trunk or proceed with the development on the branch until you are comfortable with the overall design. One thing I thought I should mention though. I personally feel uncomfortable with code that catches Errors or works with Throwables instead of Exceptions. Applications ought not mess with Errors in my opinion. There is no point trying to adjust the size of the connection pool if the application just caused a stack overflow or ran out of memory.

        Oleg

        Show
        Oleg Kalnichevski added a comment - Hi Jon Looks very promising. Great stuff! Feel free to merge the changes down to the trunk or proceed with the development on the branch until you are comfortable with the overall design. One thing I thought I should mention though. I personally feel uncomfortable with code that catches Errors or works with Throwables instead of Exceptions. Applications ought not mess with Errors in my opinion. There is no point trying to adjust the size of the connection pool if the application just caused a stack overflow or ran out of memory. Oleg
        Hide
        Jon Moore added a comment -

        Ok, branch created and patch committed. As you can see, the changes to AbstractHttpClient and ThreadSafeClientConnManager are actually pretty minimal. Let me know what you think.

        Show
        Jon Moore added a comment - Ok, branch created and patch committed. As you can see, the changes to AbstractHttpClient and ThreadSafeClientConnManager are actually pretty minimal. Let me know what you think.
        Hide
        Jon Moore added a comment -

        @Oleg: ok, will do on the branch.

        I did initially want to keep it in the connection manager, although adjusting the pool size means you need to know what happened when you used the request (did you get an I/O exception, did you get a valid HTTP response that nonetheless indicates a backoff signal, etc.), so there has to be some kind of hook in the code that uses the connections (currently in AbstractHttpClient, but I guess could also be in the RequestDirector) that provides the observation point for the control mechanism.

        Let me get what I've got into the branch and then see if there's a better way to go about it.

        Show
        Jon Moore added a comment - @Oleg: ok, will do on the branch. I did initially want to keep it in the connection manager, although adjusting the pool size means you need to know what happened when you used the request (did you get an I/O exception, did you get a valid HTTP response that nonetheless indicates a backoff signal, etc.), so there has to be some kind of hook in the code that uses the connections (currently in AbstractHttpClient, but I guess could also be in the RequestDirector) that provides the observation point for the control mechanism. Let me get what I've got into the branch and then see if there's a better way to go about it.
        Hide
        Oleg Kalnichevski added a comment -

        Sounds very interesting. My personal preference would be, though, to try to move as much of that logic out of AbstractHttpClient into the connection manager.

        I think the most efficient way to proceed is to create a feature branch off current trunk and commit your patch to that branch.

        Oleg

        Show
        Oleg Kalnichevski added a comment - Sounds very interesting. My personal preference would be, though, to try to move as much of that logic out of AbstractHttpClient into the connection manager. I think the most efficient way to proceed is to create a feature branch off current trunk and commit your patch to that branch. Oleg

          People

          • Assignee:
            Jon Moore
            Reporter:
            Jon Moore
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:

              Development