Skip to content

Commit

Permalink
Fixed bug where using profile-based credentials could cause the SDK t…
Browse files Browse the repository at this point in the history
…o read the profile file with each request. (#3950)

This bug was introduced with the profile file supplier changes. If the customer was not overriding the profile file via the client override config AND was relying on profile file-based configuration, that configuration would be read with every request. This had a significant performance cost.

```
Benchmarking Results (software.amazon.awssdk.benchmark.apicall.protocol.JsonProtocolBenchmark)
Before:   2858.805 ± 181.491  ops/s
After:    10915.079 ± 677.022  ops/s
```
  • Loading branch information
millems authored Apr 25, 2023
1 parent 34d0270 commit 8e55ccc
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 3 deletions.
6 changes: 6 additions & 0 deletions .changes/next-release/bugfix-AWSSDKforJavav2-c9734ed.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"category": "AWS SDK for Java v2",
"contributor": "",
"type": "bugfix",
"description": "Fixed bug where using profile-based credentials could cause the SDK to read the profile file with each request."
}
Original file line number Diff line number Diff line change
Expand Up @@ -264,9 +264,9 @@ protected SdkClientConfiguration mergeChildDefaults(SdkClientConfiguration confi
*/
private SdkClientConfiguration mergeGlobalDefaults(SdkClientConfiguration configuration) {
// Don't load the default profile file if the customer already gave us one.
Supplier<ProfileFile> configuredProfileFileSupplier = configuration.option(PROFILE_FILE_SUPPLIER);
Supplier<ProfileFile> profileFileSupplier = Optional.ofNullable(configuredProfileFileSupplier)
.orElseGet(() -> ProfileFile::defaultProfileFile);
Supplier<ProfileFile> profileFileSupplier =
Optional.ofNullable(configuration.option(PROFILE_FILE_SUPPLIER))
.orElseGet(() -> ProfileFileSupplier.fixedProfileFile(ProfileFile.defaultProfileFile()));

return configuration.merge(c -> c.option(EXECUTION_INTERCEPTORS, new ArrayList<>())
.option(ADDITIONAL_HTTP_HEADERS, new LinkedHashMap<>())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.function.Supplier;
import org.assertj.core.api.Assertions;
import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -371,7 +372,19 @@ public void clientBuilderFieldsHaveBeanEquivalents() throws Exception {
assertThat(property.getWriteMethod()).as(propertyName + " setter").isNotNull();
});
});
}


@Test
public void defaultProfileFileSupplier_isStaticOrHasIdentityCaching() {
SdkClientConfiguration config =
testClientBuilder().build().clientConfiguration;

Supplier<ProfileFile> defaultProfileFileSupplier = config.option(PROFILE_FILE_SUPPLIER);
ProfileFile firstGet = defaultProfileFileSupplier.get();
ProfileFile secondGet = defaultProfileFileSupplier.get();

assertThat(secondGet).isSameAs(firstGet);
}

private SdkDefaultClientBuilder<TestClientBuilder, TestClient> testClientBuilder() {
Expand Down

0 comments on commit 8e55ccc

Please sign in to comment.