LIVE-6468 Reuse http client across invocations #1218
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
waisingyiu
force-pushed
the
LIVE-6468-reuse-http-client-across-invocations
branch
from
effa81f
to
60a9b06
Compare
waisingyiu
force-pushed
the
LIVE-6468-reuse-http-client-across-invocations
branch
from
e9342ba
to
234cebd
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this change?
The PR #1217 creates a
HttpClientinstance for the processing of each SQS client so that we don't send too many requests over the same connection.However, the AWS lambda service reuses execution environment when it needs to invoke a lambda shortly after one has finished. So it is possible that within the same Java runtime, the
AndroidSenderinstance is used to process SQS messages many times and, as a result, creates too manyHttpClientinstances and the connections that are left idle. The connections consume the limit on the file descriptors, which is 1024 for Java lambda runtime. It is likely to happen when we send a huge notification (more than 100,000).What we really want is that we have dedicated
HttpClientinstances for each SQS message, but we want to reuse these instances in the subsequent invocation if the same runtime environment is reused.This PR creates 20
HttpClientinstances upfront (we process at most 20 SQS messages at a time) and uses them when processing the stream of SQS messages.This PR also creates a new configuration to control the max connection pool size to avoid "too-many-opened-files" exception.
How to test
We invoked the Android sender locally with two batches of SQS messages (with 3 messages in each batch). We printed the instance in the log message here and we saw from the logs that the three
FcmClientinstances were reused across invocation:First batch of SQS messages:
Individual send request completed - com.gu.notifications.worker.delivery.fcm.FcmClient@19d807f9Individual send request completed - com.gu.notifications.worker.delivery.fcm.FcmClient@591354ddIndividual send request completed - com.gu.notifications.worker.delivery.fcm.FcmClient@13fa8acSecond batch of SQS messages:
Individual send request completed - com.gu.notifications.worker.delivery.fcm.FcmClient@19d807f9Individual send request completed - com.gu.notifications.worker.delivery.fcm.FcmClient@591354ddIndividual send request completed - com.gu.notifications.worker.delivery.fcm.FcmClient@13fa8acThe service was also tested on
CODEand notification was sent successfully to android app on emulator.