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

Use persistent internet connection. #6

Merged
merged 1 commit into from
Apr 11, 2021
Merged

Conversation

ioquatix
Copy link
Member

Use a thread local to store the internet client. This ensures connections
persist between requests. In addition, improve the logging so that these
improvements are more visible.

This relates to the discussion in #4.

Use a thread local to store the internet client. This ensures connections
persist between requests. In addition, improve the logging so that these
improvements are more visible.
@ioquatix ioquatix force-pushed the use-persistent-connections branch from 10c6d66 to 50df978 Compare April 10, 2021 06:19
@ioquatix
Copy link
Member Author

image

The first request takes 3s and the warmed up request takes 2.2s.

@trevorturk
Copy link
Collaborator

This looks great, and I'm so happy to have a best practice to reference now.

Do you mind explaining briefly why a thread local is preferred vs a $global variable or constant? The thread-local README (https://github.com/socketry/thread-local#motivation) says "global variables are often not thread-safe" -- is that the main benefit?

/cc @josh since I know you were interested in the recommended way to reduce connection overhead etc and we had discussed using a global. The global seems to work in my limited experience, but perhaps I just haven't noticed any possible issues yet.

@ioquatix
Copy link
Member Author

Falcon supports both multi-process and multi-thread execution models, and eventually will support multi-ractor.

In multi-process, globals are shared by only one event loop (per process).

In multi-thread, globals are shared by multiple event loops, and this is not supported by Async - i.e. it is not safe (by design) to share Async:: objects between event loops.

In multi-ractor, globals don't work:

irb(main):001:1* X = {}
=> {}
irb(main):002:1* Ractor.new{X[:key] = :value}
<internal:ractor>:267: warning: Ractor is experimental, and the behavior may change in future versions of Ruby! Also there are many implementation issues.
#<Thread:0x00007fbd4f03c708 run> terminated with exception (report_on_exception is true):
(irb):2:in `block in <main>': => #<Ractor:#2 (irb):2 running>
can not access non-shareable objects in constant Object::X by non-main Ractor. (Ractor::IsolationError)

The thread-local gem solves all of these issues, in the sense that you always end up with a predictable "global" per event loop.

@ioquatix ioquatix merged commit 3a5f2b4 into main Apr 11, 2021
@ioquatix ioquatix deleted the use-persistent-connections branch April 11, 2021 06:30
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.

2 participants