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

Fix issue with Que being unable to exit cleanly #116

Merged
merged 9 commits into from
Feb 21, 2025
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 26 additions & 47 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,16 @@ name: tests
on:
push:

jobs:
jobs:
rubocop:
runs-on: ubuntu-latest
env:
BUNDLE_RUBYGEMS__PKG__GITHUB__COM: gocardless-robot-readonly:${{ secrets.GITHUB_TOKEN }}
steps:
- uses: actions/checkout@v4
- name: Set up Ruby
uses: ruby/setup-ruby@v1
with:
bundler-cache: true
ruby-version: "3.0"
ruby-version: "3.3"
- name: Run rubocop
run: |
bundle exec rubocop --extra-details --display-style-guide --no-server --parallel
Expand All @@ -23,7 +21,7 @@ jobs:
strategy:
fail-fast: false
matrix:
ruby_version: ["3.0", "3.1", "3.2", "3.3"]
ruby_version: ["3.3", "3.4"]
rack_version: ["2.2.5", "3.1"]
runs-on: ubuntu-latest
services:
Expand All @@ -34,45 +32,20 @@ jobs:
POSTGRES_USER: ubuntu
POSTGRES_PASSWORD: password
ports:
- 5432:5432
- 5435:5432
options: >-
--health-cmd pg_isready
--health-interval 10s
--health-timeout 5s
--health-retries 10
env:
PGDATABASE: que-test
PGUSER: ubuntu
PGPASSWORD: password
PGHOST: localhost
BUNDLE_RUBYGEMS__PKG__GITHUB__COM: gocardless-robot-readonly:${{ secrets.GITHUB_TOKEN }}
RACK_VERSION: "${{ matrix.rack_version }}"
steps:
- uses: actions/checkout@v4
- name: Set up Ruby
uses: ruby/setup-ruby@v1
with:
bundler-cache: true
ruby-version: "${{ matrix.ruby_version }}"
- name: Start bin/que
run: |
bundle exec bin/que ./lib/que.rb --metrics-port=8080 --ci

rspec:
strategy:
fail-fast: false
matrix:
ruby_version: ["3.0", "3.1", "3.2", "3.3"]
runs-on: ubuntu-latest
services:
postgres:
lock_database:
image: postgres:14.2
env:
POSTGRES_DB: que-test
POSTGRES_DB: lock-test
POSTGRES_USER: ubuntu
POSTGRES_PASSWORD: password
ports:
- 5432:5432
- 5436:5432
options: >-
--health-cmd pg_isready
--health-interval 10s
Expand All @@ -83,23 +56,30 @@ jobs:
PGUSER: ubuntu
PGPASSWORD: password
PGHOST: localhost
BUNDLE_RUBYGEMS__PKG__GITHUB__COM: gocardless-robot-readonly:${{ secrets.GITHUB_TOKEN }}
PGPORT: 5435
LOCK_PGDATABASE: lock-test
LOCK_PGUSER: ubuntu
LOCK_PGPASSWORD: password
LOCK_PGHOST: localhost
LOCK_PGPORT: 5436
RACK_VERSION: "${{ matrix.rack_version }}"
steps:
- uses: actions/checkout@v4
- name: Set up Ruby
uses: ruby/setup-ruby@v1
with:
bundler-cache: true
ruby-version: "${{ matrix.ruby-version }}"
- name: Run specs
ruby-version: "${{ matrix.ruby_version }}"
- name: Run smoke tests
run: |
bundle exec rspec
bundle exec rspec spec/smoke_tests/

active_record_with_lock_adapter_rspec:
rspec:
timeout-minutes: 5
strategy:
fail-fast: false
matrix:
ruby_version: ["3.0", "3.1", "3.2", "3.3"]
ruby_version: ["3.3", "3.4"]
runs-on: ubuntu-latest
services:
postgres:
Expand All @@ -109,7 +89,7 @@ jobs:
POSTGRES_USER: ubuntu
POSTGRES_PASSWORD: password
ports:
- 5432:5432
- 5435:5432
options: >-
--health-cmd pg_isready
--health-interval 10s
Expand All @@ -122,23 +102,23 @@ jobs:
POSTGRES_USER: ubuntu
POSTGRES_PASSWORD: password
ports:
- 5434:5432
- 5436:5432
options: >-
--health-cmd pg_isready
--health-interval 10s
--health-timeout 5s
--health-retries 10
--health-retries 10
env:
PGDATABASE: que-test
PGUSER: ubuntu
PGPASSWORD: password
PGHOST: localhost
BUNDLE_RUBYGEMS__PKG__GITHUB__COM: gocardless-robot-readonly:${{ secrets.GITHUB_TOKEN }}
PGPORT: 5435
LOCK_PGDATABASE: lock-test
LOCK_PGUSER: ubuntu
LOCK_PGPASSWORD: password
LOCK_PGHOST: localhost
ADAPTER: ActiveRecordWithLock
LOCK_PGPORT: 5436
steps:
- uses: actions/checkout@v4
- name: Set up Ruby
Expand All @@ -147,5 +127,4 @@ jobs:
bundler-cache: true
ruby-version: "${{ matrix.ruby-version }}"
- name: Run Specs With ActiveRecordWithLock Adapter
run: bundle exec rspec

run: bundle exec rspec --exclude-pattern "spec/smoke_tests/**/*"
7 changes: 5 additions & 2 deletions .rubocop.yml
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
inherit_gem:
gc_ruboconfig: rubocop.yml

require:
plugins:
- rubocop-rspec
- rubocop-performance
- rubocop-rake
- rubocop-rspec

require:
- rubocop-sequel

AllCops:
NewCops: enable
TargetRubyVersion: 3.3
Exclude:
- "vendor/**/*"
- "legacy_spec/**/*"
Expand Down
2 changes: 1 addition & 1 deletion .ruby-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3.3.1
3.3.7
2 changes: 1 addition & 1 deletion Dockerfile.benchmark
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM ruby:3.0.2
FROM ruby:3.3
COPY . /que
WORKDIR /que/benchmark
RUN bundle install
Expand Down
18 changes: 7 additions & 11 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

source 'https://rubygems.org'

gemspec

group :development, :test do
gem 'rake'

Expand All @@ -11,11 +13,11 @@ group :development, :test do
gem 'pg', require: nil, platform: :ruby
gem 'pg_jruby', require: nil, platform: :jruby
gem 'pond', require: nil
gem 'rubocop', '~> 1.64.1'
gem 'rubocop-performance', '~> 1.21.1'
gem 'rubocop-rake', '~> 0.6.0'
gem 'rubocop-rspec', '~> 3.0.2'
gem 'rubocop-sequel', '~> 0.3.4'
gem 'rubocop', '~> 1.72.2'
gem 'rubocop-performance', '~> 1.24.0'
gem 'rubocop-rake', '~> 0.7.1'
gem 'rubocop-rspec', '~> 3.5.0'
gem 'rubocop-sequel', '~> 0.3.8'
gem 'sequel', require: nil

rack_version = ENV.fetch('RACK_VERSION', "3.1")
Expand All @@ -32,9 +34,3 @@ group :test do
gem 'pry-byebug'
gem 'rspec', '~> 3.9'
end

gem 'prometheus-client', '~> 1.0'
source "https://rubygems.pkg.github.com/gocardless" do
gem "prometheus_gcstat"
end
gemspec
11 changes: 1 addition & 10 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,16 +139,7 @@ Note that running specs requires a running Postgres instance.
To start the server in a docker container perform the following:

```
docker run -p5432:5432 --env POSTGRES_USER=ubuntu --env POSTGRES_PASSWORD=password --env POSTGRES_DB=que-test postgres:11.2
```

Now inform the test suite where Postgres is running using environment variables

```
export PGDATABASE=que-test
export PGUSER=ubuntu
export PGPASSWORD=password
export PGHOST=localhost
docker compose up -d --force-recreate
```

A note on running specs - Que's worker system is multithreaded and therefore prone to race conditions (especially on interpreters without a global lock, like Rubinius or JRuby). As such, if you've touched that code, a single spec run passing isn't a guarantee that any changes you've made haven't introduced bugs. One thing I like to do before pushing changes is rerun the specs many times and watching for hangs. You can do this from the command line with something like:
Expand Down
73 changes: 26 additions & 47 deletions bin/que
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,15 @@ require "logger"
require "optparse"
require "ostruct"
require "prometheus/middleware/exporter"
require "prometheus_gcstat"
begin
require "prometheus_gcstat"
rescue LoadError
# If the gem is not available, we'll just skip it
end
require "puma"
require "que"
require "rack"
USE_RACKUP = Rack.release.split(".")[0].to_i >= 3
USE_RACKUP = Gem::Version.new(Rack.release) >= Gem::Version.new("3.0.0")
if USE_RACKUP
require "rackup"
end
Expand Down Expand Up @@ -73,10 +77,6 @@ OptionParser.new do |opts|
$stdout.puts opts
exit 0
end

opts.on("--ci", "Don't wait for sigterm exit after boot") do
options.ci = true
end
end.parse!(ARGV)
# rubocop:enable Layout/LineLength

Expand Down Expand Up @@ -106,21 +106,6 @@ secondary_queues = options.secondary_queues || []

Que.logger ||= Logger.new($stdout)

if options.ci
require "active_record"

ActiveRecord::Base.establish_connection(
adapter: "postgresql",
host: ENV.fetch("PGHOST", "localhost"),
user: ENV.fetch("PGUSER", "postgres"),
password: ENV.fetch("PGPASSWORD", ""),
database: ENV.fetch("PGDATABASE", "que-test"),
)

Que.connection = ActiveRecord
Que.migrate!
end

begin
Que.logger.level = Logger.const_get(log_level.upcase) if log_level
rescue NameError
Expand Down Expand Up @@ -155,8 +140,10 @@ if options.metrics_port

health_check = ->(_) { [200, {}, ["healthy"]] }

Prometheus::MemoryStats.
new(Prometheus::Client.registry).start(interval: 10.seconds, delay: 10.seconds)
if defined?(Prometheus::MemoryStats)
Prometheus::MemoryStats.
new(Prometheus::Client.registry).start(interval: 10.seconds, delay: 10.seconds)
end

app = Rack::URLMap.new(
"/" => Rack::Builder.new do
Expand All @@ -175,32 +162,24 @@ if options.metrics_port
end,
)

host = "0.0.0.0"

handler =
if USE_RACKUP
Rackup::Handler::Puma
else
Rack::Handler::Puma
end

pidfile_opts = Dir.exist?("./tmp/pids/") ? {} : { pidfile: nil }

handler.run(
app,
Host: host,
Port: options.metrics_port,
Silent: false,
AccessLog: [],
**pidfile_opts,
)
# Uses the Handler::Puma class to build the config and then we call Server directly
# instead of going through the Handler::Puma.run method. This is to prevent the Handler
# from trapping signals and which would prevent Que exiting cleanly.
# https://github.com/puma/puma/blob/master/lib/rack/handler/puma.rb
server_options = { Host: "0.0.0.0", Port: options.metrics_port, Silent: false, AccessLog: [] }
handler = USE_RACKUP ? Rackup::Handler::Puma : Rack::Handler::Puma
puma_config = handler.config(app, server_options)
puma_config.clamp

log_writer = Puma::LogWriter.stdio
puma_config.options[:log_writer] = log_writer

Puma::Server.new(app, nil, puma_config.options).tap do |s|
s.binder.parse(puma_config.options[:binds], s.log_writer)
end.run.join
end
end

# For a basic CI check we just want to ensure the app boots so don't want to
# block the main thread, so this will just exit immediately.
unless options.ci
wait_for_signals("INT", "TERM")
end
wait_for_signals("INT", "TERM")

worker_group.stop(timeout)
18 changes: 18 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
services:
default_db:
image: postgres:14.2
environment:
POSTGRES_DB: que-test
POSTGRES_USER: postgres
POSTGRES_PASSWORD: password
ports:
- 5435:5432

lock_db:
image: postgres:14.2
environment:
POSTGRES_DB: que-test-lock
POSTGRES_USER: postgres
POSTGRES_PASSWORD: password
ports:
- 5436:5432
Loading