From ded0e1ff1c2cd1c801d84f7796d6d774d6c0fc09 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Tue, 28 Jul 2026 12:40:44 +0900 Subject: [PATCH 1/3] Match servers registered under http when backfilling Backfill selected servers with `uri LIKE 'https://...'` while capture used a regexp accepting either scheme. Production registers 121 of 125 servers under http, so the first backfill covered 1,143 of the 11,861 failed builds in the last year and reported success. Put the predicate on `Server` as both a scope and an instance method so the SQL and Ruby forms stay together, and use it everywhere the question is just whether a server lives in the rubyci S3 bucket. Co-Authored-By: Claude Fable 5 --- app/models/log_excerpt.rb | 2 +- app/models/report.rb | 2 +- app/models/server.rb | 15 +++++++++++++++ lib/tasks/log_excerpts.rake | 2 +- test/models/log_excerpt_test.rb | 9 +++++++++ test/models/server_test.rb | 30 ++++++++++++++++++++++++++++++ 6 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 test/models/server_test.rb diff --git a/app/models/log_excerpt.rb b/app/models/log_excerpt.rb index 34a241533..a408d4ac1 100644 --- a/app/models/log_excerpt.rb +++ b/app/models/log_excerpt.rb @@ -12,7 +12,7 @@ class LogExcerpt < ApplicationRecord # (missing or empty objects) so that backfill can resume by max report_id. # Network errors are raised to the caller. def self.capture(report) - return nil unless %r{\Ahttps?://rubyci\.s3\.amazonaws\.com/}.match?(report.server&.uri.to_s) + return nil unless report.server&.rubyci_s3? content = fetch_content(report) excerpt = find_or_initialize_by(report_id: report.id) excerpt.content = content diff --git a/app/models/report.rb b/app/models/report.rb index 2aefd0936..fb374a1d0 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -289,7 +289,7 @@ def self.fetch_recent ary = [] Server.order(:id).all.each do |server| puts server.uri - if %r<\Ahttps?://rubyci.s3.amazonaws.com/>.match?(server.uri) + if server.rubyci_s3? t = Time.now n = self.get_reports_rubyci_s3(s3, server) ary << [server.uri, Time.now-t, n] diff --git a/app/models/server.rb b/app/models/server.rb index 93ce5a786..92d59b32b 100644 --- a/app/models/server.rb +++ b/app/models/server.rb @@ -3,6 +3,21 @@ class Server < ApplicationRecord validates :uri, :length => { :in => 20..200 } validates :ordinal, :numericality => true, :uniqueness => true + RUBYCI_S3_HOST = 'rubyci.s3.amazonaws.com' + RUBYCI_S3_URI = %r{\Ahttps?://#{Regexp.escape(RUBYCI_S3_HOST)}/} + + # Most rows still carry the http:// form these URIs were first registered + # with, so both schemes have to match. Keep the Ruby and SQL forms together + # or they drift apart and each caller covers a different set of servers. + scope :rubyci_s3, -> { + where("uri LIKE :http OR uri LIKE :https", + http: "http://#{RUBYCI_S3_HOST}/%", https: "https://#{RUBYCI_S3_HOST}/%") + } + + def rubyci_s3? + RUBYCI_S3_URI.match?(uri.to_s) + end + def recent_uri(branch) "#{uri.chomp('/')}/ruby-#{branch}/recent.html" end diff --git a/lib/tasks/log_excerpts.rake b/lib/tasks/log_excerpts.rake index 1f177a087..d86bf206c 100644 --- a/lib/tasks/log_excerpts.rake +++ b/lib/tasks/log_excerpts.rake @@ -35,7 +35,7 @@ namespace :log_excerpts do where(datetime: from..to). where.not(ltsv: nil). where("ltsv NOT LIKE '%result:success%'"). - where("servers.uri LIKE 'https://rubyci.s3.amazonaws.com/%'") + merge(Server.rubyci_s3) # Resume by skipping reports that already have an excerpt. Using the max # captured report_id instead would skip everything, because scan_recent_ltsv diff --git a/test/models/log_excerpt_test.rb b/test/models/log_excerpt_test.rb index 9fc9ca244..6d005fa70 100644 --- a/test/models/log_excerpt_test.rb +++ b/test/models/log_excerpt_test.rb @@ -101,6 +101,15 @@ def create_report(server, attrs = {}) assert_nil report.reload.log_excerpt end + test "capture handles servers registered under http" do + server = create_server("cap-http", uri: "http://rubyci.s3.amazonaws.com/cap-http/") + report = create_report(server) + stub_fetch_text(->(uri) { "body" }) do + LogExcerpt.capture(report) + end + assert_equal "body\nbody", report.reload.log_excerpt.content + end + test "capture is idempotent per report" do server = create_server("cap-idem") report = create_report(server) diff --git a/test/models/server_test.rb b/test/models/server_test.rb new file mode 100644 index 000000000..b2e32ffae --- /dev/null +++ b/test/models/server_test.rb @@ -0,0 +1,30 @@ +require "test_helper" + +class ServerTest < ActiveSupport::TestCase + @@ordinal = 500 + + def create_server(uri) + Server.create!(name: "srv-#{@@ordinal}", uri: uri, ordinal: (@@ordinal += 1)) + end + + test "rubyci_s3? accepts both schemes" do + assert_predicate create_server("https://rubyci.s3.amazonaws.com/ubuntu/"), :rubyci_s3? + assert_predicate create_server("http://rubyci.s3.amazonaws.com/rhel9"), :rubyci_s3? + end + + test "rubyci_s3? rejects other hosts" do + assert_not create_server("https://example.com/rubyci.s3.amazonaws.com/x").rubyci_s3? + assert_not create_server("https://rubyci.s3.amazonaws.com.evil.test/x").rubyci_s3? + assert_not create_server("http://ci.rvm.jp/chkbuild/logs/").rubyci_s3? + end + + test "rubyci_s3 scope selects the same servers as rubyci_s3?" do + servers = [ + create_server("https://rubyci.s3.amazonaws.com/scope-https/"), + create_server("http://rubyci.s3.amazonaws.com/scope-http/"), + create_server("http://ci.rvm.jp/chkbuild/scope-other/"), + ] + selected = Server.rubyci_s3.where(id: servers.map(&:id)).order(:id).to_a + assert_equal servers.select(&:rubyci_s3?), selected + end +end From bca6dfe71c0a7ccec12f548f26a6fcaa58a4f833 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Tue, 28 Jul 2026 12:41:08 +0900 Subject: [PATCH 2/3] Always fetch excerpts over TLS and skip archived objects Most server rows carry an http:// URI, so excerpts for them were fetched in the clear. Rewrite the scheme before connecting. The bucket uses S3 Intelligent-Tiering, and an object that has aged into an archive tier answers GET with 403 InvalidObjectState while HEAD still answers 200. That is not retryable without restoring the object, so treat it like a missing one. It accounted for all 26 failures of the first backfill, and sampling puts it near 2 percent of objects. Co-Authored-By: Claude Fable 5 --- app/models/log_excerpt.rb | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/app/models/log_excerpt.rb b/app/models/log_excerpt.rb index a408d4ac1..876240e63 100644 --- a/app/models/log_excerpt.rb +++ b/app/models/log_excerpt.rb @@ -35,14 +35,19 @@ def self.fetch_content(report) sections.map { |s| truncate_bytes(s, budget + slack) }.join("\n") end - # GET a gzipped text file. Returns nil on 404. S3 serves these either with - # Content-Encoding: gzip (Net::HTTP inflates the body) or as raw gzip bytes. + # GET a gzipped text file. Returns nil when the object cannot be read, which + # covers a plain 404 and an object Intelligent-Tiering has moved to an + # archive tier. Neither is retryable, so both become an empty excerpt rather + # than an error. S3 serves these either with Content-Encoding: gzip + # (Net::HTTP inflates the body) or as raw gzip bytes. def self.fetch_text(uri) - uri = URI(uri) - res = Net::HTTP.start(uri.host, uri.port, open_timeout: 10, read_timeout: 30, use_ssl: uri.scheme == "https") do |h| + # Servers are registered under both http:// and https://; always use TLS. + uri = URI(uri.to_s.sub(%r{\Ahttp://}, "https://")) + res = Net::HTTP.start(uri.host, uri.port, open_timeout: 10, read_timeout: 30, use_ssl: true) do |h| h.get(uri.path) end return nil if res.code == "404" + return nil if res.code == "403" && res.body.to_s.include?("InvalidObjectState") res.value body = res.body begin From 781a93de50ea1bedd36fc566913c40008827114a Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Tue, 28 Jul 2026 12:41:22 +0900 Subject: [PATCH 3/3] Unbuffer rake task output Ruby block-buffers stdout when it is not a tty, so a run under `heroku run:detached` showed no progress for its whole duration and then flushed every line at once on exit. Co-Authored-By: Claude Fable 5 --- lib/tasks/log_excerpts.rake | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/tasks/log_excerpts.rake b/lib/tasks/log_excerpts.rake index d86bf206c..cb7f22d87 100644 --- a/lib/tasks/log_excerpts.rake +++ b/lib/tasks/log_excerpts.rake @@ -26,6 +26,7 @@ end namespace :log_excerpts do desc "Backfill log excerpts from S3. FROM/TO (UTC date, default last 1 year), START_ID to force resume point, SLEEP seconds between reports (default 0.1), FORCE=1 to refetch reports that already have an excerpt" task :backfill => :environment do + $stdout.sync = true # detached dynos block-buffer, hiding progress until exit from = LogExcerptTaskEnv.time("FROM", 1.year.ago) to = LogExcerptTaskEnv.time("TO", Time.now) interval = LogExcerptTaskEnv.float("SLEEP", 0.1) @@ -72,6 +73,7 @@ namespace :log_excerpts do desc "Delete log excerpts whose report datetime is older than KEEP_DAYS (default 366) days. DRY_RUN=1 to only count." task :prune => :environment do + $stdout.sync = true keep_days = LogExcerptTaskEnv.integer("KEEP_DAYS", 366) abort "KEEP_DAYS must be at least 1, got #{keep_days}" if keep_days < 1 cutoff = keep_days.days.ago