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.

Sign up for GitHub

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

Jump to bottom

Re-enable HTTP persistent connections (keep-alive) for RPC #1856

Closed
bitcartel opened this issue Nov 14, 2016 · 7 comments
Closed

Re-enable HTTP persistent connections (keep-alive) for RPC #1856

bitcartel opened this issue Nov 14, 2016 · 7 comments
Assignees
@str4d
Labels
A-networking Area: Networking code A-rpc-interface Area: RPC interface M-has-pr To-be-removed (GitHub has linked:pr filter)
Milestone
1.0.8

Comments

@bitcartel
Copy link
Contributor

The rpckeepalive option and accepting HTTP keep-alive connection were disabled by PR #1847 to resolve ticket #1680.

When the RPC networking layer is ready to support persistent connections, we should re-enable the option.
@bitcartel bitcartel added A-networking Area: Networking code A-rpc-interface Area: RPC interface labels Nov 14, 2016
@bitcartel bitcartel added this to the Wishlist - Soon After 1.0.[Soon] milestone Nov 14, 2016
@bitcartel bitcartel changed the title Re-enable HTTP persistent connections (kee-palive) for RPC Re-enable HTTP persistent connections (keep-alive) for RPC Nov 14, 2016
@bitcartel bitcartel mentioned this issue Nov 14, 2016
Merged
@daira
Copy link
Contributor

daira commented Nov 22, 2016

See also #1593, which may be a blocker for this.

@nathan-at-least
Copy link
Contributor

Because #1593 is quite invasive, we want to ensure its necessary, so first step is to see if any users would benefit from having long-poll re-enabled, and also seeing if there are alternative solutions for those users and seeing how they stack up with a libevent refactor.

@nathan-at-least nathan-at-least modified the milestones: 1.0.5, Wishlist - Soon After 1.0.[Soon] Dec 12, 2016
@bitcartel bitcartel modified the milestones: 1.0.6, 1.0.5 Jan 12, 2017
@bitcartel bitcartel modified the milestones: 1.0.7, 1.0.6 Feb 6, 2017
@str4d str4d self-assigned this Feb 22, 2017
@arcalinea arcalinea modified the milestones: 1.0.8, 1.0.7 Feb 28, 2017
@str4d str4d mentioned this issue Mar 14, 2017
Merged
@str4d str4d added the M-has-pr To-be-removed (GitHub has linked:pr filter) label Mar 14, 2017
zkbot added a commit that referenced this issue Mar 22, 2017
@zkbot
Auto merge of #2176 - str4d:1593-libevent, r=<try>
b0afc4b 
libevent-based http server

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#5677
- bitcoin/bitcoin#6695
- bitcoin/bitcoin#6899
- bitcoin/bitcoin#7016
- bitcoin/bitcoin#7964
- bitcoin/bitcoin#8722
- bitcoin/bitcoin#8730
- bitcoin/bitcoin#9073
- bitcoin/bitcoin#9265
- bitcoin/bitcoin#9387
- bitcoin/bitcoin#9471
- bitcoin/bitcoin#9647
- bitcoin/bitcoin#9903

Closes #1593 and #1856.
@bitcartel
Copy link
Contributor Author

@str4d Merging libevent will close this ticket, right? In that PR there is no mention of rpckeepalive.

@str4d
Copy link
Contributor

str4d commented Mar 22, 2017

@bitcartel it will close it in the same fashion as upstream did: the libevent PR reverts our commit that disabled rpckeepalive, and then applies upstream changes.

zkbot added a commit that referenced this issue Mar 25, 2017
@zkbot
Auto merge of #2176 - str4d:1593-libevent, r=bitcartel
f9f4866 
libevent-based http server

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#5677
- bitcoin/bitcoin#6695
- bitcoin/bitcoin#6899
- bitcoin/bitcoin#7016
- bitcoin/bitcoin#7964
- bitcoin/bitcoin#8722
- bitcoin/bitcoin#8730
- bitcoin/bitcoin#9073
- bitcoin/bitcoin#9265
- bitcoin/bitcoin#9387
- bitcoin/bitcoin#9471
- bitcoin/bitcoin#9647
- bitcoin/bitcoin#9903
- bitcoin/bitcoin#6640
- bitcoin/bitcoin#8139
- bitcoin/bitcoin#8839

Closes #1593 and #1856.
@daira
Copy link
Contributor

daira commented Mar 26, 2017

@str4d was this fixed by merging #2176?

@str4d
Copy link
Contributor

str4d commented Mar 26, 2017

@daira yes inasmuch as it was fixed upstream: bitcoin/bitcoin#5677 mentions that it avoids bitcoin/bitcoin#5655 (which added -rpckeepalive).

@str4d
Copy link
Contributor

str4d commented Mar 28, 2017

More specifically, we are relying on upstream's assertion that migrating to libevent solved the problem that they added -rpckeepalive to address.

@str4d str4d closed this as completed Mar 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@str4d str4d

Labels
A-networking Area: Networking code A-rpc-interface Area: RPC interface M-has-pr To-be-removed (GitHub has linked:pr filter)
Projects
None yet
Milestone
1.0.8
Development

No branches or pull requests
5 participants
@bitcartel @daira @nathan-at-least @str4d @arcalinea