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

connection keepAlive does not apply. #880

Open
stager0909 opened this issue Nov 15, 2023 · 1 comment
Open

connection keepAlive does not apply. #880

stager0909 opened this issue Nov 15, 2023 · 1 comment

Comments

@stager0909
Copy link

[REQUIRED] Step 2: Describe your environment

  • Operating System version: linux
  • Firebase SDK version: firebase-admin-8.1.0.jar
  • Library version: 8.1.0.jar
  • Firebase Product: Cloud Messaging (auth, database, storage, etc)

[REQUIRED] Step 3: Describe the problem

We confirmed that keepAlive is applied when using firebase-admin version 6.8.0.
However, after upgrading to firebase-admin 8.1.0, keepAlive is not applied and the connection is immediately disconnected.

Steps to reproduce:

firebase-admin 6.8.0 FirebaseMessagingClient.sendSingleRequest()
image
(apache httpclient-4.5.11) ConnectionHolder.releaseConnection() is called due to line 129.
At this time, this.released changes from false to true.
After that, ApiClientUtils.disconnectQuietly(response) -> (apache httpclient-4.5.11) ConnectionHolder.abortConnection() on line 132 is called, but the connection is not disconnected because ConnectionHolder.released = true.
image
image

However, in firebase-admin 8.1.0, ConnectionHolder.releaseConnection() is not called, but ConnectionHolder.abortConnection() is called, so the connection is disconnected.
image

What happened? How can we make the problem occur?
Establishing connections is expensive. Therefore, recycling connections for a certain period of time can help improve performance. According to what I have checked, the code of the latest version, 9.2.0, is also the same.

Relevant Code:

private FirebaseMessaging initializeFirebaseMessaging()
        throws IOException {
        CloseableHttpClient httpClient = newHttpClient(proxyHost, proxyPort);

        HttpTransport httpTransport = new ApacheHttpTransport(httpClient);

        FirebaseOptions options = FirebaseOptions.builder()
// need credentials etc
                                                 .setHttpTransport(httpTransport)
                                                 .build();

        FirebaseApp.initializeApp(options);
        return FirebaseMessaging.getInstance();
    }

private CloseableHttpClient newHttpClient(String proxyHost, int proxyPort) {
        HttpClientBuilder httpClientBuilder = ApacheHttpTransport.newDefaultHttpClientBuilder()
                                                                 .evictExpiredConnections()
                                                                 .setKeepAliveStrategy(
                                                                     new CustomConnectionKeepAliveStrategy(
                                                                         TimeUnit.MINUTES.toMillis(3)))
                                                                 .setConnectionTimeToLive(10, TimeUnit.MINUTES)
                                                                 .setDefaultRequestConfig(requestConfig())
                                                                 .disableCookieManagement();
        return httpClientBuilder.build();
    }

public class CustomConnectionKeepAliveStrategy implements ConnectionKeepAliveStrategy {

    public static final CustomConnectionKeepAliveStrategy INSTANCE =
        new CustomConnectionKeepAliveStrategy(TimeUnit.SECONDS.toMillis(50),
            DefaultConnectionKeepAliveStrategy.INSTANCE);

    public static final ConnectionKeepAliveStrategy NO_KEEPALIVE = (response, context) -> 0;

    private static final long KEEP_ALIVE_SAFE_GAP = 500L;
    private final ConnectionKeepAliveStrategy defaultConnectionKeepAliveStrategy;
    private final long defaultKeepAliveTimeoutInMillis;

    public CustomConnectionKeepAliveStrategy(long defaultKeepAliveTimeoutInMillis) {
        this(defaultKeepAliveTimeoutInMillis, DefaultConnectionKeepAliveStrategy.INSTANCE);
    }

    public CustomConnectionKeepAliveStrategy(long defaultKeepAliveTimeoutInMillis,
        ConnectionKeepAliveStrategy defaultConnectionKeepAliveStrategy) {
        Args.notNegative(defaultKeepAliveTimeoutInMillis, "defaultKeepAliveTimeoutInMillis");
        Args.notNull(defaultConnectionKeepAliveStrategy, "defaultConnectionKeepAliveStrategy");
        this.defaultKeepAliveTimeoutInMillis = defaultKeepAliveTimeoutInMillis;
        this.defaultConnectionKeepAliveStrategy = defaultConnectionKeepAliveStrategy;
    }

    public static long adjustKeepAliveTimeout(long keepAliveTimeoutInMillis) {
        return Math.max(0L, (keepAliveTimeoutInMillis - KEEP_ALIVE_SAFE_GAP));
    }

    @Override
    public long getKeepAliveDuration(final HttpResponse response, final HttpContext context) {
        long keepAliveDuration = defaultConnectionKeepAliveStrategy.getKeepAliveDuration(response, context);
        if (keepAliveDuration == -1L) {
            return defaultKeepAliveTimeoutInMillis;
        }
        if (keepAliveDuration > 0L) {
            return adjustKeepAliveTimeout(keepAliveDuration);
        }
        return keepAliveDuration;
    }
@google-oss-bot
Copy link

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

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

No branches or pull requests

2 participants