Skip to content
This repository has been archived by the owner on Mar 7, 2018. It is now read-only.

Change webserver to puma and adapt command line interface #677

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,5 @@
*.gem
coverage/
log/
tmp/
.ruby-version
history.yml
3 changes: 2 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ rvm:
- 2.3.0
- 2.2.4
- 2.1.8

- jruby-19mode
- jruby-9.0.4.0
script: "rake test"
2 changes: 1 addition & 1 deletion dashing.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ Gem::Specification.new do |s|
s.add_dependency('execjs', '~> 2.0.2')
s.add_dependency('sinatra', '~> 1.4.4')
s.add_dependency('sinatra-contrib', '~> 1.4.2')
s.add_dependency('thin', '~> 1.6.1')
s.add_dependency('puma', '~> 2.16.0')
s.add_dependency('rufus-scheduler', '~> 2.0.24')
s.add_dependency('thor', '> 0.18.1')
s.add_dependency('sprockets', '~> 2.10.1')
Expand Down
53 changes: 32 additions & 21 deletions lib/dashing/app.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
require 'sinatra'
require 'sprockets'
require 'sinatra/content_for'
require 'rufus/scheduler'
require 'coffee-script'
require 'sass'
require 'json'
require 'rufus/scheduler'
require 'sass'
require 'sinatra'
require 'sinatra/content_for'
require 'sinatra/streaming'
require 'sprockets'
require 'yaml'
require 'thin'

SCHEDULER = Rufus::Scheduler.new

Expand Down Expand Up @@ -34,7 +34,7 @@ def authenticated?(token)
set :sprockets, Sprockets::Environment.new(settings.root)
set :assets_prefix, '/assets'
set :digest_assets, false
set server: 'thin', connections: [], history_file: 'history.yml'
set server: 'puma', connections: [], history_file: 'history.yml'
set :public_folder, File.join(settings.root, 'public')
set :views, File.join(settings.root, 'dashboards')
set :default_dashboard, nil
Expand Down Expand Up @@ -70,13 +70,23 @@ def authenticated?(token)
redirect "/" + dashboard
end


get '/events', provides: 'text/event-stream' do
protected!
response.headers['X-Accel-Buffering'] = 'no' # Disable buffering for nginx
stream :keep_open do |out|
settings.connections << out
stream do |out|
out << latest_events
out.callback { settings.connections.delete(out) }
settings.connections << connection = {out: out, mutex: Mutex.new, terminated: false}
Copy link
Member

Choose a reason for hiding this comment

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

Each connection gets its own Mutex?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it signifies the out resource which we are injecting events into inside send_event.

terminated = false

loop do
connection[:mutex].synchronize do
terminated = true if connection[:terminated]
end
break if terminated
end

Copy link
Member

Choose a reason for hiding this comment

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

Can you guys explain this section a bit more?

Copy link
Contributor

Choose a reason for hiding this comment

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

Each request gets a thread in the thread pool until Puma hits the max and multiplexes the threads. The loop is basically just "keep the SSE request open until marked as terminated".

The connection is marked as terminated in send_event when there is an exception caught when trying to send to a dead connection. send_even is triggered from Rufus -- which is multithreaded in Puma.

Copy link
Member

Choose a reason for hiding this comment

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

There's no way to keep alive a connection without doing an idle loop?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think so. I am not sure, but I think this is because Sinatra doesn't properly implement Rack hijack.

The while loop stuff comes from this: http://tenderlovemaking.com/2012/07/30/is-it-live.html

Copy link
Contributor

Choose a reason for hiding this comment

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

The idea that Sinatra doesn't implement rack hijack properly comes from smart dudes comments here: sinatra/sinatra#1035 (comment)

But most of it is over my head.

settings.connections.delete(connection)
end
end

Expand Down Expand Up @@ -123,22 +133,23 @@ def authenticated?(token)
end
end

Thin::Server.class_eval do
def stop_with_connection_closing
Sinatra::Application.settings.connections.dup.each(&:close)
stop_without_connection_closing
end

alias_method :stop_without_connection_closing, :stop
alias_method :stop, :stop_with_connection_closing
end

def send_event(id, body, target=nil)
body[:id] = id
body[:updatedAt] ||= Time.now.to_i
event = format_event(body.to_json, target)
Sinatra::Application.settings.history[id] = event unless target == 'dashboards'
Sinatra::Application.settings.connections.each { |out| out << event }
Sinatra::Application.settings.connections.each do |connection|
connection[:mutex].synchronize do
begin
connection[:out] << event unless connection[:out].closed?
rescue Puma::ConnectionError
connection[:terminated] = true
rescue Exception => e
connection[:terminated] = true
puts e
end
end
end
end

def format_event(body, name=nil)
Expand Down
14 changes: 9 additions & 5 deletions lib/dashing/cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,16 +57,20 @@ def install(gist_id, *args)
desc "start", "Starts the server in style!"
method_option :job_path, :desc => "Specify the directory where jobs are stored"
def start(*args)
port_option = args.include?('-p') ? '' : ' -p 3030'
daemonize = args.include?('-d')
args = args.join(' ')
command = "bundle exec thin -R config.ru start#{port_option} #{args}"
command = "bundle exec puma #{args}"
command.prepend "export JOB_PATH=#{options[:job_path]}; " if options[:job_path]
command.prepend "export DAEMONIZE=true; " if daemonize
run_command(command)
end

desc "stop", "Stops the thin server"
def stop
command = "bundle exec thin stop"
desc "stop", "Stops the puma server (daemon mode only)"
def stop(*args)
args = args.join(' ')
# TODO correctly handle pidfile location change in puma config
daemon_pidfile = !args.include?('--pidfile') ? '--pidfile ./tmp/pids/puma.pid' : args
command = "bundle exec pumactl #{daemon_pidfile} stop"
run_command(command)
end

Expand Down
42 changes: 42 additions & 0 deletions templates/project/config/puma.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# For a complete list of puma configuration parameters, please see
# https://github.com/puma/puma

# Puma can serve each request in a thread from an internal thread pool.
# The `threads` method setting takes two numbers a minimum and maximum.
# Any libraries that use thread pools should be configured to match
# the maximum value specified for Puma. Default is set to 5 threads for minimum
# and maximum.
#
threads_count = ENV.fetch("PUMA_MAX_THREADS") { 5 }.to_i
threads threads_count, threads_count

# Specifies the `port` that Puma will listen on to receive requests, default is 2020.
#
port ENV.fetch("DASHING_PORT") { 3030 }

# Specifies the `environment` that Puma will run in.
#
environment ENV.fetch("RACK_ENV") { "production" }

# Daemonize the server into the background. Highly suggest that
# this be combined with "pidfile" and "stdout_redirect".
#
# The default is "false".
#
daemonize ENV.fetch("DAEMONIZE") { false }

# Store the pid of the server in the file at "path".
#
pidfile './tmp/pids/puma.pid'

# Use "path" as the file to store the server info state. This is
# used by "pumactl" to query and control the server.
#
state_path './tmp/pids/puma.state'

# Redirect STDOUT and STDERR to files specified. The 3rd parameter
# ("append") specifies whether the output is appended, the default is
# "false".
#
# stdout_redirect '/u/apps/lolcat/log/stdout', '/u/apps/lolcat/log/stderr'
# stdout_redirect '/u/apps/lolcat/log/stdout', '/u/apps/lolcat/log/stderr', true
1 change: 1 addition & 0 deletions templates/project/tmp/pids/.empty_directory
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
.empty_directory
25 changes: 13 additions & 12 deletions test/app_test.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
require 'test_helper'
require 'haml'

class StreamStub < Array
def closed?
return false
end
end

class AppTest < Dashing::Test
def setup
@connection = []
app.settings.connections = [@connection]
@connection = {out: StreamStub.new, mutex: Mutex.new, terminated: false}
app.settings.connections = [ @connection ]
app.settings.auth_token = nil
app.settings.default_dashboard = nil
app.settings.history_file = File.join(Dir.tmpdir, 'history.yml')
Expand Down Expand Up @@ -48,9 +54,8 @@ def test_errors_out_when_no_dashboards_available
def test_post_widgets_without_auth_token
post '/widgets/some_widget', JSON.generate({value: 6})
assert_equal 204, last_response.status

assert_equal 1, @connection.length
data = parse_data @connection[0]
assert_equal 1, @connection[:out].length
data = parse_data @connection[:out][0]
assert_equal 6, data['value']
assert_equal 'some_widget', data['id']
assert data['updatedAt']
Expand All @@ -72,19 +77,15 @@ def test_get_events
post '/widgets/some_widget', JSON.generate({value: 8})
assert_equal 204, last_response.status

get '/events'
assert_equal 200, last_response.status
assert_equal 8, parse_data(@connection[0])['value']
assert_equal 8, parse_data(@connection[:out][0])['value']
end

def test_dashboard_events
post '/dashboards/my_super_sweet_dashboard', JSON.generate({event: 'reload'})
assert_equal 204, last_response.status

get '/events'
assert_equal 200, last_response.status
assert_equal 'dashboards', parse_event(@connection[0])
assert_equal 'reload', parse_data(@connection[0])['event']
assert_equal 'dashboards', parse_event(@connection[:out][0])
assert_equal 'reload', parse_data(@connection[:out][0])['event']
end

def test_get_dashboard
Expand Down
24 changes: 17 additions & 7 deletions test/cli_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,31 +101,41 @@ def test_install_task_warns_when_gist_not_found
assert_includes output, 'Could not find gist at '
end

def test_start_task_starts_thin_with_default_port
command = 'bundle exec thin -R config.ru start -p 3030 '
def test_start_task_starts_puma_with_default_port
command = 'bundle exec puma '
@cli.stubs(:run_command).with(command).once
@cli.start
end

def test_start_task_starts_thin_with_specified_port
command = 'bundle exec thin -R config.ru start -p 2020'
def test_start_task_starts_puma_in_daemon_mode
commands = [
'export DAEMONIZE=true; ',
'bundle exec puma -d'
]

@cli.stubs(:run_command).with(commands.join('')).once
@cli.start('-d')
end

def test_start_task_starts_puma_with_specified_port
command = 'bundle exec puma -p 2020'
@cli.stubs(:run_command).with(command).once
@cli.start('-p', '2020')
end

def test_start_task_supports_job_path_option
commands = [
'export JOB_PATH=other_spot; ',
'bundle exec thin -R config.ru start -p 3030 '
'bundle exec puma '
]

@cli.stubs(:options).returns(job_path: 'other_spot')
@cli.stubs(:run_command).with(commands.join('')).once
@cli.start
end

def test_stop_task_stops_thin_server
@cli.stubs(:run_command).with('bundle exec thin stop')
def test_stop_task_stops_puma_server
@cli.stubs(:run_command).with('bundle exec pumactl --pidfile ./tmp/pids/puma.pid stop')
@cli.stop
end

Expand Down