Skip to content

Allow user to specify whether pool is LIFO or FIFO #1188

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 14, 2016
Merged

Allow user to specify whether pool is LIFO or FIFO #1188

merged 1 commit into from
Jun 14, 2016

Conversation

tomfitzhenry
Copy link
Contributor

Issue: #1184

What do you think about whether this should be configurable via ahc.properties? I initially did this, but I wondered whether there might be implementations of ChannelPool for which LIFO vs FIFO didn't make sense, and by putting LIFO vs FIFO at properties file level, that might be a leaky abstraction.

@slandelle
Copy link
Contributor

What do you think about whether this should be configurable via ahc.properties?

Usage would be very specific/limited as it could only tune the default DefaultChannelPool.
I think this version is sufficient for a first version.

Let's see if users need more (meaning that they would need to be aware of this feature, meaning documentation, which is this project's weakness).

WDYT?

@ghost
Copy link

ghost commented Jun 14, 2016

I added this wiki page: https://github.com/AsyncHttpClient/async-http-client/wiki/Connection-pooling

Suggestions welcome!

@slandelle
Copy link
Contributor

@bbc-tomfitzhenry Awesome, thanks!

Some comments:

  • You actually have a AsyncHttpClientConfig, not a builder, as you call build
  • You could use the helpers from org.asynchttpclient.Dsl (use a static import)
  • Beware that if you're having your own timer, you're in charge of shutting it down

@slandelle slandelle merged commit c1dc343 into AsyncHttpClient:master Jun 14, 2016
@slandelle slandelle added this to the 2.0.6 milestone Jun 14, 2016
@tomfitzhenry
Copy link
Contributor Author

Fixed docs to use org.asynchttpclient.Dsl. Didn't know about that!

@slandelle
Copy link
Contributor

Thanks!

Didn't know about that!

New in AHC2. When I'm telling you doc is lacking ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants