Skip to content

Add independent modsecFinishConnection API that allows you to independen... #620

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

Conversation

thehelix112
Copy link

This adds an independent API call to free the connection allocations, independently from the request objects. Additionally updates the existing modsecFinishRequest to only free up the request's allocations.

This should facilitate better memory management by user libraries (though this is not in this pull request) to not needlessly reallocate connection objects on a per-request basis.

I tested that this builds properly using nginx stable (1.4.4).

        objs/addon/modsecurity/ngx_http_modsecurity.o \
        objs/addon/modsecurity/apr_bucket_nginx.o \
        objs/addon/modsecurity/ngx_pool_context.o \
        objs/ngx_modules.o \
        -lpthread -lcrypt ../ModSecurity/nginx/modsecurity/../../standalone/.libs/standalone.a -L/usr/lib -lapr-1 -L/usr/lib -laprutil-1 -L/usr/lib/x86_64-linux-gnu -lpcre -L/usr/lib/x86_64-linux-gnu -lxml2 -llua5.1 -lpcre -lcrypto -lcrypto -lz -lfuzzy
make[1]: Leaving directory `~/Work/dev/git/nginx-1.4.4'
make -f objs/Makefile manpage
make[1]: Entering directory `~/Work/dev/git/nginx-1.4.4'
sed -e "s|%%PREFIX%%|/usr/local/nginx|" \
                -e "s|%%PID_PATH%%|/usr/local/nginx/logs/nginx.pid|" \
                -e "s|%%CONF_PATH%%|/usr/local/nginx/conf/nginx.conf|" \
                -e "s|%%ERROR_LOG_PATH%%|/usr/local/nginx/logs/error.log|" \
                < man/nginx.8 > objs/nginx.8
make[1]: Leaving directory `~/Work/dev/git/nginx-1.4.4'
~/Work/dev/git/nginx-1.4.4$

…dently destroy the connection and request pools. This is to facilitate reuse of a connection for multiple requests.
@thehelix112
Copy link
Author

Hey Filipe,

Any progress with this pull-request? Is there anything that I need to address or improve?
Thanks,

Dave

@zimmerle
Copy link
Contributor

zimmerle commented Mar 3, 2014

Hi @thehelix112 thanks for the patch, merged!

It seems that we have a performance improvement:

Without the patch

(zimmerle@zlinux)-(~/core/nginx-1.4.3)$ ab -k -c 100 -n 80000 http://localhost:8082/
This is ApacheBench, Version 2.3 <$Revision: 1430300 $>
Copyright 1996 Adam Twiss, Zeus Technology Ltd, http://www.zeustech.net/
Licensed to The Apache Software Foundation, http://www.apache.org/

Benchmarking localhost (be patient)
Completed 8000 requests
Completed 16000 requests
Completed 24000 requests
Completed 32000 requests
Completed 40000 requests
Completed 48000 requests
Completed 56000 requests
Completed 64000 requests
Completed 72000 requests
Completed 80000 requests
Finished 80000 requests


Server Software:        nginx
Server Hostname:        localhost
Server Port:            8082

Document Path:          /
Document Length:        612 bytes

Concurrency Level:      100
Time taken for tests:   325.970 seconds
Complete requests:      80000
Failed requests:        0
Write errors:           0
Keep-Alive requests:    79201
Total transferred:      69276005 bytes
HTML transferred:       48960000 bytes
Requests per second:    245.42 [#/sec] (mean)
Time per request:       407.463 [ms] (mean)
Time per request:       4.075 [ms] (mean, across all concurrent requests)
Transfer rate:          207.54 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   0.2      0       7
Processing:     3  407 3923.2      4   41547
Waiting:        3  407 3923.2      4   41547
Total:          3  407 3923.2      4   41547

Percentage of the requests served within a certain time (ms)
  50%      4
  66%      4
  75%      4
  80%      4
  90%      4
  95%      5
  98%      5
  99%  19939
 100%  41547 (longest request)

With the patch

(zimmerle@zlinux)-(~/core/nginx-1.4.3)$ ab -k -c 100 -n 80000 http://localhost:8082/
This is ApacheBench, Version 2.3 <$Revision: 1430300 $>
Copyright 1996 Adam Twiss, Zeus Technology Ltd, http://www.zeustech.net/
Licensed to The Apache Software Foundation, http://www.apache.org/

Benchmarking localhost (be patient)
Completed 8000 requests
Completed 16000 requests
Completed 24000 requests
Completed 32000 requests
Completed 40000 requests
Completed 48000 requests
Completed 56000 requests
Completed 64000 requests
Completed 72000 requests
Completed 80000 requests
Finished 80000 requests


Server Software:        nginx
Server Hostname:        localhost
Server Port:            8082

Document Path:          /
Document Length:        612 bytes

Concurrency Level:      100
Time taken for tests:   317.006 seconds
Complete requests:      80000
Failed requests:        0
Write errors:           0
Keep-Alive requests:    79201
Total transferred:      69276005 bytes
HTML transferred:       48960000 bytes
Requests per second:    252.36 [#/sec] (mean)
Time per request:       396.257 [ms] (mean)
Time per request:       3.963 [ms] (mean, across all concurrent requests)
Transfer rate:          213.41 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   0.1      0       5
Processing:     3  396 3813.8      4   40503
Waiting:        3  396 3813.8      4   40503
Total:          3  396 3813.8      4   40503

Percentage of the requests served within a certain time (ms)
  50%      4
  66%      4
  75%      4
  80%      4
  90%      4
  95%      4
  98%      5
  99%  19564
 100%  40503 (longest request)  

@zimmerle zimmerle closed this Mar 3, 2014
@zimmerle zimmerle added this to the testing milestones milestone Mar 27, 2014
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.

2 participants