Skip to content

Improve logging and error handling in benchmark script #507

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 15, 2023
Merged

Improve logging and error handling in benchmark script #507

merged 1 commit into from
Sep 15, 2023

Conversation

dannycjones
Copy link
Contributor

Description of change

I've been running some benchmarks on my own EC2 instance and found the output messy (since fio is rewriting stdout) and also it failed to exit when jq wasn't working. This change ensures the script exits (unless we want to check for exit code), and cleans up the script output a bit.

It also retains intermediary files since we don't need to delete them. They are ignored by the benchmark job in the end.

Does this change impact existing behavior?

No.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and I agree to the terms of the Developer Certificate of Origin (DCO).

@dannycjones
Improve logging and error handling in benchmark script
7434dbc 
Signed-off-by: Daniel Carl Jones <djonesoa@amazon.com>
@dannycjones dannycjones added the performance PRs to run benchmarks on label Sep 7, 2023
@dannycjones dannycjones temporarily deployed to PR integration tests September 7, 2023 14:32 — with GitHub Actions Inactive
@dannycjones dannycjones temporarily deployed to PR benchmarks September 7, 2023 14:32 — with GitHub Actions Inactive
@dannycjones dannycjones temporarily deployed to PR integration tests September 7, 2023 14:32 — with GitHub Actions Inactive
@dannycjones dannycjones temporarily deployed to PR integration tests September 7, 2023 14:32 — with GitHub Actions Inactive
@dannycjones dannycjones temporarily deployed to PR integration tests September 7, 2023 14:32 — with GitHub Actions Inactive
sauraank
sauraank reviewed Sep 14, 2023
View reviewed changes
@dannycjones dannycjones temporarily deployed to PR benchmarks September 15, 2023 09:05 — with GitHub Actions Inactive
@dannycjones dannycjones temporarily deployed to PR benchmarks September 15, 2023 09:05 — with GitHub Actions Inactive
@dannycjones dannycjones marked this pull request as ready for review September 15, 2023 09:06
@jamesbornholt jamesbornholt added this pull request to the merge queue Sep 15, 2023
Merged via the queue into awslabs:main with commit 171c420 Sep 15, 2023
@dannycjones dannycjones deleted the benchmark-script-changes branch September 15, 2023 15:14
github-merge-queue bot pushed a commit that referenced this pull request Apr 1, 2025
@passaro
Update CRT submodules to latest releases (#1338)
c52ab15 
Update the CRT libraries to the latest releases. Notable changes:
* Update endpoints.
([awslabs/aws-c-s3#502](awslabs/aws-c-s3#502))
* Bump Default Memory Limit for Higher Target Throughput.
([awslabs/aws-c-s3#499](awslabs/aws-c-s3#499))


<details>
  <summary>Full CRT changelog:</summary>
  
```
Submodule mountpoint-s3-crt-sys/crt/aws-c-auth 01dd06ac..cd9d6afc:
  > Update docs for DefaultChain (#266)
  > Async cognito support (#267)
  > only forbid `X-Amz-S3session-Token` when signing with s3 express. (#268)
Submodule mountpoint-s3-crt-sys/crt/aws-c-cal 5d5bc553..4805a96e:
  > Fix FindCrypto behavior on win (#211)
  > Fix module export to respect ed25519 flag (#210)
  > Fix missed define in the code (#209)
Submodule mountpoint-s3-crt-sys/crt/aws-c-common 7fb0071a..8ae8f48e:
  > Simplify how inline math files are included (#1195)
  > Tests require compiler extensions (#1193)
  > CrossProcess lock -- don't unlock, just close fd (#1192)
Submodule mountpoint-s3-crt-sys/crt/aws-c-http 60c43f80..e526ac33:
  > Apple Network Framework Support (#502)
  > h1_decoder error on multiple content-length headers (#509)
  > Fix Error Handling For Connection Manager (#507)
  > HTTP/1: Support streaming requests of unknown length (#506)
Submodule mountpoint-s3-crt-sys/crt/aws-c-io 318f7e57..6c90e491:
  > Remove unused variables in aws_host_resolver (#719)
  > Grand dispatch queue (#661)
  > Fix IP address being labelled "bad" for too long (#718)
  > Add back kqueue support on iOS (#716)
Submodule mountpoint-s3-crt-sys/crt/aws-c-s3 1d0091c7..408e9c90:
  > Update endpoints (#502)
  > Newer URL for aws-lc (#500)
  > Bump Default Memory Limit for Higher Target Throughput (#499)
  > missed one file from test helper README (#498)
Submodule mountpoint-s3-crt-sys/crt/aws-checksums fb8bd0b8..66b447c0:
  > Add missing extern c to new header (#103)
  > Add init functions to support thread safe init of impls (#102)
Submodule mountpoint-s3-crt-sys/crt/aws-lc 7bca7e96..b1420f27:
  > Prepare for v1.49.1 (#2303)
  > Turn on better logging for EC2 test framework (#2298)
  > Add req to OpenSSL CLI tool (#2284)
  > Add more build options to match callback build (#2279)
  > FIPS Integrity Hash Tooling (#2296)
  > Prepare for v1.49.0 (#2297)
  > Cherrypick hardening DSA param checks from BoringSSL  (#2293)
  > Bump mysql CI to 9.2.0 (#2161)
  > AES: Add function pointer trampoline to avoid delocator issue (#2294)
  > Adding detection of out-of-bound pre-bound memory read to AES-XTS tests. (#2286)
  > Wire-up rust-openssl into GitHub CI (for the time being) (#2291)
  > Add support for more SSL BIO functions (#2273)
  > Add support for verifying PKCS7 signed attributes (#2264)
  > Reject DSA trailing garbage in EVP layer, add test cases (#2289)
  > Update patches in Ruby CI (#2233)
  > Documentation on service indicator (#2281)
  > Add the rehash utility to the openssl CLI tool (#2258)
  > Revert "Allow constructed strings in BER parsing (#2015)" (#2278)
  > Prepare AWS-LC v1.48.5 (#2274)
  > s2n-bignum build should use boringssl_prefix_symbols_asm.h (#2265)
  > ci: Nix flake and devShell (#2189)
  > GitHub CI job to verify tags are on expected branches (#2170)
  > Prepare for release v.1.48.4 (#2271)
  > Make AWS_LC_fips_failure_callback optional in builds with AWSLC_FIPS_FAILURE_CALLBACK (#2266)
  > Prepare v1.48.3 (#2269)
  > Update shard_gtest to unset environment variables once all the tests are done (#2270)
  > Minor build fixes for CMake and libssl on x86 (#2267)
  > Fix aws-lc-rs CI test (again) (#2268)
  > Add x4 batched SHAKE128 and SHAKE256 APIs (#2247)
  > Fix aws-lc-rs CI test when symbols removed (#2262)
  > Remove no-op register move from ChaCha20_ctr32_ssse3_4x (#2234)
  > Revert removal of "PEM_X509_INFO_write_bio" (#2226)
  > Use unsigned return type for BN_get_minimal_width and word size tests (#2260)
```
</details>

### Does this change impact existing behavior?

No.

### Does this change need a changelog entry? Does it require a version
change?

Yes. Updated as required.

---

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and I agree to the terms of
the [Developer Certificate of Origin
(DCO)](https://developercertificate.org/).

---------

Signed-off-by: Alessandro Passaro <alexpax@amazon.co.uk>
mansi153 pushed a commit to mansi153/mountpoint-s3 that referenced this pull request Jul 24, 2025
@passaro @mansi153
Update CRT submodules to latest releases (awslabs#1338)
ae06df9 
Update the CRT libraries to the latest releases. Notable changes:
* Update endpoints.
([awslabs/aws-c-s3#502](awslabs/aws-c-s3#502))
* Bump Default Memory Limit for Higher Target Throughput.
([awslabs/aws-c-s3#499](awslabs/aws-c-s3#499))


<details>
  <summary>Full CRT changelog:</summary>
  
```
Submodule mountpoint-s3-crt-sys/crt/aws-c-auth 01dd06ac..cd9d6afc:
  > Update docs for DefaultChain (awslabs#266)
  > Async cognito support (awslabs#267)
  > only forbid `X-Amz-S3session-Token` when signing with s3 express. (awslabs#268)
Submodule mountpoint-s3-crt-sys/crt/aws-c-cal 5d5bc553..4805a96e:
  > Fix FindCrypto behavior on win (awslabs#211)
  > Fix module export to respect ed25519 flag (awslabs#210)
  > Fix missed define in the code (awslabs#209)
Submodule mountpoint-s3-crt-sys/crt/aws-c-common 7fb0071a..8ae8f48e:
  > Simplify how inline math files are included (awslabs#1195)
  > Tests require compiler extensions (awslabs#1193)
  > CrossProcess lock -- don't unlock, just close fd (awslabs#1192)
Submodule mountpoint-s3-crt-sys/crt/aws-c-http 60c43f80..e526ac33:
  > Apple Network Framework Support (awslabs#502)
  > h1_decoder error on multiple content-length headers (awslabs#509)
  > Fix Error Handling For Connection Manager (awslabs#507)
  > HTTP/1: Support streaming requests of unknown length (awslabs#506)
Submodule mountpoint-s3-crt-sys/crt/aws-c-io 318f7e57..6c90e491:
  > Remove unused variables in aws_host_resolver (awslabs#719)
  > Grand dispatch queue (awslabs#661)
  > Fix IP address being labelled "bad" for too long (awslabs#718)
  > Add back kqueue support on iOS (awslabs#716)
Submodule mountpoint-s3-crt-sys/crt/aws-c-s3 1d0091c7..408e9c90:
  > Update endpoints (awslabs#502)
  > Newer URL for aws-lc (awslabs#500)
  > Bump Default Memory Limit for Higher Target Throughput (awslabs#499)
  > missed one file from test helper README (awslabs#498)
Submodule mountpoint-s3-crt-sys/crt/aws-checksums fb8bd0b8..66b447c0:
  > Add missing extern c to new header (awslabs#103)
  > Add init functions to support thread safe init of impls (awslabs#102)
Submodule mountpoint-s3-crt-sys/crt/aws-lc 7bca7e96..b1420f27:
  > Prepare for v1.49.1 (#2303)
  > Turn on better logging for EC2 test framework (#2298)
  > Add req to OpenSSL CLI tool (#2284)
  > Add more build options to match callback build (#2279)
  > FIPS Integrity Hash Tooling (#2296)
  > Prepare for v1.49.0 (#2297)
  > Cherrypick hardening DSA param checks from BoringSSL  (#2293)
  > Bump mysql CI to 9.2.0 (#2161)
  > AES: Add function pointer trampoline to avoid delocator issue (#2294)
  > Adding detection of out-of-bound pre-bound memory read to AES-XTS tests. (#2286)
  > Wire-up rust-openssl into GitHub CI (for the time being) (#2291)
  > Add support for more SSL BIO functions (#2273)
  > Add support for verifying PKCS7 signed attributes (#2264)
  > Reject DSA trailing garbage in EVP layer, add test cases (#2289)
  > Update patches in Ruby CI (#2233)
  > Documentation on service indicator (#2281)
  > Add the rehash utility to the openssl CLI tool (#2258)
  > Revert "Allow constructed strings in BER parsing (#2015)" (#2278)
  > Prepare AWS-LC v1.48.5 (#2274)
  > s2n-bignum build should use boringssl_prefix_symbols_asm.h (#2265)
  > ci: Nix flake and devShell (#2189)
  > GitHub CI job to verify tags are on expected branches (#2170)
  > Prepare for release v.1.48.4 (#2271)
  > Make AWS_LC_fips_failure_callback optional in builds with AWSLC_FIPS_FAILURE_CALLBACK (#2266)
  > Prepare v1.48.3 (#2269)
  > Update shard_gtest to unset environment variables once all the tests are done (#2270)
  > Minor build fixes for CMake and libssl on x86 (#2267)
  > Fix aws-lc-rs CI test (again) (#2268)
  > Add x4 batched SHAKE128 and SHAKE256 APIs (#2247)
  > Fix aws-lc-rs CI test when symbols removed (#2262)
  > Remove no-op register move from ChaCha20_ctr32_ssse3_4x (#2234)
  > Revert removal of "PEM_X509_INFO_write_bio" (#2226)
  > Use unsigned return type for BN_get_minimal_width and word size tests (#2260)
```
</details>

### Does this change impact existing behavior?

No.

### Does this change need a changelog entry? Does it require a version
change?

Yes. Updated as required.

---

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and I agree to the terms of
the [Developer Certificate of Origin
(DCO)](https://developercertificate.org/).

---------

Signed-off-by: Alessandro Passaro <alexpax@amazon.co.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jamesbornholt jamesbornholt jamesbornholt approved these changes

@sauraank sauraank Awaiting requested review from sauraank

Assignees
No one assigned
Labels
performance PRs to run benchmarks on
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dannycjones @jamesbornholt @sauraank