diff --git a/README.md b/README.md index ef71e1dd..9a1c8ae6 100644 --- a/README.md +++ b/README.md @@ -264,6 +264,11 @@ Coverband on very high volume sites with many server processes reporting can hav See more discussion [here](https://github.com/danmayer/coverband/issues/384). +Please note that with the Redis Hash Store, everytime you load the full report, Coverband will execute `HGETALL` queries in your Redis server twice for every file in the project (once for runtime coverage and once for eager loading coverage). This shouldn't have a big impact in small to medium projects, but can be quite a hassle if your project has a few thousand files. +To help reduce the extra redis load when getting the coverage report, you can enable `get_coverage_cache` (but note that when doing that, you will always get a previous version of the report, while a cache is re-populated with a newer version). + +- Use Hash Redis Store with _get coverage cache_: `config.store = Coverband::Adapters::HashRedisStore.new(redis, get_coverage_cache: true)` + ### Clear Coverage Now that Coverband uses MD5 hashes there should be no reason to manually clear coverage unless one is testing, changing versions, possibly debugging Coverband itself. diff --git a/lib/coverband/adapters/hash_redis_store.rb b/lib/coverband/adapters/hash_redis_store.rb index aa4f6769..3f331b00 100644 --- a/lib/coverband/adapters/hash_redis_store.rb +++ b/lib/coverband/adapters/hash_redis_store.rb @@ -5,6 +5,83 @@ module Coverband module Adapters class HashRedisStore < Base + class GetCoverageNullCacheStore + def self.clear!(*_local_types); end + + def self.fetch(_local_type) + yield(0) + end + end + + class GetCoverageRedisCacheStore + LOCK_LIMIT = 60 * 30 # 30 minutes + + def initialize(redis, key_prefix) + @redis = redis + @key_prefix = [key_prefix, "get-coverage"].join(".") + end + + def fetch(local_type) + cached_result = get(local_type) + + # if no cache available, block the call and populate the cache + # if cache is available, return it and start re-populating it (with a lock) + if cached_result.nil? + value = yield(0) + result = set(local_type, JSON.generate(value)) + value + else + if lock!(local_type) + Thread.new do + begin + result = yield(deferred_time) + set(local_type, JSON.generate(result)) + ensure + unlock!(local_type) + end + end + end + JSON.parse(cached_result) + end + end + + def clear!(local_types = Coverband::TYPES) + Array(local_types).each do |local_type| + del(local_type) + unlock!(local_type) + end + end + + private + + # sleep in between to avoid holding other redis commands.. + # with a small random offset so runtime and eager types can be processed "at the same time" + def deferred_time + 3 + rand(0.0..1.0) + end + + def del(local_type) + @redis.del("#{@key_prefix}.cache.#{local_type}") + end + + def get(local_type) + @redis.get("#{@key_prefix}.cache.#{local_type}") + end + + def set(local_type, value) + @redis.set("#{@key_prefix}.cache.#{local_type}", value) + end + + # lock for at most 60 minutes + def lock!(local_type) + @redis.set("#{@key_prefix}.lock.#{local_type}", "1", nx: true, ex: LOCK_LIMIT) + end + + def unlock!(local_type) + @redis.del("#{@key_prefix}.lock.#{local_type}") + end + end + FILE_KEY = "file" FILE_LENGTH_KEY = "file_length" META_DATA_KEYS = [DATA_KEY, FIRST_UPDATED_KEY, LAST_UPDATED_KEY, FILE_HASH].freeze @@ -17,7 +94,7 @@ class HashRedisStore < Base JSON_PAYLOAD_EXPIRATION = 5 * 60 - attr_reader :redis_namespace + attr_reader :redis_namespace, :get_coverage_cache def initialize(redis, opts = {}) super() @@ -29,6 +106,13 @@ def initialize(redis, opts = {}) @ttl = opts[:ttl] @relative_file_converter = opts[:relative_file_converter] || Utils::RelativeFileConverter + + @get_coverage_cache = if opts[:get_coverage_cache] + key_prefix = [REDIS_STORAGE_FORMAT_VERSION, @redis_namespace].compact.join(".") + GetCoverageRedisCacheStore.new(redis, key_prefix) + else + GetCoverageNullCacheStore + end end def supported? @@ -45,6 +129,7 @@ def clear! file_keys = files_set @redis.del(*file_keys) if file_keys.any? @redis.del(files_key) + @get_coverage_cache.clear!(type) end self.type = old_type end @@ -54,6 +139,7 @@ def clear_file!(file) relative_path_file = @relative_file_converter.convert(file) Coverband::TYPES.each do |type| @redis.del(key(relative_path_file, type, file_hash: file_hash)) + @get_coverage_cache.clear!(type) end @redis.srem(files_key, relative_path_file) end @@ -87,12 +173,21 @@ def save_report(report) end def coverage(local_type = nil) - files_set = files_set(local_type) - @redis.pipelined { |pipeline| - files_set.each do |key| - pipeline.hgetall(key) + cached_results = @get_coverage_cache.fetch(local_type || type) do |sleep_time| + files_set = files_set(local_type) + + # use batches with a sleep in between to avoid overloading redis + files_set.each_slice(250).flat_map do |key_batch| + sleep sleep_time + @redis.pipelined do |pipeline| + key_batch.each do |key| + pipeline.hgetall(key) + end + end end - }.each_with_object({}) do |data_from_redis, hash| + end + + cached_results.each_with_object({}) do |data_from_redis, hash| add_coverage_for_file(data_from_redis, hash) end end diff --git a/test/coverband/adapters/hash_redis_store_test.rb b/test/coverband/adapters/hash_redis_store_test.rb index 27c72745..0d4c7306 100644 --- a/test/coverband/adapters/hash_redis_store_test.rb +++ b/test/coverband/adapters/hash_redis_store_test.rb @@ -192,4 +192,52 @@ def test_clear_file @store.clear_file!("app_path/dog.rb") assert_nil @store.get_coverage_report[:merged]["./dog.rb"] end + + def test_get_coverage_cache + @store = Coverband::Adapters::HashRedisStore.new( + @redis, + redis_namespace: "coverband_test", + relative_file_converter: MockRelativeFileConverter, + get_coverage_cache: true + ) + @store.get_coverage_cache.stubs(:deferred_time).returns(0) + @store.get_coverage_cache.clear! + mock_file_hash + yesterday = DateTime.now.prev_day.to_time + mock_time(yesterday) + @store.save_report( + "app_path/dog.rb" => [0, 1, 2] + ) + assert_equal( + { + "first_updated_at" => yesterday.to_i, + "last_updated_at" => yesterday.to_i, + "file_hash" => "abcd", + "data" => [0, 1, 2] + }, + @store.coverage["./dog.rb"] + ) + @store.save_report( + "app_path/dog.rb" => [0, 1, 2] + ) + assert_equal( + { + "first_updated_at" => yesterday.to_i, + "last_updated_at" => yesterday.to_i, + "file_hash" => "abcd", + "data" => [0, 1, 2] + }, + @store.coverage["./dog.rb"] + ) + sleep 0.1 # wait caching thread finish + assert_equal( + { + "first_updated_at" => yesterday.to_i, + "last_updated_at" => yesterday.to_i, + "file_hash" => "abcd", + "data" => [0, 2, 4] + }, + @store.coverage["./dog.rb"] + ) + end end