From 63e03dada7e754a92ca088c683f4189424ab34b1 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 21 Sep 2016 16:12:32 +0800 Subject: [PATCH] Make various trace methods take last_lines argument: So that we could read last few lines rather than read the entire file which could be huge. --- app/controllers/projects/builds_controller.rb | 4 +- app/models/ci/build.rb | 20 ++++---- lib/gitlab/ci/trace_reader.rb | 49 +++++++++++++++++++ spec/lib/gitlab/ci/trace_reader_spec.rb | 40 +++++++++++++++ 4 files changed, 103 insertions(+), 10 deletions(-) create mode 100644 lib/gitlab/ci/trace_reader.rb create mode 100644 spec/lib/gitlab/ci/trace_reader_spec.rb diff --git a/app/controllers/projects/builds_controller.rb b/app/controllers/projects/builds_controller.rb index f13fb1df373..6069e620ba2 100644 --- a/app/controllers/projects/builds_controller.rb +++ b/app/controllers/projects/builds_controller.rb @@ -43,7 +43,9 @@ class Projects::BuildsController < Projects::ApplicationController def trace respond_to do |format| format.json do - render json: @build.trace_with_state(params[:state].presence).merge!(id: @build.id, status: @build.status) + state = params[:state].presence + render json: @build.trace_with_state(state: state). + merge!(id: @build.id, status: @build.status) end end end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index f9aa1984c1f..13f101cbecb 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -128,13 +128,14 @@ module Ci latest_builds.where('stage_idx < ?', stage_idx) end - def trace_html - trace_with_state[:html] || '' + def trace_html(**args) + trace_with_state(**args)[:html] || '' end - def trace_with_state(state = nil) - if trace.present? - Ci::Ansi2html.convert(trace, state) + def trace_with_state(state: nil, last_lines: nil) + trace_ansi = trace(last_lines) + if trace_ansi.present? + Ci::Ansi2html.convert(trace_ansi, state) else {} end @@ -220,9 +221,10 @@ module Ci raw_trace.present? end - def raw_trace + def raw_trace(last_lines: nil) if File.exist?(trace_file_path) - File.read(trace_file_path) + Gitlab::Ci::TraceReader.new(trace_file_path). + read(last_lines: last_lines) else # backward compatibility read_attribute :trace @@ -237,8 +239,8 @@ module Ci project.ci_id && File.exist?(old_path_to_trace) end - def trace - result = raw_trace + def trace(last_lines: nil) + result = raw_trace(last_lines) if project && result.present? && project.runners_token.present? result.gsub(project.runners_token, 'xxxxxx') else diff --git a/lib/gitlab/ci/trace_reader.rb b/lib/gitlab/ci/trace_reader.rb new file mode 100644 index 00000000000..37e51536e8f --- /dev/null +++ b/lib/gitlab/ci/trace_reader.rb @@ -0,0 +1,49 @@ +module Gitlab + module Ci + # This was inspired from: http://stackoverflow.com/a/10219411/1520132 + class TraceReader + BUFFER_SIZE = 4096 + + attr_accessor :path, :buffer_size + + def initialize(new_path, buffer_size: BUFFER_SIZE) + self.path = new_path + self.buffer_size = Integer(buffer_size) + end + + def read(last_lines: nil) + if last_lines + read_last_lines(last_lines) + else + File.read(path) + end + end + + def read_last_lines(max_lines) + File.open(path) do |file| + chunks = [] + pos = lines = 0 + max = file.size + + # We want an extra line to make sure fist line has full contents + while lines <= max_lines && pos < max + pos += buffer_size + + buf = if pos <= max + file.seek(-pos, IO::SEEK_END) + file.read(buffer_size) + else # Reached the head, read only left + file.seek(0) + file.read(buffer_size - (pos - max)) + end + + lines += buf.count("\n") + chunks.unshift(buf) + end + + chunks.join.lines.last(max_lines).join + end + end + end + end +end diff --git a/spec/lib/gitlab/ci/trace_reader_spec.rb b/spec/lib/gitlab/ci/trace_reader_spec.rb new file mode 100644 index 00000000000..f06d78694d6 --- /dev/null +++ b/spec/lib/gitlab/ci/trace_reader_spec.rb @@ -0,0 +1,40 @@ +require 'spec_helper' + +describe Gitlab::Ci::TraceReader do + let(:path) { __FILE__ } + let(:lines) { File.readlines(path) } + let(:bytesize) { lines.sum(&:bytesize) } + + it 'returns last few lines' do + 10.times do + subject = build_subject + last_lines = random_lines + + expected = lines.last(last_lines).join + + expect(subject.read(last_lines: last_lines)).to eq(expected) + end + end + + it 'returns everything if trying to get too many lines' do + expect(build_subject.read(last_lines: lines.size * 2)).to eq(lines.join) + end + + it 'raises an error if not passing an integer for last_lines' do + expect do + build_subject.read(last_lines: lines) + end.to raise_error(ArgumentError) + end + + def random_lines + Random.rand(lines.size) + 1 + end + + def random_buffer + Random.rand(bytesize) + 1 + end + + def build_subject + described_class.new(__FILE__, buffer_size: random_buffer) + end +end