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

switch to case-insensitive http header comparisons, also fix racing in linux.jai #2

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

DaseinPhaos
Copy link

Thanks for the awesome package!

This PR fixes a tiny issue wrt http header comparisons. According to the mdn reference:

An HTTP header consists of its case-insensitive name followed by a colon...

But the previous parsing code used hard-coded literals when parsing specific headers, resulting in some requests not being correctly parsed. This PR fixes that.

@DaseinPhaos DaseinPhaos changed the title switch to case-insensitive http header comparisons switch to case-insensitive http header comparisons, also fix racing in linux.jai Aug 14, 2024
@DaseinPhaos
Copy link
Author

Also, the crashing issue you mentioned might be related to the racing issue when accessing thread_pool: the built-in Hash_Table is not thread safe and thus each pool thread should be assigned its own hash table to avoid racing. This PR also did that.

However, I don't have a beefy ubuntu server with a lot of threads to properly test it.

Testing with THREAD_POOL_COUNT=2 and wrk --latency -t6 -c10000 -d1 http://localhost/bench results in the following result, which I'm not sure counts as a successful fix or not:

Running 1s test @ http://localhost/bench
  6 threads and 10000 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    18.74ms   64.24ms 662.23ms   91.66%
    Req/Sec     9.16k     5.14k   28.67k    74.00%
  Latency Distribution
     50%   27.00us
     75%   38.00us
     90%   17.72ms
     99%  356.75ms
  47328 requests in 1.10s, 4.38MB read
  Socket errors: connect 8981, read 0, write 0, timeout 0
  Non-2xx or 3xx responses: 47328
Requests/sec:  42991.22
Transfer/sec:      3.98MB

But it's certainly not crashing anymore!

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.

1 participant