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

Added nakayoshi_fork option. #2256

Merged
merged 2 commits into from May 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion 5.0-Upgrade.md
Expand Up @@ -6,9 +6,9 @@ Puma 5 brings new experimental performance features, a few quality-of-life featu

* **Better throughput and lower latency via new experimental option**
* **Better memory usage via new experimental option**
* **Better copy-on-write performance via new experimental option**
* **Loads of bugfixes**
* Faster phased restarts and worker timeouts
* GC Compact/friendly fork
* pumactl now has a `thread-backtraces` command to print thread backtraces, bringing thread backtrace printing to all platforms, not just *BSD and Mac. (#2053)
* Added incrementing `requests_count` to `Puma.stats`. (#2106)
* Faster phased restart and worker timeout (#2220)
Expand Down
6 changes: 3 additions & 3 deletions History.md
@@ -1,9 +1,9 @@
## 5.0.0

* Features
* EXPERIMENTAL: Add `fork_worker` option and `refork` command for reduced memory usage. (#2099)
* EXPERIMENTAL: Reduce latency on MRI through inserting a small delay before re-listening on the socket if worker is busy (#2079).
* Possibly reduced memory usage by calling `GC.compact` before fork if available (Ruby 2.7+) (#2093)
* EXPERIMENTAL: Add `fork_worker` option and `refork` command for reduced memory usage by forking from a worker process and eliminating the master process. (#2099)
* EXPERIMENTAL: Added `wait_for_less_busy_worker` config. This may reduce latency on MRI through inserting a small delay before re-listening on the socket if worker is busy (#2079).
* EXPERIMENTAL: Added `nakayoshi_fork` option. Reduce memory usage in preloaded cluster-mode apps by GCing before fork and compacting, where available. (#2093, #2256)
* Added pumactl `thread-backtraces` command to print thread backtraces (#2054)
* Added incrementing `requests_count` to `Puma.stats`. (#2106)
* Increased maximum URI path length from 2048 to 8196 bytes (#2167)
Expand Down
2 changes: 1 addition & 1 deletion lib/puma/cli.rb
Expand Up @@ -133,7 +133,7 @@ def setup_options
o.on "-f", "--fork-worker=[REQUESTS]", OptionParser::DecimalInteger,
"Fork new workers from existing worker. Cluster mode only",
"Auto-refork after REQUESTS (default 1000)" do |*args|
user_config.fork_worker *args.compact
user_config.fork_worker(*args.compact)
end

o.on "-I", "--include PATH", "Specify $LOAD_PATH directories" do |arg|
Expand Down
15 changes: 13 additions & 2 deletions lib/puma/cluster.rb
Expand Up @@ -298,7 +298,7 @@ def worker(index, master)
restart_server.clear
server.begin_restart(true)
@launcher.config.run_hooks :before_refork, nil, @launcher.events
GC.compact if GC.respond_to?(:compact)
nakayoshi_gc
end
elsif idx == 0 # restart server
restart_server << true << false
Expand Down Expand Up @@ -540,7 +540,7 @@ def run
@master_read, @worker_write = read, @wakeup

@launcher.config.run_hooks :before_fork, nil, @launcher.events
GC.compact if GC.respond_to?(:compact)
nakayoshi_gc

spawn_workers

Expand Down Expand Up @@ -644,5 +644,16 @@ def timeout_workers
end
end
end

def nakayoshi_gc
return unless @options[:nakayoshi_fork]
log "! Promoting existing objects to old generation..."
4.times { GC.start(full_mark: false) }
if GC.respond_to?(:compact)
log "! Compacting..."
GC.compact
end
log "! Friendly fork preparation complete."
end
end
end
13 changes: 13 additions & 0 deletions lib/puma/dsl.rb
Expand Up @@ -759,5 +759,18 @@ def set_remote_address(val=:socket)
def fork_worker(after_requests=1000)
@options[:fork_worker] = Integer(after_requests)
end

# When enabled, Puma will GC 4 times before forking workers.
# If available (Ruby 2.7+), we will also call GC.compact.
# Not recommended for non-MRI Rubies.
#
# Based on the work of Koichi Sasada and Aaron Patterson, this option may
# decrease memory utilization of preload-enabled cluster-mode Pumas. It will
# also increase time to boot and fork. See your logs for details on how much
# time this adds to your boot process. For most apps, it will be less than one
# second.
def nakayoshi_fork(enabled=false)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we defaulting this to false? It seems weird that adding this in my config does nothing:

nakayoshi_fork

I have to

nakayoshi_fork(true)

Was this intentional or can we change the default?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thaaaaat would be a bug

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe that’s why there was so little change in CodeTriage

@options[:nakayoshi_fork] = enabled
end
end
end
4 changes: 2 additions & 2 deletions lib/puma/thread_pool.rb
Expand Up @@ -337,11 +337,11 @@ def shutdown(timeout=-1)
# Wait for threads to finish without force shutdown.
threads.each(&:join)
else
join = ->(timeout) do
join = ->(inner_timeout) do
start = Process.clock_gettime(Process::CLOCK_MONOTONIC)
threads.reject! do |t|
elapsed = Process.clock_gettime(Process::CLOCK_MONOTONIC) - start
t.join timeout - elapsed
t.join inner_timeout - elapsed
end
end

Expand Down
16 changes: 16 additions & 0 deletions test/test_integration_cluster.rb
Expand Up @@ -168,6 +168,22 @@ def test_refork
refute_includes pids, get_worker_pids(1, WORKERS - 1)
end

def test_nakayoshi
cli_server "-w #{WORKERS} test/rackup/hello.ru", config: <<RUBY
nakayoshi_fork true
RUBY

output = nil
Timeout.timeout(10) do
until output
output = @server.gets[/Friendly fork preparation complete/]
sleep(0.01)
end
end

assert output, "Friendly fork didn't run"
end

private

def worker_timeout(timeout, iterations, config)
Expand Down
2 changes: 1 addition & 1 deletion test/test_integration_single.rb
Expand Up @@ -49,7 +49,7 @@ def test_prefer_rackup_file_specified_by_cli
cli_server "-C test/config/with_rackup_from_dsl.rb test/rackup/hello.ru"
connection = connect
reply = read_body(connection)
_, status = stop_server
stop_server

assert_match("Hello World", reply)
end
Expand Down
2 changes: 1 addition & 1 deletion test/test_puma_server.rb
Expand Up @@ -902,7 +902,7 @@ def shutdown_requests(app_delay: 2, request_delay: 1, post: false, response:, **

s2 << "\r\n"

assert_match /204/, s1.gets
assert_match(/204/, s1.gets)

assert IO.select([s2], nil, nil, app_delay), 'timeout waiting for response'
s2_result = begin
Expand Down