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

Use concurrent hashmap for headers in netty request #2911

Conversation

justinlin-linkedin
Copy link
Collaborator

@justinlin-linkedin justinlin-linkedin commented Oct 3, 2024

Summary

We are not ordering the headers and query parameters in netty request, just use HashMap, not TreeMap. And since we are now modifying the map from multiple threads due to 100-continue, we should use concurrenthashmap.

Test

Unit test

@codecov-commenter
Copy link

codecov-commenter commented Oct 3, 2024

Codecov Report

Attention: Patch coverage is 22.22222% with 7 lines in your changes missing coverage. Please review.

Project coverage is 70.10%. Comparing base (52ba813) to head (9064b07).
Report is 112 commits behind head on master.

Files with missing lines Patch % Lines
.../frontend/s3/S3MultipartCompleteUploadHandler.java 0.00% 1 Missing and 1 partial ⚠️
...mbry/frontend/s3/S3MultipartUploadPartHandler.java 0.00% 1 Missing and 1 partial ⚠️
...ava/com/github/ambry/frontend/s3/S3PutHandler.java 0.00% 1 Missing and 1 partial ⚠️
.../main/java/com/github/ambry/rest/NettyRequest.java 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #2911      +/-   ##
============================================
+ Coverage     64.24%   70.10%   +5.85%     
- Complexity    10398    11908    +1510     
============================================
  Files           840      863      +23     
  Lines         71755    73375    +1620     
  Branches       8611     8838     +227     
============================================
+ Hits          46099    51438    +5339     
+ Misses        23004    19236    -3768     
- Partials       2652     2701      +49     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@justinlin-linkedin justinlin-linkedin changed the title Use hashmap for headers in netty request Use concurrent hashmap for headers in netty request Oct 4, 2024
@justinlin-linkedin justinlin-linkedin merged commit a546a59 into linkedin:master Oct 4, 2024
5 checks passed
@justinlin-linkedin justinlin-linkedin deleted the justin/asyncrequestworker branch October 4, 2024 03:55
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.

3 participants