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

Create HTTP requests on a background queue. #720

Merged
merged 1 commit into from
Sep 28, 2017
Merged

Conversation

f2prateek
Copy link
Contributor

@f2prateek f2prateek commented Sep 26, 2017

This refactors the internal implementation of the sendData to create
the HTTP requests on a new background queue. Note that this simply changes
the creation of the request to the new background queue - the actual
execution was already on a background thread.

Previously, the creation would happen on the queueing thread. This
means that while the HTTP request was being created, events would not be
queued. By default, this is not a problem, the actual creation of the
request is a few milliseconds at most. However, this assumes that the request
factory customers might set only runs for a few milliseconds. A long running request
factory would have a chance of blocking the queueing thread for too long and
open a small window for data loss.

With this change, customers to add more complex logic in their request
factories. e.g. a customer can use the request factory to fetch a
token for a user, and attach that token to the request being made to by
the library to their proxy.

How should this be manually tested?
This is already being tested by the existing code paths. I also manually tested by adding a few logging statements https://cloudup.com/chrCiBv2E_f.

2017-09-26 15:59:09.503 CocoapodsExample[17623:1103499] test: requestFactory invoked
2017-09-26 15:59:09.504 CocoapodsExample[17623:1103544] test: Enqueing message
2017-09-26 15:59:09.505 CocoapodsExample[17623:1103544] test: Enqueing message
2017-09-26 15:59:09.507 CocoapodsExample[17623:1103544] test: Enqueing message
2017-09-26 15:59:09.509 CocoapodsExample[17623:1103544] test: Enqueing message
2017-09-26 15:59:14.505 CocoapodsExample[17623:1103499] test: requestFactory invoked
2017-09-26 15:59:14.647 CocoapodsExample[17623:1103497] test: upload done
2017-09-26 15:59:17.118 CocoapodsExample[17623:1103544] test: Enqueing message
2017-09-26 15:59:17.991 CocoapodsExample[17623:1103496] test: Enqueing message
2017-09-26 15:59:18.619 CocoapodsExample[17623:1103496] test: Enqueing message
2017-09-26 15:59:19.250 CocoapodsExample[17623:1103496] test: Enqueing message
2017-09-26 15:59:19.524 CocoapodsExample[17623:1103544] test: upload done
2017-09-26 15:59:20.602 CocoapodsExample[17623:1103499] test: Enqueing message
2017-09-26 15:59:21.201 CocoapodsExample[17623:1103499] test: Enqueing message
2017-09-26 15:59:21.775 CocoapodsExample[17623:1103499] test: Enqueing message
2017-09-26 15:59:21.778 CocoapodsExample[17623:1103544] test: requestFactory invoked
2017-09-26 15:59:22.585 CocoapodsExample[17623:1103496] test: Enqueing message
2017-09-26 15:59:23.313 CocoapodsExample[17623:1103496] test: Enqueing message
2017-09-26 15:59:23.842 CocoapodsExample[17623:1103496] test: Enqueing message
2017-09-26 15:59:24.392 CocoapodsExample[17623:1103499] test: Enqueing message
2017-09-26 15:59:24.851 CocoapodsExample[17623:1103496] test: Enqueing message
2017-09-26 15:59:25.318 CocoapodsExample[17623:1103499] test: Enqueing message
2017-09-26 15:59:25.734 CocoapodsExample[17623:1103496] test: Enqueing message
2017-09-26 15:59:25.925 CocoapodsExample[17623:1103499] test: Enqueing message
2017-09-26 15:59:26.106 CocoapodsExample[17623:1103499] test: Enqueing message
2017-09-26 15:59:26.303 CocoapodsExample[17623:1103496] test: Enqueing message
2017-09-26 15:59:26.495 CocoapodsExample[17623:1103496] test: Enqueing message
2017-09-26 15:59:26.800 CocoapodsExample[17623:1103499] test: upload done
2017-09-26 15:59:27.003 CocoapodsExample[17623:1103496] test: Enqueing message
2017-09-26 15:59:27.802 CocoapodsExample[17623:1103496] test: Enqueing message
2017-09-26 15:59:28.036 CocoapodsExample[17623:1113360] test: Enqueing message
2017-09-26 15:59:28.222 CocoapodsExample[17623:1113360] test: Enqueing message

You'll see that in between the factory being invoked (which is faking a slow factory by sleeping) and the request being done, the library was able to queue messages.

Any background context you want to provide?
Internal Doc: https://paper.dropbox.com/doc/poHde883uCtEZwJ8MDGwH

@segmentio/gateway

@codecov-io
Copy link

codecov-io commented Sep 26, 2017

Codecov Report

Merging #720 into dev will increase coverage by 0.77%.
The diff coverage is 70.58%.

@@            Coverage Diff             @@
##              dev     #720      +/-   ##
==========================================
+ Coverage   71.24%   72.01%   +0.77%     
==========================================
  Files          39       39              
  Lines        1624     1633       +9     
  Branches      173      174       +1     
==========================================
+ Hits         1157     1176      +19     
+ Misses        350      338      -12     
- Partials      117      119       +2

This refactors the internal implementation of the `sendData` to _create_
the HTTP requests on a new background queue. Note that this simply changes
the _creation_ of the request to the new background queue - the actual
execution was already on a background thread.

With this change, customers to add more complex logic in their connection
factories. e.g. a customer can use the connection factory to fetch a
token for a user, and attach that token to the request being made to by
the library to their proxy.

Previously, the creation would happen on the queueing thread. This
means that while the HTTP request was being _created_, events would not be
queued. By default, this is not a problem, the actual creation of the
request is a few ms at most. However, this assumes that the connection
factory customers might set only runs for a few ms. A long running connection
factory would have a chance of blocking the queueing thread for too long and
open a small window for data loss.

This opened up a slight chance that the integration would call flush
(for various reasons) while the HTTP request was being created
(particularly if the connection factory is slow). Hence we add a check to
verify that a request is not in progress before proceeding to create the
request. This is done with a boolean flag that is only read/updated on
the serialQueue.

Lastly, the code that sets the `sentAt` flag is now moved to inside the
HTTP Client. This is to move it closer to the time when the HTTP request
is actually made - otherwise a slow connection factory would cause the
server to think that the device timestamps are skewed.
@f2prateek
Copy link
Contributor Author

This opened up a slight chance that the integration would call flush
(for various reasons) while the HTTP request was being created
(particularly if the connection factory is slow). Hence we add a check to
verify that a request is not in progress before proceeding to create the
request. This is done with a boolean flag that is only read/updated on
the serialQueue.

Lastly, the code that sets the sentAt flag is now moved to inside the
HTTP Client. This is to move it closer to the time when the HTTP request
is actually made - otherwise a slow connection factory would cause the
server to think that the device timestamps are skewed.

@f2prateek f2prateek merged commit 0da980c into dev Sep 28, 2017
@f2prateek f2prateek deleted the async-connection-factory branch September 28, 2017 18:10
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