Commit 538a4a96 authored by Kerri Miller's avatar Kerri Miller Committed by charlie ablett

Remove gzip_diff_cache feature flag

This flag was added in 12.10, and has been default_enabled: true the
entire time without incident. Removing, but leaving the rescue's for
failed encoding attempts.
parent d242cf09
---
title: Remove :gzip_diff_cache feature flag
merge_request: 38838
author:
type: other
...@@ -152,34 +152,24 @@ module Gitlab ...@@ -152,34 +152,24 @@ module Gitlab
end end
def compose_data(json_data) def compose_data(json_data)
if ::Feature.enabled?(:gzip_diff_cache, default_enabled: true) # #compress returns ASCII-8BIT, so we need to force the encoding to
# #compress returns ASCII-8BIT, so we need to force the encoding to # UTF-8 before caching it in redis, else we risk encoding mismatch
# UTF-8 before caching it in redis, else we risk encoding mismatch # errors.
# errors. #
# ActiveSupport::Gzip.compress(json_data).force_encoding("UTF-8")
ActiveSupport::Gzip.compress(json_data).force_encoding("UTF-8")
else
json_data
end
rescue Zlib::GzipFile::Error rescue Zlib::GzipFile::Error
json_data json_data
end end
def extract_data(data) def extract_data(data)
# Since when we deploy this code, we'll be dealing with an already # Since we could be dealing with an already populated cache full of data
# populated cache full of data that isn't gzipped, we want to also # that isn't gzipped, we want to also check to see if the data is
# check to see if the data is gzipped before we attempt to #decompress # gzipped before we attempt to #decompress it, thus we check the first
# it, thus we check the first 2 bytes for "\x1F\x8B" to confirm it is # 2 bytes for "\x1F\x8B" to confirm it is a gzipped string. While a
# a gzipped string. While a non-gzipped string will raise a # non-gzipped string will raise a Zlib::GzipFile::Error, which we're
# Zlib::GzipFile::Error, which we're rescuing, we don't want to count # rescuing, we don't want to count on rescue for control flow.
# on rescue for control flow. This check can be removed in the release
# after this change is released.
# #
if ::Feature.enabled?(:gzip_diff_cache, default_enabled: true) && data[0..1] == "\x1F\x8B" data[0..1] == "\x1F\x8B" ? ActiveSupport::Gzip.decompress(data) : data
ActiveSupport::Gzip.decompress(data)
else
data
end
rescue Zlib::GzipFile::Error rescue Zlib::GzipFile::Error
data data
end end
......
...@@ -173,43 +173,17 @@ RSpec.describe Gitlab::Diff::HighlightCache, :clean_gitlab_redis_cache do ...@@ -173,43 +173,17 @@ RSpec.describe Gitlab::Diff::HighlightCache, :clean_gitlab_redis_cache do
fallback_diff_refs: diffs.fallback_diff_refs) fallback_diff_refs: diffs.fallback_diff_refs)
end end
context "feature flag :gzip_diff_cache disabled" do it "uses ActiveSupport::Gzip when reading from the cache" do
before do expect(ActiveSupport::Gzip).to receive(:decompress).at_least(:once).and_call_original
stub_feature_flags(gzip_diff_cache: true)
end
it "uses ActiveSupport::Gzip when reading from the cache" do
expect(ActiveSupport::Gzip).to receive(:decompress).at_least(:once).and_call_original
cache.write_if_empty
cache.decorate(diff_file)
end
it "uses ActiveSupport::Gzip to compress data when writing to cache" do
expect(ActiveSupport::Gzip).to receive(:compress).and_call_original
cache.send(:write_to_redis_hash, diff_hash) cache.write_if_empty
end cache.decorate(diff_file)
end end
context "feature flag :gzip_diff_cache disabled" do it "uses ActiveSupport::Gzip to compress data when writing to cache" do
before do expect(ActiveSupport::Gzip).to receive(:compress).and_call_original
stub_feature_flags(gzip_diff_cache: false)
end
it "doesn't use ActiveSupport::Gzip when reading from the cache" do
expect(ActiveSupport::Gzip).not_to receive(:decompress)
cache.write_if_empty
cache.decorate(diff_file)
end
it "doesn't use ActiveSupport::Gzip to compress data when writing to cache" do
expect(ActiveSupport::Gzip).not_to receive(:compress)
expect { cache.send(:write_to_redis_hash, diff_hash) } cache.send(:write_to_redis_hash, diff_hash)
.to change { Gitlab::Redis::Cache.with { |r| r.hgetall(cache_key) } }
end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment