Skip to content

Commit

Permalink
Merge pull request #3002 from DataDog/ivoanjo/remove-barrier-wait-next
Browse files Browse the repository at this point in the history
Remove unused Barrier#wait_next functionality
  • Loading branch information
ivoanjo authored Jul 26, 2023
2 parents defd6bc + 79f022b commit 9b9522f
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 138 deletions.
20 changes: 2 additions & 18 deletions lib/datadog/core/remote/component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,9 @@ def initialize(settings, capabilities, agent_settings)
end
end

def barrier(kind)
def barrier(_kind)
@worker.start

case kind
when :once
@barrier.wait_once
end
@barrier.wait_once
end

def shutdown!
Expand Down Expand Up @@ -105,18 +101,6 @@ def wait_once(timeout = nil)
end
end

# Wait for next lift to happen
def wait_next(timeout = nil)
@mutex.lock

timeout ||= @timeout

# rbs/core has a bug, timeout type is incorrectly ?Integer
@condition.wait(@mutex, _ = timeout)
ensure
@mutex.unlock
end

# Release all current waiters
def lift
@mutex.lock
Expand Down
133 changes: 13 additions & 120 deletions spec/datadog/core/remote/component_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -242,40 +242,13 @@

RSpec.describe Datadog::Core::Remote::Component::Barrier do
let(:delay) { 0.5 }
let(:record) { [] }
let(:timeout) { nil }
let(:instance_timeout) { nil }

subject(:barrier) { described_class.new(instance_timeout) }

shared_context('recorder') do
let(:record) { [] }
end

shared_context('waiter thread') do
include_context 'recorder'

let(:thr) do
Thread.new do
loop do
record << :wait
barrier.wait_next
end
end
end

before do
thr.run
end

after do
thr.kill
thr.join
end
end

shared_context('lifter thread') do
include_context 'recorder'

let(:thr) do
Thread.new do
loop do
Expand Down Expand Up @@ -309,8 +282,6 @@

describe '#lift' do
context 'without waiters' do
include_context 'recorder'

it 'does not block' do
record << :one
barrier.lift
Expand All @@ -321,21 +292,22 @@
end

context 'with waiters' do
include_context 'waiter thread'

it 'unblocks waiters' do
sleep delay
record << :one
barrier.lift
waiter_thread_queue = Queue.new
waiter_thread = Thread.new(record) do |record|
waiter_thread_queue << :ready
record << :wait
barrier.wait_once
record << :woke_up
end

sleep delay
record << :two
barrier.lift
waiter_thread_queue.pop # Wait for ready

# there may be an additional :wait if waiter thread gets switched to
recorded = record[0, 4]
record << :one
barrier.lift
waiter_thread.join

expect(recorded).to eq [:wait, :one, :wait, :two]
expect(record).to eq [:wait, :one, :woke_up]
end
end
end
Expand Down Expand Up @@ -420,83 +392,4 @@
end
end
end

describe '#wait_next' do
include_context 'lifter thread'

it 'blocks once' do
record << :one
barrier.wait_next
record << :two

expect(record).to eq [:one, :lift, :two]
end

it 'blocks each time' do
record << :one
barrier.wait_next
record << :two
barrier.wait_next
record << :three

expect(record).to eq [:one, :lift, :two, :lift, :three]
end

context('with a local timeout') do
let(:timeout) { delay / 100 }

context('shorter than lift') do
it 'unblocks on timeout' do
record << :one
barrier.wait_next(timeout)
record << :two
barrier.wait_next(timeout)

expect(record).to eq [:one, :two]
end
end

context('longer than lift') do
let(:delay) { 0.2 }
let(:timeout) { delay * 2 }

it 'unblocks before timeout' do
record << :one
barrier.wait_next(timeout)
record << :two
barrier.wait_next(timeout)
record << :three

expect(record).to eq [:one, :lift, :two, :lift, :three]
end
end

context('and an instance timeout') do
let(:instance_timeout) { delay * 2 }

it 'prefers the local timeout' do
record << :one
barrier.wait_next(timeout)
record << :two
barrier.wait_next(timeout)
record << :three

expect(record).to eq [:one, :two, :three]
end
end
end

context('with an instance timeout') do
let(:instance_timeout) { delay / 100 }

it 'unblocks on timeout' do
record << :one
barrier.wait_next
record << :two
barrier.wait_next

expect(record).to eq [:one, :two]
end
end
end
end

0 comments on commit 9b9522f

Please sign in to comment.