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

Bypass for CreateSession reqeust #384

Merged
merged 8 commits into from
Nov 29, 2023
Merged

Bypass for CreateSession reqeust #384

merged 8 commits into from
Nov 29, 2023

Conversation

TingDaoK
Copy link
Contributor

@TingDaoK TingDaoK commented Nov 29, 2023

Issue #, if available:

  • Fix the issue where too many request being prepared just waiting to be signed, while we need to make another CreateSession request from the same client to sign the requests.
  • So, the requests will block the CreateSession to be made and hang

Description of changes:

  • Bypass the throttle on client for CreateSession request, so that it will always get scheduled to be sent

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@TingDaoK TingDaoK changed the title Bypass create session Bypass create session reqeust Nov 29, 2023
@codecov-commenter
Copy link

codecov-commenter commented Nov 29, 2023

Codecov Report

Merging #384 (fcda93b) into main (ecd0143) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #384      +/-   ##
==========================================
+ Coverage   88.58%   88.59%   +0.01%     
==========================================
  Files          21       21              
  Lines        5938     5944       +6     
==========================================
+ Hits         5260     5266       +6     
  Misses        678      678              
Files Coverage Δ
source/s3_client.c 88.01% <100.00%> (+0.05%) ⬆️
source/s3express_credentials_provider.c 92.23% <100.00%> (+0.01%) ⬆️

Copy link
Contributor

@waahm7 waahm7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bunch of trivial comments; feel free to merge and we can agonize over names later, maybe.

source/s3express_credentials_provider.c Outdated Show resolved Hide resolved
source/s3_client.c Outdated Show resolved Hide resolved
tests/s3_s3express_client_test.c Outdated Show resolved Hide resolved
tests/CMakeLists.txt Outdated Show resolved Hide resolved
source/s3_client.c Outdated Show resolved Hide resolved
tests/s3_data_plane_tests.c Outdated Show resolved Hide resolved
tests/s3_s3express_client_test.c Outdated Show resolved Hide resolved
source/s3_client.c Outdated Show resolved Hide resolved
source/s3_client.c Show resolved Hide resolved
source/s3_client.c Show resolved Hide resolved
@TingDaoK TingDaoK changed the title Bypass create session reqeust Bypass for CreateSession reqeust Nov 29, 2023
@TingDaoK TingDaoK enabled auto-merge (squash) November 29, 2023 23:10
@TingDaoK TingDaoK merged commit de36fee into main Nov 29, 2023
30 checks passed
@TingDaoK TingDaoK deleted the bypass_create_session branch November 29, 2023 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants