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.

Sign up for GitHub

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

Add safety net for unresponsive docker containers #5120

Merged
merged 4 commits into from
Nov 14, 2023
Merged

Add safety net for unresponsive docker containers #5120

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1d9afaa
Add safety net for unreponsive docker containers
jorg-vr Nov 8, 2023
56e2c85
Notify error on failed jobs
jorg-vr Nov 14, 2023
298bd24
Raise errors from within thread
jorg-vr Nov 14, 2023
2e2d608
Add warning log line
jorg-vr Nov 14, 2023
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
21 changes: 16 additions & 5 deletions app/runners/submission_runner.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,19 @@
timer = Thread.new do
while Time.zone.now - before_time < time_limit
before_stats = Time.zone.now
# Check if container is still running
if !Rails.env.test? && (Docker::Container.all.any? { |c| c.id.starts_with?(container.id) || container.id.starts_with?(container.id) } && container.refresh!.info['State']['Running'])
# If we don't pass these extra options gathering stats takes 1+ seconds (https://github.com/moby/moby/issues/23188#issuecomment-223211481)
stats = container.stats({ 'one-shot': true, stream: false })
memory = [stats['memory_stats']['usage'] / (1024.0 * 1024.0), memory].max if stats['memory_stats']&.fetch('usage', nil)

begin
# Check if container is still running
if !Rails.env.test? && (Docker::Container.all.any? { |c| c.id.starts_with?(container.id) || container.id.starts_with?(container.id) } && container.refresh!.info['State']['Running'])
# If we don't pass these extra options gathering stats takes 1+ seconds (https://github.com/moby/moby/issues/23188#issuecomment-223211481)
stats = container.stats({ 'one-shot': true, stream: false })
memory = [stats['memory_stats']['usage'] / (1024.0 * 1024.0), memory].max if stats['memory_stats']&.fetch('usage', nil)

Check warning on line 172 in app/runners/submission_runner.rb

View check run for this annotation

Codecov / codecov/patch

app/runners/submission_runner.rb#L171-L172 

Added lines #L171 - L172 were not covered by tests
end
rescue Docker::Error::TimeoutError, Docker::Error::ServerError
# The docker container might be in a bad state
# We just ignore this and try again later
# The timeout will clean up the container if this lasts too long
Rails.logger.warn "Failed to get stats from docker container #{container.id} for submission #{@submission.id}"

Check warning on line 178 in app/runners/submission_runner.rb

View check run for this annotation

Codecov / codecov/patch

app/runners/submission_runner.rb#L178 

Added line #L178 was not covered by tests
end

# Gathering stats still takes a long time, so if we spent enough time on
Expand All @@ -179,6 +187,9 @@
timeout = true if timeout.nil?
end
end
# errors raised in the thread should also be raised in the main thread
# This ensures they are reported and handled correctly
timer.abort_on_exception = true

begin
outlines, errlines = container.tap(&:start).attach(
Expand Down