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

query by pool instead of using the connection directly #804

Closed
wants to merge 2 commits into from

Conversation

ericdum
Copy link
Contributor

@ericdum ericdum commented May 17, 2021

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Proposed changes

Checklist

  • I have read the contribution guidelines
  • For non-bugfix PRs, I have discussed this change on the forum/slack team.
  • I have run grunt to verify the unit tests pass
  • I have added suitable unit tests to cover the new/changed functionality

@coveralls
Copy link

coveralls commented May 17, 2021

Coverage Status

Coverage remained the same at 49.315% when pulling 5c36b4b on ericdum:master into 0d1a41f on node-red:master.

@ericdum
Copy link
Contributor Author

ericdum commented May 17, 2021

This PR is intended to increase the performance of Concurrent Request and to resolve some errors like CONNREST.
To do that by let MySQL Pool manage the connections instead of manually manage connections.

@dceejay
Copy link
Member

dceejay commented May 17, 2021

Hi,

thanks for this - a couple of points.. 1) are you able to accept/click/sign the CLA so we can merge it ?
and 2) does this handle connection errors OK - There has been along standing issue of connections seeming to timeout and/or drop over a long time - and not reconnect... Does this handle this somehow ? I can' see any error handler for he pool ?

@ericdum
Copy link
Contributor Author

ericdum commented May 18, 2021

Hi @dceejay

  1. I have been signed the CLA, but the checking program seems not to recheck. I don't know how to make it work.
  2. After switching the management to the pool, most of the errors will be handled by the pool also. So, there's only error left to the program is pool.query(err, ....), so I have already handled it.

And FYI, there's some benchmark result below.
Before optimize: ab -n 500 -c 30

Document Path:          /data-flow/api/benchmark
Document Length:        13 bytes

Concurrency Level:      30
Time taken for tests:   13.528 seconds
Complete requests:      500
Failed requests:        0
Total transferred:      138500 bytes
HTML transferred:       6500 bytes
Requests per second:    36.96 [#/sec] (mean)
Time per request:       811.655 [ms] (mean)
Time per request:       27.055 [ms] (mean, across all concurrent requests)
Transfer rate:          10.00 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:       55  693 2318.4     88    9888
Processing:    59  102  34.5     93     204
Waiting:       59  102  34.5     93     203
Total:        117  795 2316.3    189    9995

Percentage of the requests served within a certain time (ms)
  50%    189
  66%    210
  75%    239
  80%    271
  90%    500
  95%   9902
  98%   9994
  99%   9994
 100%   9995 (longest request)

After optimize: ab -n 500 -c 30

Document Path:          /data-flow/api/benchmark
Document Length:        13 bytes

Concurrency Level:      30
Time taken for tests:   0.477 seconds
Complete requests:      500
Failed requests:        0
Total transferred:      138500 bytes
HTML transferred:       6500 bytes
Requests per second:    1048.70 [#/sec] (mean)
Time per request:       28.607 [ms] (mean)
Time per request:       0.954 [ms] (mean, across all concurrent requests)
Transfer rate:          283.68 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        8   12   1.6     12      17
Processing:     9   14   2.4     14      21
Waiting:        9   14   2.4     14      21
Total:         19   26   2.8     26      34

Percentage of the requests served within a certain time (ms)
  50%     26
  66%     27
  75%     28
  80%     29
  90%     30
  95%     32
  98%     32
  99%     33
 100%     34 (longest request)

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