Skip to content
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

expose netty channel to the KeepAliveStrategy #1622

Merged
merged 3 commits into from
May 9, 2019

Conversation

wuguangkuo
Copy link
Contributor

@wuguangkuo wuguangkuo commented Mar 7, 2019

expose netty channel to the KeepAliveStrategy, so that we can get current InetAddress used by current request. when the custom DNS cache is updated, we can set current request's keepalive to false, just like what we can do used the apache httpcomponents "ConnectionReuseStrategy".
Below is my pseudocode using the new KeepAliveStrategy:

public boolean keepAlive(Channel channel, Request ahcRequest, HttpRequest request, HttpResponse response) {
        String hostname = ahcRequest.getUri().getHost();
        InetSocketAddress socketAddress = (InetSocketAddress) channel.remoteAddress();
        InetAddress currentAddress = socketAddress.getAddress();
        List<InetAddress> newAddressList = DnsCache.getAddress(hostname);
        if (currentAddress not in newAddressList) {
                //the "DnsCache" has been updated:the remote server is going to off line.
                return false;
        }
        return super.keepAlive(channel, ahcRequest, request, response);
    }

…rent InetAddress used by current request. when the custom DNS cache is updated, we can set current request's keepalive to false, just like what we can do used the apache httpcomponents "ConnectionReuseStrategy".
@wuguangkuo
Copy link
Contributor Author

@slandelle Hi, could you deal this pull request?

@slandelle
Copy link
Contributor

I'm not fond of exposing Channel. Could you please update your PR to expose the InetSocketAddress instead please?

@wuguangkuo
Copy link
Contributor Author

I'm not fond of exposing Channel. Could you please update your PR to expose the InetSocketAddress instead please?

Ok, I encapsulate a class "RealConnection" as a holder of localAddress and remoteAddress.

@slandelle
Copy link
Contributor

I don’t get how local address is interesting for deciding on keeping the connection alive. Could you please explain ?

@wuguangkuo
Copy link
Contributor Author

I don’t get how local address is interesting for deciding on keeping the connection alive. Could you please explain ?

In my case, the remote address is enough. But I think there may be someone need the local address to do something. In fact, I modified the code emulating "HttpInetConnection" in apache httpcomponents.

@slandelle
Copy link
Contributor

I really don't see a use case for local address here. Please just expose remote one.

@wuguangkuo
Copy link
Contributor Author

modified

@slandelle slandelle merged commit 71dad6a into AsyncHttpClient:master May 9, 2019
@slandelle slandelle added this to the 2.9.0 milestone May 21, 2019
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