htwkmooc/codeocean
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove semaphore where no longer required and re-enable

Browse Source
This commit is contained in:
Sebastian Serth
2020-03-22 16:53:14 +01:00
parent 7b10da8eb4
commit ccb5998b72
2 changed files with 10 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
lib/docker_client.rb
View File

@ -313,7 +313,7 @@ class DockerClient
# remove container from pool, then destroy it # remove container from pool, then destroy it
if (DockerContainerPool.config[:active]) if (DockerContainerPool.config[:active])
DockerContainerPool.acquire_semaphore DockerContainerPool.acquire_semaphore
DockerContainerPool.remove_from_all_containers(container, @execution_environment, bypass_semaphore: true) DockerContainerPool.remove_from_all_containers(container, @execution_environment)
# create new container and add it to @all_containers and @containers. # create new container and add it to @all_containers and @containers.
# ToDo: How long does creating a new cotainer take? We're still locking the semaphore. # ToDo: How long does creating a new cotainer take? We're still locking the semaphore.
@ -321,7 +321,7 @@ class DockerClient
if missing_counter_count > 0 if missing_counter_count > 0
Rails.logger.error('kill_container: Creating a new container.') Rails.logger.error('kill_container: Creating a new container.')
new_container = self.class.create_container(@execution_environment) new_container = self.class.create_container(@execution_environment)
DockerContainerPool.add_to_all_containers(new_container, @execution_environment, bypass_semaphore: true) DockerContainerPool.add_to_all_containers(new_container, @execution_environment)
else else
Rails.logger.error('Container killed and removed for ' + execution_environment.to_s + ' but not creating a new one. Currently, ' + missing_counter_count.abs + ' more containers than the configured pool size are available.') Rails.logger.error('Container killed and removed for ' + execution_environment.to_s + ' but not creating a new one. Currently, ' + missing_counter_count.abs + ' more containers than the configured pool size are available.')
end end

27
lib/docker_container_pool.rb
View File

@ -6,7 +6,6 @@ class DockerContainerPool
# Always use a semaphore when dealing with write access to `@containers` or `@all_containers` or read access if that involves creating a new container # Always use a semaphore when dealing with write access to `@containers` or `@all_containers` or read access if that involves creating a new container
# Some methods have the `bypass_semaphore` flag which should be rarely used. It was introduced when adding the semaphore to the `replace_broken_container` method. # Some methods have the `bypass_semaphore` flag which should be rarely used. It was introduced when adding the semaphore to the `replace_broken_container` method.
# From there, it was added to `remove_from_all_containers` and `add_to_all_containers` first. That ensures, replacing is atomic.
# When `replace_broken_container` was extended to check the total amount of containers first, the `bypass_semaphore` flag was also added to `get_container` and # When `replace_broken_container` was extended to check the total amount of containers first, the `bypass_semaphore` flag was also added to `get_container` and
# back to `replace_broken_container`. When the semaphore is not released, no container operations can be done! # back to `replace_broken_container`. When the semaphore is not released, no container operations can be done!
# Furthermore, the semaphore is used when killing a container in the DockerClient. # Furthermore, the semaphore is used when killing a container in the DockerClient.
@ -38,9 +37,8 @@ class DockerContainerPool
end end
def self.acquire_semaphore def self.acquire_semaphore
return
Rails.logger.info("Semaphore - Acquire: Trying " + @semaphore.inspect.to_s + " for " + caller_locations(1, 1)[0].label) Rails.logger.info("Semaphore - Acquire: Trying " + @semaphore.inspect.to_s + " for " + caller_locations(1, 1)[0].label)
got_semaphore = @semaphore.try_acquire(1, 10) got_semaphore = @semaphore.try_acquire(1, 20)
if got_semaphore if got_semaphore
Rails.logger.info("Semaphore - Acquire: Got " + @semaphore.inspect.to_s + " for " + caller_locations(1, 1)[0].label) Rails.logger.info("Semaphore - Acquire: Got " + @semaphore.inspect.to_s + " for " + caller_locations(1, 1)[0].label)
else else
@ -49,7 +47,6 @@ class DockerContainerPool
end end
def self.release_semaphore def self.release_semaphore
return
Rails.logger.info("Semaphore - Release: Trying " + @semaphore.inspect.to_s + " for " + caller_locations(1, 1)[0].label) Rails.logger.info("Semaphore - Release: Trying " + @semaphore.inspect.to_s + " for " + caller_locations(1, 1)[0].label)
if @semaphore.available_permits < 1 if @semaphore.available_permits < 1
@semaphore.release @semaphore.release
@ -59,20 +56,17 @@ class DockerContainerPool
end end
end end
def self.remove_from_all_containers(container, execution_environment, bypass_semaphore: false) def self.remove_from_all_containers(container, execution_environment)
acquire_semaphore unless bypass_semaphore
@all_containers[execution_environment.id].delete(container)
Rails.logger.debug('Removed container ' + container.to_s + ' from all_pool for execution environment ' + execution_environment.to_s + '. Remaining containers in all_pool ' + @all_containers[execution_environment.id].size.to_s)
if @containers[execution_environment.id].include?(container) if @containers[execution_environment.id].include?(container)
@containers[execution_environment.id].delete(container) @containers[execution_environment.id].delete(container)
Rails.logger.debug('Removed container ' + container.to_s + ' from available_pool for execution environment ' + execution_environment.to_s + '. Remaining containers in available_pool ' + @containers[execution_environment.id].size.to_s) Rails.logger.debug('Removed container ' + container.to_s + ' from available_pool for execution environment ' + execution_environment.to_s + '. Remaining containers in available_pool ' + @containers[execution_environment.id].size.to_s)
end end
release_semaphore unless bypass_semaphore
@all_containers[execution_environment.id].delete(container)
Rails.logger.debug('Removed container ' + container.to_s + ' from all_pool for execution environment ' + execution_environment.to_s + '. Remaining containers in all_pool ' + @all_containers[execution_environment.id].size.to_s)
end end
def self.add_to_all_containers(container, execution_environment, bypass_semaphore: false) def self.add_to_all_containers(container, execution_environment)
acquire_semaphore unless bypass_semaphore
@all_containers[execution_environment.id].push(container) @all_containers[execution_environment.id].push(container)
if !@containers[execution_environment.id].include?(container) if !@containers[execution_environment.id].include?(container)
@containers[execution_environment.id].push(container) @containers[execution_environment.id].push(container)
@ -80,7 +74,6 @@ class DockerContainerPool
else else
Rails.logger.error('failed trying to add existing container ' + container.to_s + ' to execution_environment ' + execution_environment.to_s) Rails.logger.error('failed trying to add existing container ' + container.to_s + ' to execution_environment ' + execution_environment.to_s)
end end
release_semaphore unless bypass_semaphore
end end
def self.create_container(execution_environment) def self.create_container(execution_environment)
@ -92,22 +85,18 @@ class DockerContainerPool
end end
def self.return_container(container, execution_environment) def self.return_container(container, execution_environment)
acquire_semaphore
container.status = 'available' # FIXME: String vs Symbol usage? container.status = 'available' # FIXME: String vs Symbol usage?
if @containers[execution_environment.id] && !@containers[execution_environment.id].include?(container) if @containers[execution_environment.id] && !@containers[execution_environment.id].include?(container)
@containers[execution_environment.id].push(container) @containers[execution_environment.id].push(container)
else else
Rails.logger.error('trying to return existing container ' + container.to_s + ' to execution_environment ' + execution_environment.to_s) Rails.logger.error('trying to return existing container ' + container.to_s + ' to execution_environment ' + execution_environment.to_s)
end end
release_semaphore
end end
def self.get_container(execution_environment, bypass_semaphore: false) def self.get_container(execution_environment, bypass_semaphore: false)
# if pooling is active, do pooling, otherwise just create an container and return it # if pooling is active, do pooling, otherwise just create an container and return it
if config[:active] if config[:active]
acquire_semaphore unless bypass_semaphore
container = @containers[execution_environment.id].try(:shift) || nil container = @containers[execution_environment.id].try(:shift) || nil
release_semaphore unless bypass_semaphore
Rails.logger.debug('get_container fetched container ' + container.to_s + ' for execution environment ' + execution_environment.to_s) Rails.logger.debug('get_container fetched container ' + container.to_s + ' for execution environment ' + execution_environment.to_s)
# just access and the following if we got a container. Otherwise, the execution_environment might be just created and not fully exist yet. # just access and the following if we got a container. Otherwise, the execution_environment might be just created and not fully exist yet.
if(container) if(container)
@ -137,12 +126,12 @@ class DockerContainerPool
def self.replace_broken_container(container, execution_environment, bypass_semaphore: false) def self.replace_broken_container(container, execution_environment, bypass_semaphore: false)
# See note at the top for `bypass_semaphore` # See note at the top for `bypass_semaphore`
acquire_semaphore unless bypass_semaphore acquire_semaphore unless bypass_semaphore
remove_from_all_containers(container, execution_environment, bypass_semaphore: true) remove_from_all_containers(container, execution_environment)
missing_counter_count = execution_environment.pool_size - @all_containers[execution_environment.id].length missing_counter_count = execution_environment.pool_size - @all_containers[execution_environment.id].length
if missing_counter_count > 0 if missing_counter_count > 0
Rails.logger.error('replace_broken_container: Creating a new container and returning that.') Rails.logger.error('replace_broken_container: Creating a new container and returning that.')
new_container = create_container(execution_environment) new_container = create_container(execution_environment)
DockerContainerPool.add_to_all_containers(new_container, execution_environment, bypass_semaphore: true) DockerContainerPool.add_to_all_containers(new_container, execution_environment)
else else
Rails.logger.error('Broken container removed for ' + execution_environment.to_s + ' but not creating a new one. Currently, ' + missing_counter_count.abs + ' more containers than the configured pool size are available.') Rails.logger.error('Broken container removed for ' + execution_environment.to_s + ' but not creating a new one. Currently, ' + missing_counter_count.abs + ' more containers than the configured pool size are available.')
new_container = get_container(execution_environment, bypass_semaphore: true) new_container = get_container(execution_environment, bypass_semaphore: true)