From b29a41e6934470f3fbff58e6aadf379bc87781df Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Tue, 3 Nov 2015 17:34:34 +0100 Subject: [PATCH 01/17] some more logging --- app/controllers/submissions_controller.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index 62301102..fb7ab0e2 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -103,7 +103,7 @@ class SubmissionsController < ApplicationController socket = result[:socket] socket.on :message do |event| - Rails.logger.info("Docker sending: " + event.data) + Rails.logger.info( Time.now.getutc.to_s + ": Docker sending: " + event.data) handle_message(event.data, tubesock) end @@ -112,7 +112,7 @@ class SubmissionsController < ApplicationController end tubesock.onmessage do |data| - Rails.logger.debug("Client sending: " + data) + Rails.logger.info(Time.now.getutc.to_s + ": Client sending: " + data) # Check wether the client send a JSON command and kill container # if the command is 'exit', send it to docker otherwise. begin @@ -122,9 +122,11 @@ class SubmissionsController < ApplicationController @docker_client.exit_container(result[:container]) else socket.send data + Rails.logger.info('Sent the received data to docker:' + data) end rescue JSON::ParserError socket.send data + Rails.logger.info('Sent the received data to docker:' + data) end end else @@ -157,6 +159,7 @@ class SubmissionsController < ApplicationController begin parsed = JSON.parse(message) socket.send_data message + Rails.logger.info('parse_message sent: ' + message) rescue JSON::ParserError => e # Check wether the message contains multiple lines, if true try to parse each line if ((recursive == true) && (message.include? "\n")) @@ -166,6 +169,7 @@ class SubmissionsController < ApplicationController else parsed = {'cmd'=>'write','stream'=>output_stream,'data'=>message} socket.send_data JSON.dump(parsed) + Rails.logger.info('parse_message sent: ' + JSON.dump(parsed)) end end end From bde7c21ead1ea3f58a6a325fd1d97fd07d5ae2b5 Mon Sep 17 00:00:00 2001 From: Maximilian Grundke Date: Thu, 5 Nov 2015 14:52:49 +0100 Subject: [PATCH 02/17] Fix leaking interval creating potentially unlimited unwanted requests --- app/assets/javascripts/dashboard.js | 31 ++++++++++++++++++----------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/app/assets/javascripts/dashboard.js b/app/assets/javascripts/dashboard.js index e528e5de..5f570d9e 100644 --- a/app/assets/javascripts/dashboard.js +++ b/app/assets/javascripts/dashboard.js @@ -2,6 +2,8 @@ $(function() { var CHART_START = window.vis ? vis.moment().add(-1, 'minute') : undefined; var DEFAULT_REFRESH_INTERVAL = 5000; + var refreshInterval; + var dataset; var graph; var groups; @@ -46,17 +48,21 @@ $(function() { }; var refreshData = function(callback) { - var jqxhr = $.ajax({ - dataType: 'json', - method: 'GET' - }); - jqxhr.done(function(response) { - (callback || _.noop)(response); - setGroupVisibility(response); - updateChartData(response); - updateTable(response); - requestAnimationFrame(refreshChart); - }); + if (! $.isController('dashboard')) { + clearInterval(refreshInterval); + } else { + var jqxhr = $.ajax({ + dataType: 'json', + method: 'GET' + }); + jqxhr.done(function(response) { + (callback || _.noop)(response); + setGroupVisibility(response); + updateChartData(response); + updateTable(response); + requestAnimationFrame(refreshChart); + }); + } }; var setGroupVisibility = function(response) { @@ -101,6 +107,7 @@ $(function() { initializeChart(); refreshData(); var refresh_interval = location.search.match(/interval=(\d+)/) ? parseInt(RegExp.$1) : DEFAULT_REFRESH_INTERVAL; - setInterval(refreshData, refresh_interval); + refreshInterval = setInterval(refreshData, refresh_interval); } + }); From 95c461a0554a4975c6be031384550382dd56845a Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Sun, 8 Nov 2015 12:37:11 +0100 Subject: [PATCH 03/17] Hide parametrized run and test commands. Some logging changes, comment on possible thread code to release database connections, not sure whether necessary. --- app/controllers/submissions_controller.rb | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index fb7ab0e2..3aa3adf4 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -25,6 +25,11 @@ class SubmissionsController < ApplicationController create_and_respond(object: @submission) end + def command_substitutions(filename) + {class_name: File.basename(filename, File.extname(filename)).camelize, filename: filename, module_name: File.basename(filename, File.extname(filename)).underscore} + end + private :command_substitutions + def copy_comments # copy each annotation and set the target_file.id unless(params[:annotations_arr].nil?) @@ -88,6 +93,11 @@ class SubmissionsController < ApplicationController # end hijack do |tubesock| + # probably add: + # ensure + # #guarantee that the thread is releasing the DB connection after it is done + # ActiveRecord::Base.connectionpool.releaseconnection + # end Thread.new { EventMachine.run } unless EventMachine.reactor_running? && EventMachine.reactor_thread.alive? @@ -122,11 +132,11 @@ class SubmissionsController < ApplicationController @docker_client.exit_container(result[:container]) else socket.send data - Rails.logger.info('Sent the received data to docker:' + data) + Rails.logger.debug('Sent the received client data to docker:' + data) end rescue JSON::ParserError socket.send data - Rails.logger.info('Sent the received data to docker:' + data) + Rails.logger.debug('Rescued parsing error, sent the received client data to docker:' + data) end end else @@ -147,8 +157,8 @@ class SubmissionsController < ApplicationController kill_socket(tubesock) else # Filter out information about run_command, test_command, user or working directory - run_command = @submission.execution_environment.run_command - test_command = @submission.execution_environment.test_command + run_command = @submission.execution_environment.run_command % command_substitutions(params[:filename]) + test_command = @submission.execution_environment.test_command % command_substitutions(params[:filename]) if !(/root|workspace|#{run_command}|#{test_command}/.match(message)) parse_message(message, 'stdout', tubesock) end From c958307af151e034122d5384a4fda82a83c8450f Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Sun, 8 Nov 2015 12:37:43 +0100 Subject: [PATCH 04/17] Render HTML for the exercise description --- app/views/exercises/implement.html.slim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/exercises/implement.html.slim b/app/views/exercises/implement.html.slim index d01792d1..a9ce3a54 100644 --- a/app/views/exercises/implement.html.slim +++ b/app/views/exercises/implement.html.slim @@ -4,7 +4,7 @@ span.badge.pull-right.score - p.lead = @exercise.description + p.lead = render_markdown(@exercise.description) #alert.alert.alert-danger role='alert' h4 = t('.alert.title') From 23645a60b14e5eedf8367342279f6154ad05b4ab Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Sun, 8 Nov 2015 12:38:48 +0100 Subject: [PATCH 05/17] get submission based solely on exercise, user and RFC creation timestamp in request_for_comment --- app/views/request_for_comments/show.html.erb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/views/request_for_comments/show.html.erb b/app/views/request_for_comments/show.html.erb index 7894d66f..c27e5478 100644 --- a/app/views/request_for_comments/show.html.erb +++ b/app/views/request_for_comments/show.html.erb @@ -4,6 +4,14 @@ <% user = @request_for_comment.user + submission_id = self.class.connection.execute("select id from submissions + where exercise_id = + #{@request_for_comment.exercise_id} AND + user_id = #{@request_for_comment.user_id} AND + #{@request_for_comment.user_id} > created_at + order by created_at desc + limit 1").first['id'].to_i + submission = Submission.find(submission_id) %> <%= user %> | <%= @request_for_comment.requested_at %> From 1aa877d50610953d9797cda00d8eb9d8e901314a Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Sun, 8 Nov 2015 12:39:42 +0100 Subject: [PATCH 06/17] release database connections after thread termination of the "kill after timeout thread" --- lib/docker_client.rb | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/lib/docker_client.rb b/lib/docker_client.rb index 4a353394..91e8b0b4 100644 --- a/lib/docker_client.rb +++ b/lib/docker_client.rb @@ -174,15 +174,20 @@ class DockerClient as it is impossible to determine whether further input is requested. """ @thread = Thread.new do - timeout = @execution_environment.permitted_execution_time.to_i # seconds - sleep(timeout) - if container.status != :returned - Rails.logger.info('Killing container after timeout of ' + timeout.to_s + ' seconds.') - # send timeout to the tubesock socket - if(@tubesock) - @tubesock.send_data JSON.dump({'cmd' => 'timeout'}) + begin + timeout = @execution_environment.permitted_execution_time.to_i # seconds + sleep(timeout) + if container.status != :returned + Rails.logger.info('Killing container after timeout of ' + timeout.to_s + ' seconds.') + # send timeout to the tubesock socket + if(@tubesock) + @tubesock.send_data JSON.dump({'cmd' => 'timeout'}) + end + kill_container(container) end - kill_container(container) + ensure + #guarantee that the thread is releasing the DB connection after it is done + ActiveRecord::Base.connectionpool.releaseconnection end end end From 0fd20c479b76fcf166c439870f680a0284f9dc40 Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Sun, 8 Nov 2015 23:55:45 +0100 Subject: [PATCH 07/17] set encoding utf-8 for stdout and stderr from docker --- lib/docker_client.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/docker_client.rb b/lib/docker_client.rb index 91e8b0b4..4d36b64b 100644 --- a/lib/docker_client.rb +++ b/lib/docker_client.rb @@ -303,7 +303,7 @@ class DockerClient output = container.exec(['bash', '-c', command]) Rails.logger.info "output from container.exec" Rails.logger.info output - result = {status: output[2] == 0 ? :ok : :failed, stdout: output[0].join, stderr: output[1].join} + result = {status: output[2] == 0 ? :ok : :failed, stdout: output[0].join.force_encoding('utf-8'), stderr: output[1].join.force_encoding('utf-8')} end # if we use pooling and recylce the containers, put it back. otherwise, destroy it. (DockerContainerPool.config[:active] && RECYCLE_CONTAINERS) ? self.class.return_container(container, @execution_environment) : self.class.destroy_container(container) From 589a58b520963fe25ed793f50e8947daba9a5a9a Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Sun, 8 Nov 2015 23:56:38 +0100 Subject: [PATCH 08/17] Adapted locales --- config/locales/de.yml | 2 +- config/locales/en.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/config/locales/de.yml b/config/locales/de.yml index 80d89fb8..c9a71a6e 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -288,7 +288,7 @@ de: failure: Fehlerhafte E-Mail oder Passwort. success: Sie haben sich erfolgreich angemeldet. create_through_lti: - session_with_outcome: 'Nachdem Sie die Aufgabe bearbeitet haben, wird Ihre Bewertung an %{consumer} übermittelt.' + session_with_outcome: 'Bitte beachten Sie, dass zur Gutschrift der Punkte Ihr Code nach der Bearbeitung durch Klicken auf den Button "Code zur Bewertung abgeben" eingetragen werden muss.' session_without_outcome: 'Dies ist eine Übungs-Session. Ihre Bewertung wird nicht an %{consumer} übermittelt.' destroy: link: Abmelden diff --git a/config/locales/en.yml b/config/locales/en.yml index 0103fdd0..586a4a97 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -288,7 +288,7 @@ en: failure: Invalid email or password. success: Successfully signed in. create_through_lti: - session_with_outcome: 'After you have finished the exercise, your grade will be transmitted to %{consumer}.' + session_with_outcome: 'Please click "Submit Code for Assessment" after scoring to claim your sc.' session_without_outcome: 'This is a practice session. Your grade will not be transmitted to %{consumer}.' destroy: link: Sign out From 7f3189615f367db8b9822999a5fab8827995a351 Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Mon, 9 Nov 2015 09:38:44 +0100 Subject: [PATCH 09/17] temporarily uncomment database connection removal (in order to be sure that this does not cause problems) --- lib/docker_client.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/docker_client.rb b/lib/docker_client.rb index 4d36b64b..606d5ede 100644 --- a/lib/docker_client.rb +++ b/lib/docker_client.rb @@ -174,7 +174,7 @@ class DockerClient as it is impossible to determine whether further input is requested. """ @thread = Thread.new do - begin + #begin timeout = @execution_environment.permitted_execution_time.to_i # seconds sleep(timeout) if container.status != :returned @@ -185,10 +185,10 @@ class DockerClient end kill_container(container) end - ensure - #guarantee that the thread is releasing the DB connection after it is done - ActiveRecord::Base.connectionpool.releaseconnection - end + # ensure + # #guarantee that the thread is releasing the DB connection after it is done + # ActiveRecord::Base.connectionpool.releaseconnection + # end end end From b05b24ee621576cc94f965cb4f66db82642c39c1 Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Mon, 9 Nov 2015 10:23:26 +0100 Subject: [PATCH 10/17] some more logging --- lib/docker_client.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/docker_client.rb b/lib/docker_client.rb index 606d5ede..4e28485b 100644 --- a/lib/docker_client.rb +++ b/lib/docker_client.rb @@ -162,6 +162,7 @@ class DockerClient @socket ||= create_socket(@container) # Newline required to flush @socket.send command + "\n" + Rails.logger.info('Sent command: ' + command.to_s) {status: :container_running, socket: @socket, container: @container} else {status: :container_depleted} From e927a3904126a120bf6cdadec586c86e8996505a Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Mon, 9 Nov 2015 19:26:56 +0100 Subject: [PATCH 11/17] removed calls to the docker daemon, to hopefully solve load problems. --- lib/docker_client.rb | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/docker_client.rb b/lib/docker_client.rb index 4e28485b..35ebe5bf 100644 --- a/lib/docker_client.rb +++ b/lib/docker_client.rb @@ -239,6 +239,7 @@ class DockerClient end def self.find_image_by_tag(tag) + # todo: cache this. Docker::Image.all.detect { |image| image.info['RepoTags'].flatten.include?(tag) } end @@ -252,8 +253,10 @@ class DockerClient def initialize(options = {}) @execution_environment = options[:execution_environment] - @image = self.class.find_image_by_tag(@execution_environment.docker_image) - fail(Error, "Cannot find image #{@execution_environment.docker_image}!") unless @image + # todo: eventually re-enable this if it is cached. But in the end, we do not need this. + # docker daemon got much too much load. all not 100% necessary calls to the daemon were removed. + #@image = self.class.find_image_by_tag(@execution_environment.docker_image) + #fail(Error, "Cannot find image #{@execution_environment.docker_image}!") unless @image end def self.initialize_environment @@ -261,7 +264,9 @@ class DockerClient fail(Error, 'Docker configuration missing!') end Docker.url = config[:host] if config[:host] - check_availability! + # todo: availability check disabled for performance reasons. Reconsider if this is necessary. + # docker daemon got much too much load. all not 100% necessary calls to the daemon were removed. + # check_availability! FileUtils.mkdir_p(LOCAL_WORKSPACE_ROOT) end From b1733d1a16359f42e08b676a24badebb7a219ee2 Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Tue, 10 Nov 2015 15:04:21 +0100 Subject: [PATCH 12/17] ensure database connection removal activated again --- lib/docker_client.rb | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/docker_client.rb b/lib/docker_client.rb index 35ebe5bf..18b80f56 100644 --- a/lib/docker_client.rb +++ b/lib/docker_client.rb @@ -174,22 +174,22 @@ class DockerClient We need to start a second thread to kill the websocket connection, as it is impossible to determine whether further input is requested. """ - @thread = Thread.new do - #begin - timeout = @execution_environment.permitted_execution_time.to_i # seconds - sleep(timeout) - if container.status != :returned - Rails.logger.info('Killing container after timeout of ' + timeout.to_s + ' seconds.') - # send timeout to the tubesock socket - if(@tubesock) - @tubesock.send_data JSON.dump({'cmd' => 'timeout'}) + begin + @thread = Thread.new do + timeout = @execution_environment.permitted_execution_time.to_i # seconds + sleep(timeout) + if container.status != :returned + Rails.logger.info('Killing container after timeout of ' + timeout.to_s + ' seconds.') + # send timeout to the tubesock socket + if(@tubesock) + @tubesock.send_data JSON.dump({'cmd' => 'timeout'}) + end + kill_container(container) end - kill_container(container) - end - # ensure - # #guarantee that the thread is releasing the DB connection after it is done - # ActiveRecord::Base.connectionpool.releaseconnection - # end + end + ensure + # guarantee that the thread is releasing the DB connection after it is done + ActiveRecord::Base.connectionpool.releaseconnection end end From 28bbd84153306417c989c237694722c1f39941ab Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Wed, 11 Nov 2015 00:11:34 +0100 Subject: [PATCH 13/17] remove safety check on LTI submission --- app/views/exercises/implement.html.slim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/exercises/implement.html.slim b/app/views/exercises/implement.html.slim index a9ce3a54..81f39cc0 100644 --- a/app/views/exercises/implement.html.slim +++ b/app/views/exercises/implement.html.slim @@ -76,7 +76,7 @@ br - if session[:lti_parameters].try(:has_key?, 'lis_outcome_service_url') - p.text-center = render('editor_button', classes: 'btn-lg btn-success', data: {:'data-message-confirm' => t('exercises.editor.confirm_submit'), :'data-url' => submit_exercise_path(@exercise)}, icon: 'fa fa-send', id: 'submit', label: t('exercises.editor.submit')) + p.text-center = render('editor_button', classes: 'btn-lg btn-success', data: {:'data-url' => submit_exercise_path(@exercise)}, icon: 'fa fa-send', id: 'submit', label: t('exercises.editor.submit')) - if qa_url #questions-column From 207ff90fc8bd403d261ae5473a6ab56a48646457 Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Wed, 11 Nov 2015 00:25:26 +0100 Subject: [PATCH 14/17] remove this again. it caused immediate exit. --- lib/docker_client.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/docker_client.rb b/lib/docker_client.rb index 18b80f56..654c6504 100644 --- a/lib/docker_client.rb +++ b/lib/docker_client.rb @@ -174,7 +174,7 @@ class DockerClient We need to start a second thread to kill the websocket connection, as it is impossible to determine whether further input is requested. """ - begin + #begin @thread = Thread.new do timeout = @execution_environment.permitted_execution_time.to_i # seconds sleep(timeout) @@ -187,10 +187,10 @@ class DockerClient kill_container(container) end end - ensure + #ensure # guarantee that the thread is releasing the DB connection after it is done - ActiveRecord::Base.connectionpool.releaseconnection - end + # ActiveRecord::Base.connectionpool.releaseconnection + #end end def exit_container(container) From e67e378e5912a16c0670648e11bd6c73c4733d45 Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Wed, 11 Nov 2015 00:38:04 +0100 Subject: [PATCH 15/17] add data message confirm again. this was the wrong place to delete something --- app/views/exercises/implement.html.slim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/exercises/implement.html.slim b/app/views/exercises/implement.html.slim index 81f39cc0..a9ce3a54 100644 --- a/app/views/exercises/implement.html.slim +++ b/app/views/exercises/implement.html.slim @@ -76,7 +76,7 @@ br - if session[:lti_parameters].try(:has_key?, 'lis_outcome_service_url') - p.text-center = render('editor_button', classes: 'btn-lg btn-success', data: {:'data-url' => submit_exercise_path(@exercise)}, icon: 'fa fa-send', id: 'submit', label: t('exercises.editor.submit')) + p.text-center = render('editor_button', classes: 'btn-lg btn-success', data: {:'data-message-confirm' => t('exercises.editor.confirm_submit'), :'data-url' => submit_exercise_path(@exercise)}, icon: 'fa fa-send', id: 'submit', label: t('exercises.editor.submit')) - if qa_url #questions-column From 04e51caf7ea32c297aa52a97984b51aed71daa04 Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Tue, 17 Nov 2015 16:18:29 +0100 Subject: [PATCH 16/17] Show Poolsize on ExecutionEnvironments Index Page --- app/views/execution_environments/index.html.slim | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/views/execution_environments/index.html.slim b/app/views/execution_environments/index.html.slim index 7ed2cebb..3043be9d 100644 --- a/app/views/execution_environments/index.html.slim +++ b/app/views/execution_environments/index.html.slim @@ -6,6 +6,7 @@ h1 = ExecutionEnvironment.model_name.human(count: 2) tr th = t('activerecord.attributes.execution_environment.name') th = t('activerecord.attributes.execution_environment.user') + th = t('activerecord.attributes.execution_environment.pool_size') th = t('activerecord.attributes.execution_environment.memory_limit') th = t('activerecord.attributes.execution_environment.network_enabled') th = t('activerecord.attributes.execution_environment.permitted_execution_time') @@ -16,6 +17,7 @@ h1 = ExecutionEnvironment.model_name.human(count: 2) tr td = execution_environment.name td = link_to(execution_environment.author, execution_environment.author) + td = execution_environment.pool_size td = execution_environment.memory_limit td = symbol_for(execution_environment.network_enabled) td = execution_environment.permitted_execution_time From 98a5cd618ca3cec6b37e5712b04ccb58abd17d25 Mon Sep 17 00:00:00 2001 From: Ralf Teusner Date: Wed, 18 Nov 2015 17:13:52 +0100 Subject: [PATCH 17/17] Fixed typo in locales. --- config/locales/en.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index 586a4a97..5cb09f4f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -288,7 +288,7 @@ en: failure: Invalid email or password. success: Successfully signed in. create_through_lti: - session_with_outcome: 'Please click "Submit Code for Assessment" after scoring to claim your sc.' + session_with_outcome: 'Please click "Submit Code for Assessment" after scoring to send your score %{consumer}.' session_without_outcome: 'This is a practice session. Your grade will not be transmitted to %{consumer}.' destroy: link: Sign out