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

Enable connection pooling #49

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

Conversation

abhinav-upadhyay
Copy link
Contributor

Saves a number of system calls because of connection reuse, below is a
comparison of syscall counts - without vs with connection pool for calling
send() 1000 times (with 10,000 dps in each call)

Without connection pool:

  sigaltstack                                                       2
  _lwp_self                                                        33
  sigprocmask                                                      66
  connect                                                         965
  getsockopt                                                      965
  recvfrom                                                        965
  setsockopt                                                      965
  socket                                                          965
  issetugid                                                      1930
  stat                                                           1930
  sendto                                                         1943
  fcntl                                                          2895
  fstat                                                          3860
  poll                                                           3873
  close                                                          4826
  open                                                           5790
  mmap                                                          14139
  munmap                                                        14143

With connection pool:

  sigaltstack                                                       2
  _lwp_self                                                        33
  sigprocmask                                                      66
  fcntl                                                           959
  recvfrom                                                        959
  issetugid                                                      1916
  sendto                                                         1916
  stat                                                           1916
  fstat                                                          3832
  poll                                                           3833
  close                                                          3834
  open                                                           5748
  mmap                                                          12851
  munmap                                                        12856
  getpeername                                                       1
  getsockname                                                       1

Saves a number of system calls because of connection reuse, below is a
comparison of syscall counts - without vs with connection pool for calling
send() 1000 times (with 10000 dps in each call)

Without connection pool:
  sigaltstack                                                       2
  _lwp_self                                                        33
  sigprocmask                                                      66
  connect                                                         965
  getsockopt                                                      965
  recvfrom                                                        965
  setsockopt                                                      965
  socket                                                          965
  issetugid                                                      1930
  stat                                                           1930
  sendto                                                         1943
  fcntl                                                          2895
  fstat                                                          3860
  poll                                                           3873
  close                                                          4826
  open                                                           5790
  mmap                                                          14139
  munmap                                                        14143

With connection pool:
  sigaltstack                                                       2
  _lwp_self                                                        33
  sigprocmask                                                      66
  fcntl                                                           959
  recvfrom                                                        959
  issetugid                                                      1916
  sendto                                                         1916
  stat                                                           1916
  fstat                                                          3832
  poll                                                           3833
  close                                                          3834
  open                                                           5748
  mmap                                                          12851
  munmap                                                        12856
  getpeername                                                       1
  getsockname                                                       1
@codecov
Copy link

codecov bot commented Feb 19, 2019

Codecov Report

Merging #49 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #49      +/-   ##
==========================================
+ Coverage   99.71%   99.71%   +<.01%     
==========================================
  Files          11       11              
  Lines        1398     1399       +1     
==========================================
+ Hits         1394     1395       +1     
  Misses          4        4
Impacted Files Coverage Δ
tests/test_send.py 99.69% <100%> (ø) ⬆️
apptuit/apptuit_client.py 99.06% <100%> (ø) ⬆️
apptuit/utils.py 100% <100%> (ø) ⬆️
tests/test_query.py 100% <100%> (ø) ⬆️
tests/test_pyformance_reporter.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c5e3c2e...9c5290a. Read the comment docs.

@codecov
Copy link

codecov bot commented Feb 19, 2019

Codecov Report

Merging #49 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #49      +/-   ##
==========================================
+ Coverage   99.66%   99.66%   +<.01%     
==========================================
  Files          12       12              
  Lines        1482     1483       +1     
==========================================
+ Hits         1477     1478       +1     
  Misses          5        5
Impacted Files Coverage Δ
tests/test_send.py 99.69% <100%> (ø) ⬆️
apptuit/apptuit_client.py 99.06% <100%> (ø) ⬆️
apptuit/utils.py 100% <100%> (ø) ⬆️
tests/test_query.py 100% <100%> (ø) ⬆️
tests/test_pyformance_reporter.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cca2ac8...d9d019c. Read the comment docs.

@abhinav-upadhyay
Copy link
Contributor Author

The requests session needs to be closed as well, otherwise we run risk of leaving open sockets behind. The recommended practice in Python world is to implement the context manager protocol to ensure cleanup.

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