Skip to content

Reuse TCP connections#149

Merged
f2prateek merged 1 commit into
masterfrom
reuse-tcp-connections-off-master
Jan 11, 2018
Merged

Reuse TCP connections#149
f2prateek merged 1 commit into
masterfrom
reuse-tcp-connections-off-master

Conversation

@rohitpaulk

@rohitpaulk rohitpaulk commented Jan 10, 2018

Copy link
Copy Markdown
Collaborator

Fixes #148.

@rohitpaulk rohitpaulk force-pushed the reuse-tcp-connections-off-master branch from b725454 to 6b203ad Compare January 10, 2018 10:18
@rohitpaulk rohitpaulk mentioned this pull request Jan 10, 2018
Closed
@rohitpaulk

rohitpaulk commented Jan 10, 2018

Copy link
Copy Markdown
Collaborator Author

This took me longer than expected, the ruby API was a bit unintuitive.

Wireshark screenshots of this working:

Before:

screen shot 2018-01-10 at 2 55 16 pm

After:

screen shot 2018-01-10 at 3 36 13 pm

@rohitpaulk rohitpaulk changed the title Reuse TCP connections [WIP] Reuse TCP connections Jan 10, 2018
@rohitpaulk rohitpaulk force-pushed the reuse-tcp-connections-off-master branch from 6b203ad to bf5788d Compare January 10, 2018 10:29
rohitpaulk
rohitpaulk commented Jan 10, 2018
View reviewed changes
Comment thread lib/segment/analytics/request.rb
# If `start` is not called, Ruby adds a 'Connection: close' header
# too all requests, preventing us from reusing a connection for multiple
# HTTP requests
@http.start unless @http.started?

@rohitpaulk rohitpaulk Jan 10, 2018

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this comment because it took me a while to figure this out, and the Ruby docs don't mention it explicitly.

I'll also try raising this on the Ruby issue tracker once I've experimented a bit.

@rohitpaulk rohitpaulk Jan 10, 2018

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reported upstream: https://bugs.ruby-lang.org/issues/14349

@rohitpaulk rohitpaulk force-pushed the reuse-tcp-connections-off-master branch from bf5788d to 94179b8 Compare January 10, 2018 10:32
@codecov-io

codecov-io commented Jan 10, 2018 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #149      +/-   ##
==========================================
+ Coverage   98.38%   98.39%   +<.01%     
==========================================
  Files           9        9              
  Lines         371      373       +2     
==========================================
+ Hits          365      367       +2     
  Misses          6        6
Impacted Files Coverage Δ
lib/segment/analytics/request.rb 100% <100%> (ø) ⬆️
lib/segment/analytics/worker.rb 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 779cc77...94179b8. Read the comment docs.

@rohitpaulk rohitpaulk changed the title [WIP] Reuse TCP connections Reuse TCP connections Jan 10, 2018
@rohitpaulk rohitpaulk requested a review from f2prateek January 10, 2018 10:49
@f2prateek f2prateek merged commit 204d5a2 into master Jan 11, 2018
@rohitpaulk rohitpaulk deleted the reuse-tcp-connections-off-master branch June 22, 2018 11:33
@rohitpaulk rohitpaulk mentioned this pull request Dec 6, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@f2prateek f2prateek f2prateek approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rohitpaulk @codecov-io @f2prateek