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

Agents 'n' timeouts #13

Merged
merged 4 commits into from
Sep 10, 2019
Merged

Agents 'n' timeouts #13

merged 4 commits into from
Sep 10, 2019

Conversation

flintinatux
Copy link
Contributor

@flintinatux flintinatux commented Sep 9, 2019

dilbert

Lots of ooey-gooey deliciousness in this one.

First up: Agents. They help "manage connection persistence". In a nutshell, they can help you reuse a pool of sockets for a given host and port, so you don't need a new TCP connection for every request. Think about any application making lots of requests in a row to an upstream service.

Also included: timeouts. Sometimes your upstream service is unreliable, and just hangs. You'd like the option to end the relationship early and try again with another socket. Now you can pass the timeout option, and when the request times out, you can backoff based on errors with a code of 'ECONNRESET'. I'm sure you can think of other reasons to timeout, but here's a quick example:

const { backoff } = require('@articulate/funky')
const gimme = require('@articulate/gimme')
const { prop, propEq } = require('tinyfunk')

const request = () =>
  gimme({ url, timeout: 1000 }).then(prop('body'))

const when =
  propEq('code', 'ECONNRESET')

const getTheStuff =
  backoff({ base: 250, tries: 4, when }, request)

to review:

  • Wait for the green check.
  • Read the updated docs.
  • For extra credit, try adding an Agent to a service or script of your choice:
    • Set it up with const agent = new Agent({ keepAlive: true })
    • Don't forget to agent.destroy() when everything is done. Your OS will thank you for releasing the sockets.
  • Find a squirrel agent who is out of time.

Fixes #12

@flintinatux flintinatux requested review from a team September 9, 2019 05:00
@flintinatux flintinatux self-assigned this Sep 9, 2019
Copy link
Contributor

@spencerfdavis spencerfdavis left a comment

Choose a reason for hiding this comment

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

Code looks good 👍

Copy link
Member

@mgreystone mgreystone left a comment

Choose a reason for hiding this comment

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

Code review only. I attempted to bump okta-profile-monitor but tests got quite upset, & i can't fall in that rabbit hole at the moment.

lib/gimme.js Show resolved Hide resolved
Copy link

@rpearce rpearce left a comment

Choose a reason for hiding this comment

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

code lookin 👌

Copy link
Member

@mgreystone mgreystone left a comment

Choose a reason for hiding this comment

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

I am failing to figure out how to test effectively. How do i know which agent is being used or the state of the tcp socket at a given time?

Code looks good to me, though, & i definitely see the value in this. 👍

@flintinatux
Copy link
Contributor Author

Yeah, I get that. I added it to a script of mine, and it cut the runtime by 40%, so that was the biggest sign for me that it was working.

@flintinatux
Copy link
Contributor Author

squirrel

@flintinatux flintinatux merged commit 377ea82 into master Sep 10, 2019
@flintinatux flintinatux deleted the agents-n-timeouts branch September 10, 2019 18:41
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.

Support timeout option
4 participants