Move SDK http client to CRT

[jeromevdl]

Is your feature request related to a problem? Please describe.
No

Describe the solution you'd like
As the CRT http client is now available (https://aws.amazon.com/blogs/developer/announcing-availability-of-the-aws-crt-http-client-in-the-aws-sdk-for-java-2-x/), we could use it in the modules we use the SDK (Parameters, Idempotency, SQS, CloudFormation) to reduce the cold start and latency.

Due to the scale, we should do this on one module first for review, and then go from there. Idempotency would be a good candidate.

Describe alternatives you've considered
N/A

Additional context
N/A

[codesometech]

@jeromevdl @msailes I can pick this up

[jeromevdl]

I think this should be tagged v2. It's not really a breaking change as it's internal but it introduces a new library and quite a new way of doing things (asynchronous).

[jreijn]

I would like to pick this up if possible? I guess changes for this have to be made against the v2 branch?

[jeromevdl]

Hi @jreijn , we're not yet ready for this.

[scottgerring]

We've setup the v2 branch so that we're ready to start taking v2 contributions ( #1337 ) and we can start working on this. Following on from What Eleni linked, we'll want to set this up so that common initialization of the clients can be configured in one place in the core module. That is - we should see if we can have one place that does stuff like setting User-Agent, region configuration, etc.), and so on, rather than duplicating this everywhere. This would have to work without coupling powertools-core onto all of the other SDKs of course.

Speaking with @jeromevdl , we think this is going to be a big, cross-cutting change, because CRT imposes async on everything. We'd love to see a single module setup with it, so we have a point of discussion, rather than big-banging the whole thing.

@jreijn are you still interested in helping here?

[jreijn]

@scottgerring yes, I would still like to work on this. Not sure if there is any time planning for the release of v2 as I will be out for the next 2.5 weeks. Otherwise I will continue with my work on this after my vacation.

I agree with taking it single step by step, so we can discuss while moving forward.

[scottgerring]

Hey @jreijn , we'd like to get a bit more definitive with our release planning, but I don't think that being off for a few weeks now makes a difference! Can you reach out when you are back? Suggest we have a quick kick-off chat. Will start having a think about this in the meantime.

Enjoy the break!

[msailes]

Lets watch this issue in the AWS SDK for Java v2 aws/aws-sdk-java-v2#3343

[scottgerring]

@msailes do you have any insight into the priority of this?

[scottgerring]

If we decide we want to do it sync, we could do all of the geting inside of PT itself, rather than exposing futures out. That way we don't change interface to migrate to a future sync interface in the CRT later.

[msailes]

I think that would be the best approach.

[scottgerring]

Hey @jreijn , are you still interested on working on this? If not no stress at all - I will see if we can get someone else to pick it up - we've ramped up work on v2 in the last month and this will be an important bit in the middle.

[jreijn]

@scottgerring yes I am! I'll try to synch with the work that has been done for v2 and come up with some ideas so we can discuss solutions. WDYT?

[jeromevdl]

Sounds good, let us know if you have question, feel free to write down your first ideas here in this issue. You should start with the Large Message module as it's not currently refactored (parameters, idempotency are).

[jreijn]

I've started looking at the large messages module as @jeromevdl suggested and and while trying to support the async crt client I already hit a bump which probably needs to get fixed first before moving forward. The large messages module uses an amazon library for payload offloading. It leverages s3, but only supports a synch client as far as I can see. See awslabs/payload-offloading-java-common-lib-for-aws#17

[scottgerring]

Hey @jreijn I think best case here we get that PR merged into upstream. I will chase it up today!

[scottgerring]

Hey @jreijn i've got the attention of the maintainers internally to try and push this and will hopefully have some news on this soon. The PR itself needs to be rebased onto the current main of the repo so its a bit more effort than initially expected.

I am torn between working on this - updating the PR on payload-offloading-java-common-lib-for-aws and finishing my open branch on #1402 . This is close to completion and is also a module that uses the SDK and could be used to prove out the path with the CRT while we wait for the payload offloading library.

@jeromevdl / @jreijn - what do you both think?

[scottgerring]

Since writing the previous comment i've got a new PR into the payload offloading library using Richard's changes with the latest there from master. I've not tested it, i've just done the mechanical process of resolving the merge issues and fixing the tests, but I will see if we can get some eyes on it over there, and move back onto the parameters merge.

awslabs/payload-offloading-java-common-lib-for-aws#50

[jreijn]

Most of the current modules provide some sort of Config or Manager object that supports providing a custom client.

See for instance (

powertools-lambda-java/powertools-large-messages/src/main/java/software/amazon/lambda/powertools/largemessages/LargeMessageConfig.java

Line 62 in 00d04c3

public void withS3Client(S3Client s3Client) {

)

I'm wondering if we want to switch all these methods over to only providing an async client or do we want to support both? What are your thoughts on this?

[scottgerring]

My gut feeling is we should use the CRT everywhere and ditch the sync clients; we should encourage best practices, and this would also reduce complexity in the codebase & interface (vs supporting both).

@msailes @jeromevdl what do you both think - ?

[msailes]

The CRT client is the fastest and lowest memory HTTP client. If we can incorporate it into Powertools with a good DX / API I think we absolutely should.

[scottgerring]

Good morning @jreijn; now that we've merged #1402 into v2, we have a reasonable sized chunk of code that uses the AWS SDK clients you could use as a testbed, instead of the large message handling !

[msailes]

aws/aws-sdk-java-v2#3343 (comment)

Good timing

[jreijn]

@msailes @scottgerring @jeromevdl this raises an interesting question. Do we still want to go async as the sync client can be used as is I guess.

[jeromevdl]

Now that sync is available I think it's easier than playing with Future everywhere. @scottgerring agree?

[scottgerring]

My personal gut feeling is sync makes more sense for PT now that we have it; general use case for Lambda workloads is unlikely to be firing up a pile of async work and waiting on it, and the ergonomics of sync are going to be better 💘

[scottgerring]

In #1541 we decided that the CRT will not be the default client in v2. This is because the default configuration of the client leads to a larger Lambda size, which decreases startup time. It is possible to configure our Lambda build to avoid this, but as it is more complex for the user we will document it as a optimization rather than making it the default getting-started experience.

[scottgerring]

As part of #1092 we have updated the documentation in v2 to reflect the above - CRT client will not be the default for the moment, but we now clearly document how to set it up.