From a37cfb0e0b0408bd648e05963e9e8b51aa863bec Mon Sep 17 00:00:00 2001 From: ganmacs Date: Mon, 26 Sep 2016 11:40:22 +0900 Subject: [PATCH 01/23] Implement Counter API --- lib/fluent/counter.rb | 23 +++ lib/fluent/counter/base_socket.rb | 44 +++++ lib/fluent/counter/client.rb | 197 ++++++++++++++++++++++ lib/fluent/counter/error.rb | 65 ++++++++ lib/fluent/counter/mutex_hash.rb | 97 +++++++++++ lib/fluent/counter/server.rb | 261 ++++++++++++++++++++++++++++++ lib/fluent/counter/store.rb | 146 +++++++++++++++++ lib/fluent/counter/validator.rb | 144 +++++++++++++++++ 8 files changed, 977 insertions(+) create mode 100644 lib/fluent/counter.rb create mode 100644 lib/fluent/counter/base_socket.rb create mode 100644 lib/fluent/counter/client.rb create mode 100644 lib/fluent/counter/error.rb create mode 100644 lib/fluent/counter/mutex_hash.rb create mode 100644 lib/fluent/counter/server.rb create mode 100644 lib/fluent/counter/store.rb create mode 100644 lib/fluent/counter/validator.rb diff --git a/lib/fluent/counter.rb b/lib/fluent/counter.rb new file mode 100644 index 0000000000..8bcb0008a5 --- /dev/null +++ b/lib/fluent/counter.rb @@ -0,0 +1,23 @@ +# +# Fluentd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +require 'fluent/counter/client' +require 'fluent/counter/server' + +module Fluent + module Counter + end +end diff --git a/lib/fluent/counter/base_socket.rb b/lib/fluent/counter/base_socket.rb new file mode 100644 index 0000000000..483c826ad9 --- /dev/null +++ b/lib/fluent/counter/base_socket.rb @@ -0,0 +1,44 @@ +# +# Fluentd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +require 'cool.io' +require 'fluent/msgpack_factory' + +module Fluent + module Counter + class BaseSocket < Coolio::TCPSocket + include Fluent::MessagePackFactory::Mixin + + def packed_write(data) + write pack(data) + end + + def on_read(data) + msgpack_unpacker.feed_each(data) do |d| + on_message d + end + end + + def on_message + raise NotImplementedError + end + + def pack(data) + msgpack_packer.pack(data) + end + end + end +end diff --git a/lib/fluent/counter/client.rb b/lib/fluent/counter/client.rb new file mode 100644 index 0000000000..80e12ea02c --- /dev/null +++ b/lib/fluent/counter/client.rb @@ -0,0 +1,197 @@ +# +# Fluentd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +require 'cool.io' +require 'fluent/counter/base_socket' + +module Fluent + module Counter + class Client + DEFAULT_PORT = 4321 + DEFAULT_HOST = '127.0.0.1' + ID_LIMIT_COUNT = 1 << 31 + + def initialize(loop = Coolio::Loop.new, opt = {}) + @loop = loop + @port = opt[:port] || DEFAULT_PORT + @host = opt[:host] || DEFAULT_HOST + @lsock = Connection.connect(@host, @port, method(:on_message)) + @id_mutex = Mutex.new + @log = opt[:log] || $log + @responses = {} + @id = 0 + @loop_mutex = Mutex.new + end + + def start + @loop.attach(@lsock) + self + rescue => e + @log.error e + end + + def stop + @lsock.close + end + + def establish(scope) + response = send_request('establish', nil, [scope]) + + raise response.errors if response.errors? + data = response.data + @scope = data.first + end + + # if `async` is true, return a Future object (nonblocking). + # if `async` is false, block at this method and return a Hash object. + def init(*params, options: {}) + # raise 'call `estabslish` method to set a scope before call this method' unless @scope + res = send_request('init', @scope, params, options) + options[:async] ? res : res.get + end + + def delete(*params, options: {}) + res = send_request('delete', @scope, params, options) + options[:async] ? res : res.get + end + + def inc(*params, options: {}) + res = send_request('inc', @scope, params, options) + options[:async] ? res : res.get + end + + def get(*params, options: {}) + res = send_request('get', @scope, params, options) + options[:async] ? res : res.get + end + + def reset(*params, options: {}) + res = send_request('reset', @scope, params, options) + options[:async] ? res : res.get + end + + def on_message(data) + if response = @responses.delete(data['id']) + response.set(data) + else response.nil? + # logging or raise error + end + end + + private + + def send_request(method, scope, params, opt = {}) + id = generate_id + res = Future.new(@loop, @loop_mutex) + @responses[id] = res # set a response value to this future object at `on_message` + request = build_request(method, id, scope, params, opt) + @lsock.send_data request + res + end + + def build_request(method, id, scope = nil, params = nil, options = nil) + r = { id: id, method: method } + r[:scope] = scope if scope + r[:params] = params if params + r[:options] = options if options + r + end + + def generate_id + id = 0 + @id_mutex.synchronize do + id = @id + @id += 1 + @id = 0 if ID_LIMIT_COUNT < @id + end + id + end + end + + class Connection < Fluent::Counter::BaseSocket + def initialize(io, on_message) + super(io) + @connection = false + @buffer = '' + @on_message = on_message + end + + def send_data(data) + if @connection + packed_write data + else + @buffer += pack(data) + end + end + + def on_connect + @connection = true + write @buffer + @buffer = '' + end + + def on_close + @connection = false + end + + def on_message(data) + @on_message.call(data) + end + end + + class Future + def initialize(loop, mutex) + @set = false + @result = nil + @mutex = mutex + @loop = loop + end + + def set(v) + @result = v + @set = true + end + + def errors + get['errors'] + end + + def errors? + es = errors + es && !es.empty? + end + + def data + get['data'] + end + + def get + join if @result.nil? + @result + end + + private + + def join + until @set + @mutex.synchronize do + @loop.run_once + end + end + end + end + end +end diff --git a/lib/fluent/counter/error.rb b/lib/fluent/counter/error.rb new file mode 100644 index 0000000000..1052d122c5 --- /dev/null +++ b/lib/fluent/counter/error.rb @@ -0,0 +1,65 @@ +# +# Fluentd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +module Fluent + module Counter + class BaseError < StandardError + def to_hash + { 'code' => code, 'message' => message } + end + + def code + raise NotImplementedError + end + end + + class InvalidParams < BaseError + def code + 'invalid_params' + end + end + + class UnknownKey < BaseError + def code + 'unknown_key' + end + end + + class ParseError < BaseError + def code + 'parse_error' + end + end + + class InvalidRequest < BaseError + def code + 'invalid_request' + end + end + + class MethodNotFound < BaseError + def code + 'method_not_found' + end + end + + class InternalServerError < BaseError + def code + 'internal_server_error' + end + end + end +end diff --git a/lib/fluent/counter/mutex_hash.rb b/lib/fluent/counter/mutex_hash.rb new file mode 100644 index 0000000000..49138ea78a --- /dev/null +++ b/lib/fluent/counter/mutex_hash.rb @@ -0,0 +1,97 @@ +# +# Fluentd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +module Fluent + module Counter + class MutexHash + def initialize(data_store) + @mutex = Mutex.new + @data_store = data_store + @mutex_hash = {} + # @thread = nil + # @cleaup_thread = CleaupThread.new(@store, @dict, @dict_mutex) + end + + # def start + # @cleaup_thread.start + # end + + # def stop + # @cleaup_thread.stop + # end + + def synchronize(*keys) + return if keys.empty? + + locks = {} + loop do + @mutex.synchronize do + keys.each do |key| + mutex = @mutex_hash[key] + unless mutex + v = Mutex.new + @mutex_hash[key] = v + mutex = v + end + + if mutex.try_lock + locks[key] = mutex + else + locks.values.each(&:unlock) + locks = {} # flush locked keys + break + end + end + end + + next if locks.empty? # failed to lock all keys + + locks.each do |(k, v)| + yield @data_store, k + v.unlock + end + break + end + end + + def synchronize_keys(*keys) + return if keys.empty? + keys = keys.dup + + while key = keys.shift + @mutex.lock + + mutex = @mutex_hash[key] + unless mutex + v = Mutex.new + @mutex_hash[key] = v + mutex = v + end + + if mutex.try_lock + @mutex.unlock + yield @data_store, key + mutex.unlock + else + # release global lock + @mutex.unlock + keys.push(key) # failed lock, retry this key + end + end + end + end + end +end diff --git a/lib/fluent/counter/server.rb b/lib/fluent/counter/server.rb new file mode 100644 index 0000000000..f42e760fbb --- /dev/null +++ b/lib/fluent/counter/server.rb @@ -0,0 +1,261 @@ +# +# Fluentd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +require 'cool.io' +require 'fluent/counter/base_socket' +require 'fluent/counter/validator' +require 'fluent/counter/store' +require 'fluent/counter/mutex_hash' + +module Fluent + module Counter + class Server + DEFAULT_HOST = '127.0.0.1' + DEFAULT_PORT = 4321 + + def initialize(opt = {}) + @opt = opt + @host = @opt[:host] || DEFAULT_HOST + @port = @opt[:port] || DEFAULT_PORT + @loop = @opt[:loop] || Coolio::Loop.new + + @counter = Fluent::Counter::Counter.new(opt) + @server = Coolio::TCPServer.new(@host, @port, Handler, @counter.method(:on_message)) + @thread = nil + @run = false + end + + def start + @server.attach(@loop) + @thread = Thread.new do + @loop.run(0.5) + @run = true + end + self + end + + def stop + # This `sleep` for a test to wait for a `@loop` to begin to run + sleep 0.1 unless @run + @server.close + @loop.stop + @thread.join if @thread + end + end + + class Counter + def initialize(opt = {}) + @opt = opt + @store = Fluent::Counter::Store.new + @mutex_hash = MutexHash.new(@store) + end + + def on_message(data) + errors = Validator.request(data) + unless errors.empty? + return { 'id' => data['id'], 'data' => [], 'errors' => errors } + end + + result = safe_run do + send(data['method'], data['params'], data['scope'], data['options']) + end + result.merge('id' => data['id']) + end + + private + + def establish(params, _scope, _options) + validator = Fluent::Counter::ArrayValidator.new(:empty, :scope) + valid_params, errors = validator.call(params) + res = Response.new(errors) + + valid_params.take(1).each do |param| + # TODO + res.push_data "somthing_name\t#{param}" + end + + res.to_hash + end + + def init(params, scope, options) + validator = Fluent::Counter::HashValidator.new(:empty, :name, :reset_interval) + valid_params, errors = validator.call(params) + res = Response.new(errors) + vp = valid_params.map { |e| e['name'] } + + do_init = lambda do |store, key| + begin + param = valid_params.find { |par| par['name'] == key } + v = store.init(key, scope, param, ignore: options['ignore']) + res.push_data v + rescue => e + res.push_error e + end + end + + if options['random'] + @mutex_hash.synchronize_keys(*vp, &do_init) + else + @mutex_hash.synchronize(*vp, &do_init) + end + + res.to_hash + end + + def delete(params, scope, options) + validator = Fluent::Counter::ArrayValidator.new(:empty, :key) + valid_params, errors = validator.call(params) + res = Response.new(errors) + + do_delete = lambda do |store, key| + begin + v = store.delete(key, scope) + res.push_data v + rescue => e + res.push_error e + end + end + + if options['random'] + @mutex_hash.synchronize_keys(*valid_params, &do_delete) + else + @mutex_hash.synchronize(*valid_params, &do_delete) + end + + res.to_hash + end + + def inc(params, scope, options) + validate_param = [:empty, :name, :value] + validate_param << :reset_interval if options['force'] + validator = Fluent::Counter::HashValidator.new(*validate_param) + + valid_params, errors = validator.call(params) + res = Response.new(errors) + vp = valid_params.map { |par| par['name'] } + + do_inc = lambda do |store, key| + begin + param = valid_params.find { |par| par['name'] == key } + v = store.inc(key, scope, param, force: options['force']) + res.push_data v + rescue => e + res.push_error e + end + end + + if options['random'] + @mutex_hash.synchronize_keys(*vp, &do_inc) + else + @mutex_hash.synchronize(*vp, &do_inc) + end + + res.to_hash + end + + def reset(params, scope, options) + validator = Fluent::Counter::ArrayValidator.new(:empty, :key) + valid_params, errors = validator.call(params) + res = Response.new(errors) + + do_reset = lambda do |store, key| + begin + v = store.reset(key, scope) + res.push_data v + rescue => e + res.push_error e + end + end + + if options['random'] + @mutex_hash.synchronize_keys(*valid_params, &do_reset) + else + @mutex_hash.synchronize(*valid_params, &do_reset) + end + + res.to_hash + end + + def get(params, scope, _options) + validator = Fluent::Counter::ArrayValidator.new(:empty, :key) + valid_params, errors = validator.call(params) + res = Response.new(errors) + + valid_params.each do |key| + begin + v = @store.get(key, scope, raise_error: true) + res.push_data v + rescue => e + res.push_error e + end + end + res.to_hash + end + + def safe_run + yield + rescue => e + { + 'errors' => [InternalServerError.new(e).to_hash], + 'data' => [] + } + end + + class Response + def initialize(errors = [], data = []) + @errors = errors + @data = data + end + + def push_error(error) + @errors << error + end + + def push_data(data) + @data << data + end + + def to_hash + data = @data.map { |d| d.respond_to?(:to_response_hash) ? d.to_response_hash : d } + + if @errors.empty? + { 'data' => data } + else + errors = @errors.map do |e| + error = e.respond_to?(:to_hash) ? e : InternalServerError.new(e.to_s) + error.to_hash + end + { 'data' => data, 'errors' => errors } + end + end + end + end + + class Handler < Fluent::Counter::BaseSocket + def initialize(io, on_message) + super(io) + @on_message = on_message + end + + def on_message(data) + res = @on_message.call(data) + packed_write res + rescue => e + puts "server #{e}" + end + end + end +end diff --git a/lib/fluent/counter/store.rb b/lib/fluent/counter/store.rb new file mode 100644 index 0000000000..6876026382 --- /dev/null +++ b/lib/fluent/counter/store.rb @@ -0,0 +1,146 @@ +# +# Fluentd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +require 'fluent/time' +require 'fluent/counter/error' + +module Fluent + module Counter + class Store + Value = Struct.new(:name, :total, :current, :type, :reset_interval, :last_reset_at, :last_modified_at) do + class << self + def init(data) + type = data['type'] || 'numeric' + now = EventTime.now + v = initial_value(type) + Value.new(data['name'], v, v, type, data['reset_interval'], now, now) + end + + def initial_value(type) + case type + when 'numeric', 'integer' then 0 + when 'float' then 0.0 + else raise InvalidParams.new('`type` should be integer, float, or numeric') + end + end + end + + def to_response_hash + { + 'name' => name, + 'total' => total, + 'current' => current, + 'type' => type, + 'reset_interval' => reset_interval, + 'last_reset_at' => last_reset_at, + } + end + end + + def self.gen_key(scope, key) + "#{scope}\t#{key}" + end + + def initialize + @mutex = Mutex.new + @store = {} + end + + def init(name, scope, data, ignore: false) + if v = get(name, scope) + raise InvalidParams.new("#{name} already exists in counter") unless ignore + v + else + key = Store.gen_key(scope, name) + @store[key] = Value.init(data) + end + end + + def get(name, scope, raise_error: false) + key = Store.gen_key(scope, name) + if raise_error + @store[key] or raise UnknownKey.new("`#{name}` doesn't exist in counter") + else + @store[key] + end + end + + def key?(name, scope) + key = Store.gen_key(scope, name) + @store.key?(key) + end + + def delete(name, scope) + key = Store.gen_key(scope, name) + @store.delete(key) or raise UnknownKey.new("`#{name}` doesn't exist in counter") + end + + def inc(name, scope, data, force: false) + init(name, scope, data) if force + v = get(name, scope, raise_error: true) + value = data['value'] + valida_type!(v, value) + + v.total += value + v.current += value + v.last_modified_at = EventTime.now + v + end + + def reset(name, scope) + v = get(name, scope, raise_error: true) + now = EventTime.now + success = false + old_data = v.to_response_hash + + # Check it is need reset or not + if (v.last_reset_at + v.reset_interval) <= now + success = true + v.current = Value.initial_value(v.type) + v.last_reset_at = now + v.last_modified_at = now + end + + { + 'elapsed_time' => now - old_data['last_reset_at'], + 'success' => success, + 'counter_data' => old_data + } + end + + private + + def valida_type!(v, value) + if (v.type != 'numeric') && (extract_type(value) != v.type) + raise InvalidParams.new("`type` is #{v.type}. You should pass #{v.type} value as a `value`") + end + end + + def extract_type(v) + case v + when Integer + 'integer' + when Float + 'flaot' + when Numeric + 'numeric' + else + raise InvalidParams.new("`type` should be integer, float, or numeric") + end + end + end + end +end diff --git a/lib/fluent/counter/validator.rb b/lib/fluent/counter/validator.rb new file mode 100644 index 0000000000..49b02185b1 --- /dev/null +++ b/lib/fluent/counter/validator.rb @@ -0,0 +1,144 @@ +# +# Fluentd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +require 'fluent/counter/error' + +module Fluent + module Counter + class Validator + VALID_NAME = /\A[a-z][a-zA-Z0-9\-_]*\Z/ + VALID_SCOPE_NAME = /\A[a-z][\ta-zA-Z0-9\-_]*\Z/ + VALID_METHODS = %w(establish init delete inc get reset) + + def self.request(data) + errors = [] + + if !data['id'] + errors << Fluent::Counter::InvalidRequest.new('Request should include `id`') + end + + if !data['method'] + errors << Fluent::Counter::InvalidRequest.new('Request should include `method`') + elsif !(VALID_NAME =~ data['method']) + errors << Fluent::Counter::InvalidRequest.new('`method` is the invalid format') + elsif !VALID_METHODS.include?(data['method']) + errors << Fluent::Counter::MethodNotFound.new("Unknown Method name passed: #{data['method']}") + end + + errors.map(&:to_hash) + end + + def initialize(*types) + @types = types.map(&:to_s) + @empty = @types.delete('empty') + end + + def call(data) + success = [] + errors = [] + + if @empty && data.empty? + errors << Fluent::Counter::InvalidParams.new('One or more `params` are required') + else + data.each do |d| + begin + @types.each { |type| dispatch(type, d) } + success << d + rescue => e + errors << e + end + end + end + + [success, errors] + end + + private + + def dispatch(type, data) + send("validate_#{type}!", data) + rescue NoMethodError => e + raise Fluent::Counter::InternalServerError.new(e) + end + end + + class ArrayValidator < Validator + def validate_key!(name) + unless name.is_a?(String) + raise Fluent::Counter::InvalidParams.new('The type of `key` should be String') + end + + unless VALID_NAME =~ name + raise Fluent::Counter::InvalidParams.new('`key` is the invalid format') + end + end + + def validate_scope!(name) + unless name.is_a?(String) + raise Fluent::Counter::InvalidParams.new('The type of `scope` should be String') + end + + unless VALID_SCOPE_NAME =~ name + raise Fluent::Counter::InvalidParams.new('`scope` is the invalid format') + end + end + end + + class HashValidator < Validator + def validate_name!(hash) + name = hash['name'] + unless name + raise Fluent::Counter::InvalidParams.new('`name` is required') + end + + unless name.is_a?(String) + raise Fluent::Counter::InvalidParams.new('The type of `name` should be String') + end + + unless VALID_NAME =~ name + raise Fluent::Counter::InvalidParams.new("`name` is the invalid format") + end + end + + def validate_value!(hash) + value = hash['value'] + unless value + raise Fluent::Counter::InvalidParams.new('`value` is required') + end + + unless value.is_a?(Numeric) + raise Fluent::Counter::InvalidParams.new("The type of `value` type should be Numeric") + end + end + + def validate_reset_interval!(hash) + interval = hash['reset_interval'] + + unless interval + raise Fluent::Counter::InvalidParams.new('`reset_interval` is required') + end + + unless interval.is_a?(Numeric) + raise Fluent::Counter::InvalidParams.new('The type of `reset_interval` should be Numeric') + end + + if interval < 0 + raise Fluent::Counter::InvalidParams.new('`reset_interval` should be a positive number') + end + end + end + end +end From 8d2192a5cab706985d0d4fea81dce545b6e92c2b Mon Sep 17 00:00:00 2001 From: ganmacs Date: Mon, 26 Sep 2016 11:41:10 +0900 Subject: [PATCH 02/23] Add Counter API test --- test/counter/test_client.rb | 477 +++++++++++++++++++++++++++ test/counter/test_error.rb | 44 +++ test/counter/test_mutex_hash.rb | 113 +++++++ test/counter/test_server.rb | 568 ++++++++++++++++++++++++++++++++ test/counter/test_store.rb | 345 +++++++++++++++++++ test/counter/test_validator.rb | 136 ++++++++ 6 files changed, 1683 insertions(+) create mode 100644 test/counter/test_client.rb create mode 100644 test/counter/test_error.rb create mode 100644 test/counter/test_mutex_hash.rb create mode 100644 test/counter/test_server.rb create mode 100644 test/counter/test_store.rb create mode 100644 test/counter/test_validator.rb diff --git a/test/counter/test_client.rb b/test/counter/test_client.rb new file mode 100644 index 0000000000..d47ac1c2a4 --- /dev/null +++ b/test/counter/test_client.rb @@ -0,0 +1,477 @@ +require_relative '../helper' +require 'fluent/counter/client' +require 'fluent/counter/server' +require 'timecop' + +class CounterClientTest < ::Test::Unit::TestCase + TEST_HOST = '127.0.0.1' + TEST_PORT = '8277' + + setup do + # timecop isn't compatible with EventTime + t = Time.parse('2016-09-22 16:59:59 +0900') + Timecop.freeze(t) + @now = Fluent::EventTime.now + + @options = { + host: TEST_HOST, + port: TEST_PORT, + } + + @scope = "server\tplugins" + @loop = Coolio::Loop.new + @server = Fluent::Counter::Server.new(@options).start + @client = Fluent::Counter::Client.new(@loop, @options).start + end + + teardown do + Timecop.return + @server.stop + @client.stop + end + + test 'Callable API' do + [:establish, :init, :delete, :inc, :reset, :get].each do |m| + assert_true @client.respond_to?(m) + end + end + + def extract_value_from_server(server, scope, name) + counter = server.instance_variable_get(:@counter) + store = counter.instance_variable_get(:@store).instance_variable_get(:@store) + key = Fluent::Counter::Store.gen_key(scope, name) + store[key] + end + + sub_test_case 'establish' do + test 'establish a scope' do + @client.establish(@scope) + assert_equal "somthing_name\t#{@scope}", @client.instance_variable_get(:@scope) + end + + data( + empty: '', + invalid_string: '_scope', + invalid_string2: 'Scope' + ) + test 'raise an error when passed scope is invalid' do |scope| + assert_raise do + @client.establish(scope) + end + end + end + + sub_test_case 'init' do + setup do + @client.instance_variable_set(:@scope, @scope) + end + + data( + numeric_type: [ + { 'name' => 'key', 'reset_interval' => 20, 'type' => 'numeric' }, 0 + ], + float_type: [ + { 'name' => 'key', 'reset_interval' => 20, 'type' => 'float' }, 0.0 + ], + integer_type: [ + { 'name' => 'key', 'reset_interval' => 20, 'type' => 'integer' }, 0 + ] + ) + test 'create a value' do |(param, initial_value)| + assert_nil extract_value_from_server(@server, @scope, 'key') + + response = @client.init(param) + data = response['data'].first + + assert_nil response['errors'] + assert_equal 'key', data['name'] + assert_equal 20, data['reset_interval'] + assert_equal param['type'], data['type'] + assert_equal initial_value, data['current'] + assert_equal initial_value, data['total'] + + v = extract_value_from_server(@server, @scope, 'key') + assert_equal 'key', v.name + assert_equal 20, v.reset_interval + assert_equal param['type'], v.type + assert_equal initial_value, v.total + assert_equal initial_value, v.current + end + + data( + already_exist_key: [ + { 'name' => 'key1', 'reset_interval' => 10 }, + { 'code' => 'invalid_params', 'message' => 'key1 already exists in counter' } + ], + missing_name: [ + { 'reset_interval' => 10 }, + { 'code' => 'invalid_params', 'message' => '`name` is required' }, + ], + missing_reset_interval: [ + { 'name' => 'key' }, + { 'code' => 'invalid_params', 'message' => '`reset_interval` is required' }, + ], + invalid_key: [ + { 'name' => '\tkey' }, + { 'code' => 'invalid_params', 'message' => '`name` is the invalid format' } + ] + ) + test 'return an error object' do |(param, expected_error)| + @client.init({ 'name' => 'key1', 'reset_interval' => 10 }) + resopnse = @client.init(param) + + errors = resopnse['errors'].first + + assert_empty resopnse['data'] + assert_equal expected_error, errors + end + + test 'return an error object and data object' do + param = { 'name' => 'key1', 'reset_interval' => 10 } + param2 = { 'name' => 'key2', 'reset_interval' => 10 } + @client.init(param) + + response = @client.init(param2, param) + data = response['data'].first + error = response['errors'].first + + assert_equal param2['name'], data['name'] + assert_equal param2['reset_interval'], data['reset_interval'] + + assert_equal 'invalid_params', error['code'] + assert_equal "#{param['name']} already exists in counter", error['message'] + end + + test 'return a future object when asyn option is true' do + param = { 'name' => 'key', 'reset_interval' => 10 } + r = @client.init(param, options: { async: true }) + assert_true r.is_a?(Fluent::Counter::Future) + end + end + + sub_test_case 'delete' do + setup do + @client.instance_variable_set(:@scope, @scope) + @name = 'key' + + @init_obj = { 'name' => @name, 'reset_interval' => 20, 'type' => 'numeric' } + @client.init(@init_obj) + end + + test 'delete a value' do + assert extract_value_from_server(@server, @scope, @name) + + response = @client.delete(@name) + v = response['data'].first + + assert_nil response['errors'] + assert_equal @name, v['name'] + assert_equal 20, v['reset_interval'] + assert_equal 'numeric', v['type'] + + assert_nil extract_value_from_server(@server, @scope, @name) + end + + data( + key_not_found: [ + 'key2', + { 'code' => 'unknown_key', 'message' => "`key2` doesn't exist in counter" } + ], + invalid_key: [ + '\tkey', + { 'code' => 'invalid_params', 'message' => '`key` is the invalid format' } + ] + ) + test 'return an error object' do |(param, expected_error)| + resopnse = @client.delete(param) + + errors = resopnse['errors'].first + + assert_empty resopnse['data'] + assert_equal expected_error, errors + end + + test 'return an error object and data object' do + unknow_name = 'key2' + + response = @client.delete(@name, unknow_name) + data = response['data'].first + error = response['errors'].first + + assert_equal @name, data['name'] + assert_equal @init_obj['reset_interval'], data['reset_interval'] + + assert_equal 'unknown_key', error['code'] + assert_equal "`key2` doesn't exist in counter", error['message'] + + assert_nil extract_value_from_server(@server, @scope, @name) + end + + test 'return a future object when asyn option is true' do + r = @client.delete(@name, options: { async: true }) + assert_true r.is_a?(Fluent::Counter::Future) + end + end + + sub_test_case 'inc' do + setup do + @client.instance_variable_set(:@scope, @scope) + @name = 'key' + + @init_obj = { 'name' => @name, 'reset_interval' => 20, 'type' => 'numeric' } + @client.init(@init_obj) + end + + test 'increment a value' do + v = extract_value_from_server(@server, @scope, @name) + assert_equal 0, v.total + assert_equal 0, v.current + + @client.inc({ 'name' => @name, 'value' => 10 }) + + v = extract_value_from_server(@server, @scope, @name) + assert_equal 10, v.total + assert_equal 10, v.current + assert_equal @now, v.last_modified_at + end + + test 'create and increment a value when force option is true' do + name = 'new_key' + param = { 'name' => name, 'value' => 11, 'reset_interval' => 1 } + + assert_nil extract_value_from_server(@server, @scope, name) + + @client.inc(param, options: { force: true }) + + v = extract_value_from_server(@server, @scope, name) + assert_equal name, v.name + assert_equal 1, v.reset_interval + assert_equal 11, v.total + assert_equal 11, v.total + end + + data( + not_exist_key: [ + { 'name' => 'key2', 'value' => 10 }, + { 'code' => 'unknown_key', 'message' => "`key2` doesn't exist in counter" } + ], + missing_name: [ + { 'value' => 10 }, + { 'code' => 'invalid_params', 'message' => '`name` is required' }, + ], + missing_value: [ + { 'name' => 'key' }, + { 'code' => 'invalid_params', 'message' => '`value` is required' }, + ], + invalid_key: [ + { 'name' => '\tkey' }, + { 'code' => 'invalid_params', 'message' => '`name` is the invalid format' } + ] + ) + test 'return an error object' do |(param, expected_error)| + resopnse = @client.inc(param) + + errors = resopnse['errors'].first + assert_empty resopnse['data'] + assert_equal expected_error, errors + end + + test 'return an error object and data object' do + parmas = [ + { 'name' => @name, 'value' => 10 }, + { 'name' => 'unknown_key', 'value' => 9 } + ] + response = @client.inc(*parmas) + + data = response['data'].first + error = response['errors'].first + + assert_equal @name, data['name'] + assert_equal 10, data['current'] + assert_equal 10, data['total'] + + assert_equal 'unknown_key', error['code'] + assert_equal "`unknown_key` doesn't exist in counter", error['message'] + end + + test 'return a future object when asyn option is true' do + param = { 'name' => 'key', 'value' => 10 } + r = @client.inc(param, options: { async: true }) + assert_true r.is_a?(Fluent::Counter::Future) + end + end + + sub_test_case 'get' do + setup do + @client.instance_variable_set(:@scope, @scope) + @name = 'key' + + @init_obj = { 'name' => @name, 'reset_interval' => 20, 'type' => 'numeric' } + @client.init(@init_obj) + end + + test 'get a value' do + v1 = extract_value_from_server(@server, @scope, @name) + v2 = @client.get(@name)['data'].first + + assert_equal v1.name, v2['name'] + assert_equal v1.current, v2['current'] + assert_equal v1.total, v2['total'] + assert_equal v1.type, v2['type'] + end + + data( + key_not_found: [ + 'key2', + { 'code' => 'unknown_key', 'message' => "`key2` doesn't exist in counter" } + ], + invalid_key: [ + '\tkey', + { 'code' => 'invalid_params', 'message' => '`key` is the invalid format' } + ] + ) + test 'return an error object' do |(param, expected_error)| + resopnse = @client.get(param) + + errors = resopnse['errors'].first + + assert_empty resopnse['data'] + assert_equal expected_error, errors + end + + test 'return an error object and data object' do + unknow_name = 'key2' + + response = @client.get(@name, unknow_name) + data = response['data'].first + error = response['errors'].first + + assert_equal @name, data['name'] + assert_equal @init_obj['reset_interval'], data['reset_interval'] + + assert_equal 'unknown_key', error['code'] + assert_equal "`key2` doesn't exist in counter", error['message'] + end + + test 'return a future object when asyn option is true' do + r = @client.get(@name, options: { async: true }) + assert_true r.is_a?(Fluent::Counter::Future) + end + end + + sub_test_case 'rseet' do + setup do + @client.instance_variable_set(:@scope, @scope) + @name = 'key' + + @init_obj = { 'name' => @name, 'reset_interval' => 5, 'type' => 'numeric' } + @client.init(@init_obj) + @client.inc({ 'name' => @name, 'value' => 10 }) + end + + test 'reset a value after `reset_interval` passed' do + v1 = extract_value_from_server(@server, @scope, @name) + assert_equal 10, v1.total + assert_equal 10, v1.current + assert_equal @now, v1.last_reset_at + + Timecop.travel(6) # greater than reset_interval + + v2 = @client.reset(@name) + data = v2['data'].first + + c = data['counter_data'] + + assert_equal 6, data['elapsed_time'] + assert_true data['success'] + + assert_equal 10, c['current'] + assert_equal 10, c['total'] + assert_equal @now, c['last_reset_at'] + + v1 = extract_value_from_server(@server, @scope, @name) + assert_equal 0, v1.current + assert_equal 10, v1.total + assert_equal (@now + 6), v1.last_reset_at + assert_equal (@now + 6), v1.last_modified_at + end + + test 'return a value object before `reset_interval` passed' do + v1 = extract_value_from_server(@server, @scope, @name) + assert_equal 10, v1.total + assert_equal 10, v1.current + assert_equal @now, v1.last_reset_at + + Timecop.travel(4) # less than reset_interval + + v2 = @client.reset(@name) + data = v2['data'].first + + c = data['counter_data'] + + assert_equal 4, data['elapsed_time'] + assert_equal false, data['success'] + + assert_equal 10, c['current'] + assert_equal 10, c['total'] + assert_equal @now, c['last_reset_at'] + + v1 = extract_value_from_server(@server, @scope, @name) + assert_equal 10, v1.current + assert_equal 10, v1.total + assert_equal @now, v1.last_reset_at + assert_equal @now, v1.last_reset_at + end + + data( + key_not_found: [ + 'key2', + { 'code' => 'unknown_key', 'message' => "`key2` doesn't exist in counter" } + ], + invalid_key: [ + '\tkey', + { 'code' => 'invalid_params', 'message' => '`key` is the invalid format' } + ] + ) + test 'return an error object' do |(param, expected_error)| + resopnse = @client.reset(param) + + errors = resopnse['errors'].first + + assert_empty resopnse['data'] + assert_equal expected_error, errors + end + + test 'return an error object and data object' do + unknow_name = 'key2' + + Timecop.travel(6) # greater than reset_interval + + response = @client.reset(@name, unknow_name) + data = response['data'].first + error = response['errors'].first + counter = data['counter_data'] + + assert_true data['success'] + assert_equal 6, data['elapsed_time'] + assert_equal @name, counter['name'] + assert_equal @init_obj['reset_interval'], counter['reset_interval'] + assert_equal 10, counter['total'] + assert_equal 10, counter['current'] + + assert_equal 'unknown_key', error['code'] + assert_equal "`key2` doesn't exist in counter", error['message'] + + v1 = extract_value_from_server(@server, @scope, @name) + assert_equal 0, v1.current + assert_equal 10, v1.total + assert_equal (@now + 6), v1.last_reset_at + assert_equal (@now + 6), v1.last_modified_at + end + + test 'return a future object when asyn option is true' do + r = @client.reset(@name, options: { async: true }) + assert_true r.is_a?(Fluent::Counter::Future) + end + end +end diff --git a/test/counter/test_error.rb b/test/counter/test_error.rb new file mode 100644 index 0000000000..c0d0eae874 --- /dev/null +++ b/test/counter/test_error.rb @@ -0,0 +1,44 @@ +require_relative '../helper' +require 'fluent/counter/error' + +class CounterErrorTest < ::Test::Unit::TestCase + setup do + @message = 'error message' + end + + test 'invalid_params' do + error = Fluent::Counter::InvalidParams.new(@message) + expected = { 'code' => 'invalid_params', 'message' => @message } + assert_equal expected, error.to_hash + end + + test 'unknown_key' do + error = Fluent::Counter::UnknownKey.new(@message) + expected = { 'code' => 'unknown_key', 'message' => @message } + assert_equal expected, error.to_hash + end + + test 'parase_error' do + error = Fluent::Counter::ParseError.new(@message) + expected = { 'code' => 'parse_error', 'message' => @message } + assert_equal expected, error.to_hash + end + + test 'method_not_found' do + error = Fluent::Counter::MethodNotFound.new(@message) + expected = { 'code' => 'method_not_found', 'message' => @message } + assert_equal expected, error.to_hash + end + + test 'invalid_request' do + error = Fluent::Counter::InvalidRequest.new(@message) + expected = { 'code' => 'invalid_request', 'message' => @message } + assert_equal expected, error.to_hash + end + + test 'internal_server_error' do + error = Fluent::Counter::InternalServerError.new(@message) + expected = { 'code' => 'internal_server_error', 'message' => @message } + assert_equal expected, error.to_hash + end +end diff --git a/test/counter/test_mutex_hash.rb b/test/counter/test_mutex_hash.rb new file mode 100644 index 0000000000..d03c14c60b --- /dev/null +++ b/test/counter/test_mutex_hash.rb @@ -0,0 +1,113 @@ +require_relative '../helper' +require 'fluent/counter/mutex_hash' +require 'timecop' + +class MutexHashTest < ::Test::Unit::TestCase + setup do + @store = {} + @value = 'sample value' + @counter_store_mutex = Fluent::Counter::MutexHash.new(@store) + end + + sub_test_case 'synchronize' do + test "create new mutex values if keys don't exist" do + keys = ['key', 'key1'] + + @counter_store_mutex.synchronize(*keys) do |store, k| + store[k] = @value + end + + mhash = @counter_store_mutex.instance_variable_get(:@mutex_hash) + keys.each do |key| + assert_true mhash[key].is_a?(Mutex) + assert_equal @value, @store[key] + end + end + + test 'nothing to do when an empty array passed' do + @counter_store_mutex.synchronize(*[]) do |store, k| + store[k] = @value + end + + mhash = @counter_store_mutex.instance_variable_get(:@mutex_hash) + assert_true mhash.empty? + assert_true @store.empty? + end + + test 'use a one mutex value when the same key specifed' do + key = 'key' + @counter_store_mutex.synchronize(key) do |store, k| + store[k] = @value + end + + mhash = @counter_store_mutex.instance_variable_get(:@mutex_hash) + m = mhash[key] + assert_true m.is_a?(Mutex) + assert_equal @value, @store[key] + + # access the same key once again + value2 = 'test value2' + @counter_store_mutex.synchronize(key) do |store, k| + store[k] = value2 + end + + mhash = @counter_store_mutex.instance_variable_get(:@mutex_hash) + m2 = mhash[key] + assert_true m2.is_a?(Mutex) + assert_equal value2, @store[key] + + assert_equal m, m2 + end + end + + sub_test_case 'synchronize_key' do + test "create new mutex values if keys don't exist" do + keys = ['key', 'key1'] + + @counter_store_mutex.synchronize_keys(*keys) do |store, k| + store[k] = @value + end + + mhash = @counter_store_mutex.instance_variable_get(:@mutex_hash) + keys.each do |key| + assert_true mhash[key].is_a?(Mutex) + assert_equal @value, @store[key] + end + end + + test 'nothing to do when an empty array passed' do + @counter_store_mutex.synchronize_keys(*[]) do |store, k| + store[k] = @value + end + + mhash = @counter_store_mutex.instance_variable_get(:@mutex_hash) + assert_true mhash.empty? + assert_true @store.empty? + end + + test 'use a one mutex value when the same key specifed' do + key = 'key' + @counter_store_mutex.synchronize_keys(key) do |store, k| + store[k] = @value + end + + mhash = @counter_store_mutex.instance_variable_get(:@mutex_hash) + m = mhash[key] + assert_true m.is_a?(Mutex) + assert_equal @value, @store[key] + + # access the same key once again + value2 = 'test value2' + @counter_store_mutex.synchronize_keys(key) do |store, k| + store[k] = value2 + end + + mhash = @counter_store_mutex.instance_variable_get(:@mutex_hash) + m2 = mhash[key] + assert_true m2.is_a?(Mutex) + assert_equal value2, @store[key] + + assert_equal m, m2 + end + end +end diff --git a/test/counter/test_server.rb b/test/counter/test_server.rb new file mode 100644 index 0000000000..1879c13103 --- /dev/null +++ b/test/counter/test_server.rb @@ -0,0 +1,568 @@ +require_relative '../helper' +require 'fluent/counter/server' +require 'fluent/time' +require 'flexmock/test_unit' +require 'timecop' + +class CounterCounterTest < ::Test::Unit::TestCase + setup do + # timecop isn't compatible with EventTime + t = Time.parse('2016-09-22 16:59:59 +0900') + Timecop.freeze(t) + @now = Fluent::EventTime.now + + @scope = "server\tworker\tplugin" + @counter = Fluent::Counter::Counter.new + end + + teardown do + Timecop.return + end + + def extract_value_from_counter(counter, scope, name) + store = counter.instance_variable_get(:@store).instance_variable_get(:@store) + key = "#{scope}\t#{name}" + store[key] + end + + sub_test_case 'on_message' do + data( + establish: 'establish', + init: 'init', + delete: 'delete', + inc: 'inc', + get: 'get', + reset: 'reset', + ) + test 'call valid methods' do |method| + stub(@counter).send do |_m, params, scope, options| + { 'data' => [params, scope, options] } + end + + request = { 'id' => 0, 'method' => method } + expected = { 'id' => 0, 'data' => [nil, nil, nil] } + assert_equal expected, @counter.on_message(request) + end + + data( + missing_id: [ + { 'method' => 'init' }, + { 'code' => 'invalid_request', 'message' => 'Request should include `id`' } + ], + missing_mehtod: [ + { 'id' => 0 }, + { 'code' => 'invalid_request', 'message' => 'Request should include `method`' } + ], + invalid_mehtod: [ + { 'id' => 0, 'method' => 'invalid_method'}, + { 'code' => 'method_not_found', 'message' => 'Unknown Method name passed: invalid_method'} + ] + ) + test 'invalid request' do |(request, error)| + expected = { + 'id' => request['id'], + 'data' => [], + 'errors' => [error], + } + + assert_equal expected, @counter.on_message(request) + end + + test 'return an `internal_server_error` error object when an error raises in safe_run' do + stub(@counter).send do |_m, _params, _scope, _options| + raise 'Error in safe_run' + end + + request = { 'id' => 0, 'method' => 'init' } + expected = { + 'id' => request['id'], + 'data' => [], + 'errors' => [ + { 'code' => 'internal_server_error', 'message' => 'Error in safe_run' } + ] + } + assert_equal expected, @counter.on_message(request) + end + end + + sub_test_case 'establish' do + test 'establish a scope in a counter' do + result = @counter.send('establish', ['key'], nil, nil) + expected = { 'data' => ["somthing_name\tkey"] } + assert_equal expected, result + end + + data( + empty: [[], 'One or more `params` are required'], + empty_key: [[''], '`scope` is the invalid format'], + invalid_key: [['_key'], '`scope` is the invalid format'], + ) + test 'raise an error: invalid_params' do |(params, msg)| + result = @counter.send('establish', params, nil, nil) + expected = { + 'data' => [], + 'errors' => [{ 'code' => 'invalid_params', 'message' => msg }] + } + assert_equal expected, result + end + end + + sub_test_case 'init' do + setup do + @name = 'key1' + end + + test 'create new value in a counter' do + assert_nil extract_value_from_counter(@counter, @scope, @name) + result = @counter.send('init', [{ 'name' => @name, 'reset_interval' => 1 }], @scope, {}) + assert_nil result['errors'] + + counter = result['data'].first + assert_equal 'numeric', counter['type'] + assert_equal @name, counter['name'] + assert_equal 0, counter['current'] + assert_equal 0, counter['total'] + assert_equal @now, counter['last_reset_at'] + assert extract_value_from_counter(@counter, @scope, @name) + end + + data( + numeric: 'numeric', + integer: 'integer', + float: 'float' + ) + test 'set the type of a counter value' do |type| + result = @counter.send('init', [{ 'name' => @name, 'reset_interval' => 1, 'type' => type }], @scope, {}) + counter = result['data'].first + assert_equal type, counter['type'] + + v = extract_value_from_counter(@counter, @scope, @name) + assert_equal type, v.type + end + + data( + empty: [[], 'One or more `params` are required'], + missing_name: [ + [{ 'rest_interval' => 20 }], + '`name` is required' + ], + invalid_name: [ + [{ 'name' => '_test', 'reset_interval' => 20 }], + '`name` is the invalid format' + ], + missing_interval: [ + [{ 'name' => 'test' }], + '`reset_interval` is required' + ], + minus_interval: [ + [{ 'name' => 'test', 'reset_interval' => -1 }], + '`reset_interval` should be a positive number' + ], + invalid_type: [ + [{ 'name' => 'test', 'reset_interval' => 1, 'type' => 'invalid_type' }], + '`type` should be integer, float, or numeric' + ] + ) + test 'return an error object: invalid_params' do |(params, msg)| + result = @counter.send('init', params, @scope, {}) + assert_empty result['data'] + error = result['errors'].first + assert_equal 'invalid_params', error['code'] + assert_equal msg, error['message'] + end + + test 'return error objects when passed key already exists' do + @counter.send('init', [{ 'name' => @name, 'reset_interval' => 1 }], @scope, {}) + + # call `init` to same key twice + result = @counter.send('init', [{ 'name' => @name, 'reset_interval' => 1 }], @scope, {}) + assert_empty result['data'] + error = result['errors'].first + + expected = { 'code' => 'invalid_params', 'message' => "#{@name} already exists in counter" } + assert_equal expected, error + end + + test 'return existing value when passed key already exists and ignore option is true' do + v1 = @counter.send('init', [{ 'name' => @name, 'reset_interval' => 1 }], @scope, {}) + + # call `init` to same key twice + v2 = @counter.send('init', [{ 'name' => @name, 'reset_interval' => 1 }], @scope, { 'ignore' => true }) + assert_nil v2['errors'] + assert_equal v1['data'], v2['data'] + end + + test 'call `synchronize_keys` when random option is true' do + mhash = @counter.instance_variable_get(:@mutex_hash) + mock(mhash).synchronize(@name).once + @counter.send('init', [{ 'name' => @name, 'reset_interval' => 1 }], @scope, {}) + + mhash = @counter.instance_variable_get(:@mutex_hash) + mock(mhash).synchronize_keys(@name).once + @counter.send('init', [{ 'name' => @name, 'reset_interval' => 1 }], @scope, { 'random' => true }) + end + end + + sub_test_case 'delete' do + setup do + @name = 'key1' + @counter.send('init', [{ 'name' => @name, 'reset_interval' => 20 }], @scope, {}) + end + + test 'delete a value from a counter' do + assert extract_value_from_counter(@counter, @scope, @name) + + result = @counter.send('delete', [@name], @scope, {}) + assert_nil result['errors'] + + counter = result['data'].first + assert_equal 0, counter['current'] + assert_equal 0, counter['total'] + assert_equal 'numeric', counter['type'] + assert_equal @name, counter['name'] + assert_equal @now, counter['last_reset_at'] + + assert_nil extract_value_from_counter(@counter, @scope, @name) + end + + data( + empty: [[], 'One or more `params` are required'], + empty_key: [[''], '`key` is the invalid format'], + invalid_key: [['_key'], '`key` is the invalid format'], + ) + test 'return an error object: invalid_params' do |(params, msg)| + result = @counter.send('delete', params, @scope, {}) + + assert_empty result['data'] + error = result['errors'].first + assert_equal 'invalid_params', error['code'] + assert_equal msg, error['message'] + end + + test 'return an error object: unknown_key' do + unknown_key = 'unknown_key' + result = @counter.send('delete', [unknown_key], @scope, {}) + + assert_empty result['data'] + error = result['errors'].first + assert_equal unknown_key, error['code'] + assert_equal "`#{unknown_key}` doesn't exist in counter", error['message'] + end + + test 'call `synchronize_keys` when random option is true' do + mhash = @counter.instance_variable_get(:@mutex_hash) + mock(mhash).synchronize(@name).once + @counter.send('delete', [@name], @scope, {}) + + mhash = @counter.instance_variable_get(:@mutex_hash) + mock(mhash).synchronize_keys(@name).once + @counter.send('delete', [@name], @scope, { 'random' => true }) + end + end + + sub_test_case 'inc' do + setup do + @name1 = 'key1' + @name2 = 'key2' + inc_objects = [ + { 'name' => @name1, 'reset_interval' => 20 }, + { 'name' => @name2, 'type' => 'integer', 'reset_interval' => 20 } + ] + @counter.send('init', inc_objects, @scope, {}) + end + + test 'increment or decrement a value in counter' do + result = @counter.send('inc', [{ 'name' => @name1, 'value' => 10 }], @scope, {}) + assert_nil result['errors'] + + counter = result['data'].first + assert_equal 10, counter['current'] + assert_equal 10, counter['total'] + assert_equal 'numeric', counter['type'] + assert_equal @name1, counter['name'] + assert_equal @now, counter['last_reset_at'] + + c = extract_value_from_counter(@counter, @scope, @name1) + assert_equal 10, c.current + assert_equal 10, c.total + assert_equal @now, c.last_reset_at + assert_equal @now, c.last_modified_at + end + + test 'create new value and increment/decrement its value when `force` option is true' do + new_name = 'new_key' + assert_nil extract_value_from_counter(@counter, @scope, new_name) + + v1 = @counter.send('inc', [{ 'name' => new_name, 'value' => 10 }], @scope, {}) + assert_empty v1['data'] + error = v1['errors'].first + assert_equal 'unknown_key', error['code'] + + assert_nil extract_value_from_counter(@counter, @scope, new_name) + + v2 = @counter.send( + 'inc', + [{ 'name' => new_name, 'value' => 10, 'reset_interval' => 20 }], + @scope, + { 'force' => true } + ) + + assert_nil v2['errors'] + + counter = v2['data'].first + assert_equal 10, counter['current'] + assert_equal 10, counter['total'] + assert_equal 'numeric', counter['type'] + assert_equal new_name, counter['name'] + assert_equal @now, counter['last_reset_at'] + + assert extract_value_from_counter(@counter, @scope, new_name) + end + + data( + empty: [[], 'One or more `params` are required', {}], + missing_name: [ + [{ 'value' => 10 }], + '`name` is required', {} + ], + missing_value: [ + [{ 'name' => 'key1' }], + '`value` is required', {} + ], + invalid_type: [ + [{ 'name' => 'key2', 'value' => 10.0 }], + '`type` is integer. You should pass integer value as a `value`', {} + ], + missing_reset_interval: [ + [{ 'name' => 'key1', 'value' => 1 }], + '`reset_interval` is required', + { 'force' => true } + ] + ) + test 'return an error object: invalid_params' do |(params, msg, opt)| + result = @counter.send('inc', params, @scope, opt) + assert_empty result['data'] + + error = result['errors'].first + assert_equal 'invalid_params', error['code'] + assert_equal msg, error['message'] + end + + test 'call `synchronize_keys` when random option is true' do + mhash = @counter.instance_variable_get(:@mutex_hash) + mock(mhash).synchronize(@name1).once + params = [{ 'name' => @name1, 'value' => 1 }] + @counter.send('inc', params, @scope, {}) + + mhash = @counter.instance_variable_get(:@mutex_hash) + mock(mhash).synchronize_keys(@name1).once + @counter.send('inc', params, @scope, { 'random' => true }) + end + end + + sub_test_case 'reset' do + setup do + @name = 'key' + @travel_sec = 10 + @counter.send('init', [{ 'name' => @name, 'reset_interval' => 10 }], @scope, {}) + @counter.send('inc', [{ 'name' => @name, 'value' => 10 }], @scope, {}) + end + + test 'reset a value in the counter' do + Timecop.travel(@travel_sec) + + result = @counter.send('reset', [@name], @scope, {}) + assert_nil result['errors'] + + data = result['data'].first + assert_true data['success'] + assert_equal @travel_sec, data['elapsed_time'] + + counter = data['counter_data'] + assert_equal 10, counter['current'] + assert_equal 10, counter['total'] + assert_equal 'numeric', counter['type'] + assert_equal @name, counter['name'] + assert_equal @now, counter['last_reset_at'] + + v = extract_value_from_counter(@counter, @scope, @name) + assert_equal 0, v.current + assert_equal 10, v.total + assert_equal (@now + @travel_sec), v.last_modified_at + assert_equal (@now + @travel_sec) , v.last_modified_at + end + + test 'reset a value after `reset_interval` passed' do + first_travel_sec = 5 + Timecop.travel(first_travel_sec) # jump time less than reset_interval + result = @counter.send('reset', [@name], @scope, {}) + v = result['data'].first + + assert_equal false, v['success'] + assert_equal first_travel_sec, v['elapsed_time'] + + store = extract_value_from_counter(@counter, @scope, @name) + assert_equal 10, store.current + assert_equal @now, store.last_reset_at + + # time is passed greater than reset_interval + Timecop.travel(@travel_sec) + result = @counter.send('reset', [@name], @scope, {}) + v = result['data'].first + + assert_true v['success'] + assert_equal @travel_sec + first_travel_sec, v['elapsed_time'] + + v1 = extract_value_from_counter(@counter, @scope, @name) + assert_equal 0, v1.current + assert_equal (@now + @travel_sec + first_travel_sec), v1.last_reset_at + assert_equal (@now + @travel_sec + first_travel_sec), v1.last_modified_at + end + + data( + empty: [[], 'One or more `params` are required'], + empty_key: [[''], '`key` is the invalid format'], + invalid_key: [['_key'], '`key` is the invalid format'], + ) + test 'return an error object: invalid_params' do |(params, msg)| + result = @counter.send('reset', params, @scope, {}) + assert_empty result['data'] + assert_equal 'invalid_params', result['errors'].first['code'] + assert_equal msg, result['errors'].first['message'] + end + + test 'return an error object: unknown_key' do + unknown_key = 'unknown_key' + result = @counter.send('reset', [unknown_key], @scope, {}) + + assert_empty result['data'] + error = result['errors'].first + assert_equal unknown_key, error['code'] + assert_equal "`#{unknown_key}` doesn't exist in counter", error['message'] + end + end + + sub_test_case 'get' do + setup do + @name1 = 'key1' + @name2 = 'key2' + init_objects = [ + { 'name' => @name1, 'reset_interval' => 0 }, + { 'name' => @name2, 'reset_interval' => 0 }, + ] + @counter.send('init', init_objects, @scope, {}) + end + + test 'get a counter value' do + key = @name1 + result = @counter.send('get', [key], @scope, {}) + assert_nil result['errors'] + + counter = result['data'].first + assert_equal 0, counter['current'] + assert_equal 0, counter['total'] + assert_equal 'numeric', counter['type'] + assert_equal key, counter['name'] + end + + test 'get counter values' do + result = @counter.send('get', [@name1, @name2], @scope, {}) + assert_nil result['errors'] + + counter1 = result['data'][0] + assert_equal 0, counter1['current'] + assert_equal 0, counter1['total'] + assert_equal 'numeric', counter1['type'] + assert_equal @name1, counter1['name'] + + counter2 = result['data'][1] + assert_equal 0, counter2['current'] + assert_equal 0, counter2['total'] + assert_equal 'numeric', counter2['type'] + assert_equal @name2, counter2['name'] + end + + data( + empty: [[], 'One or more `params` are required'], + empty_key: [[''], '`key` is the invalid format'], + invalid_key: [['_key'], '`key` is the invalid format'], + ) + test 'return an error object: invalid_params' do |(params, msg)| + result = @counter.send('get', params, @scope, {}) + assert_empty result['data'] + assert_equal 'invalid_params', result['errors'].first['code'] + assert_equal msg, result['errors'].first['message'] + end + + test 'return an error object: unknown_key' do + unknown_key = 'unknown_key' + result = @counter.send('get', [unknown_key], @scope, {}) + + assert_empty result['data'] + error = result['errors'].first + assert_equal unknown_key, error['code'] + assert_equal "`#{unknown_key}` doesn't exist in counter", error['message'] + end + end +end + +class CounterCounterResponseTest < ::Test::Unit::TestCase + setup do + @response = Fluent::Counter::Counter::Response.new + @errors = [ + StandardError.new('standard error'), + Fluent::Counter::InternalServerError.new('internal server error') + ] + @now = Fluent::EventTime.now + value = Fluent::Counter::Store::Value.new('name', 100, 11, 'numeric', 10, @now, @now) + @values = [value, 'test'] + end + + test 'push_error' do + @errors.each do |e| + @response.push_error(e) + end + + v = @response.instance_variable_get(:@errors) + assert_equal @errors, v + end + + test 'push_data' do + @values.each do |v| + @response.push_data v + end + + data = @response.instance_variable_get(:@data) + assert_equal @values, data + end + + test 'to_hash' do + @errors.each do |e| + @response.push_error(e) + end + + @values.each do |v| + @response.push_data v + end + + expected_errors = [ + { 'code' => 'internal_server_error', 'message' => 'standard error' }, + { 'code' => 'internal_server_error', 'message' => 'internal server error' } + ] + expected_data = [ + { + 'name' => 'name', + 'total' => 100, + 'current' => 11, + 'type' => 'numeric', + 'reset_interval' => 10, + 'last_reset_at' => @now, + }, + 'test' + ] + + hash = @response.to_hash + assert_equal expected_errors, hash['errors'] + assert_equal expected_data, hash['data'] + end +end diff --git a/test/counter/test_store.rb b/test/counter/test_store.rb new file mode 100644 index 0000000000..111b24745f --- /dev/null +++ b/test/counter/test_store.rb @@ -0,0 +1,345 @@ +require_relative '../helper' +require 'fluent/counter/store' +require 'timecop' + +class CounterStoreValueTest < ::Test::Unit::TestCase + setup do + # timecop isn't compatible with EventTime + t = Time.parse('2016-09-22 16:59:59 +0900') + Timecop.freeze(t) + @now = Fluent::EventTime.now + end + + teardown do + Timecop.return + end + + sub_test_case 'init' do + test 'default values' do + v = Fluent::Counter::Store::Value.init( + 'name' => 'name', + 'reset_interval' => 10 + ) + assert_equal 'name', v.name + assert_equal 10, v.reset_interval + assert_equal 'numeric', v.type + assert_equal @now, v.last_reset_at + assert_equal @now, v.last_modified_at + assert_equal 0, v.current + assert_equal 0, v.total + end + + data( + integer: ['integer', 0, Integer], + numeric: ['numeric', 0, Numeric], + flaot: ['float', 0.0, Float] + ) + test 'set a type' do |(type_name, value, type)| + v = Fluent::Counter::Store::Value.init( + 'type' => type_name, + 'name' => 'name', + 'reset_interval' => 10 + ) + + assert_true v.current.is_a?(type) + assert_equal value, v.current + assert_equal value, v.total + assert_equal type_name, v.type + end + + test 'raise an error when an invalid type has passed' do + assert_raise do + Fluent::Counter::Store::Value.init( + 'type' => 'invalid_type', + 'name' => 'name', + 'reset_interval' => 10 + ) + end + end + end + + sub_test_case 'initial_value' do + data( + integer: ['integer', Integer], + numeric: ['numeric', Integer], + float: ['float', Float], + ) + test 'return a initial value' do |(type, type_class)| + assert_true Fluent::Counter::Store::Value.initial_value(type).is_a?(type_class) + end + + test 'raise an error when passed string is invalid' do + assert_raise Fluent::Counter::InvalidParams do + Fluent::Counter::Store::Value.initial_value('invalid value') + end + end + end + + test 'to_response_hash' do + v = Fluent::Counter::Store::Value.init( + 'name' => 'name', + 'reset_interval' => 10 + ) + expected = { + 'name' => 'name', + 'total' => 0, + 'current' => 0, + 'type' => 'numeric', + 'reset_interval' => 10, + 'last_reset_at' => @now, + } + assert_equal expected, v.to_response_hash + end +end + +class CounterStoreTest < ::Test::Unit::TestCase + setup do + @name = 'key_name' + @scope = "server\tworker\tplugin" + + # timecop isn't compatible with EventTime + t = Time.parse('2016-09-22 16:59:59 +0900') + Timecop.freeze(t) + @now = Fluent::EventTime.now + end + + shutdown do + Timecop.return + end + + def extract_value_from_counter(counter, scope, name) + store = counter.instance_variable_get(:@store) + key = Fluent::Counter::Store.gen_key(scope, name) + store[key] + end + + sub_test_case 'init' do + setup do + @reset_interval = 10 + @store = Fluent::Counter::Store.new + @data = { 'name' => @name, 'reset_interval' => @reset_interval } + end + + test 'create new value in the counter' do + v = @store.init(@name, @scope, @data) + + assert_equal @name, v.name + assert_equal @reset_interval, v.reset_interval + + v2 = extract_value_from_counter(@store, @scope, @name) + assert_equal v, v2 + end + + test 'raise an error when a passed key already exists' do + @store.init(@name, @scope, @data) + + assert_raise Fluent::Counter::InvalidParams do + @store.init(@name, @scope, @data) + end + end + + test 'return a value when passed key already exists and a ignore option is true' do + v = @store.init(@name, @scope, @data) + v1 = extract_value_from_counter(@store, @scope, @name) + v2 = @store.init(@name, @scope, @data, ignore: true) + assert_equal v, v2 + assert_equal v1, v2 + end + end + + sub_test_case 'get' do + setup do + @store = Fluent::Counter::Store.new + data = { 'name' => @name, 'reset_interval' => 10 } + @store.init(@name, @scope, data) + end + + test 'return a value from the counter' do + v = extract_value_from_counter(@store, @scope, @name) + assert_equal v, @store.get(@name, @scope) + end + + test "return nil when a passed key doesn't exist" do + assert_equal nil, @store.get('unknown_key', @scope) + assert_equal nil, @store.get(@name, 'unknown_scope') + end + + test "raise a error when when a passed key doesn't exist and raise_error option is true" do + assert_raise Fluent::Counter::UnknownKey do + @store.get('unkonwn_key', @scope, raise_error: true) + end + + assert_raise Fluent::Counter::UnknownKey do + @store.get(@name, 'unknown_key', raise_error: true) + end + end + end + + sub_test_case 'key?' do + setup do + @store = Fluent::Counter::Store.new + data = { 'name' => @name, 'reset_interval' => 10 } + @store.init(@name, @scope, data) + end + + test 'return true when passed key exists' do + assert_true @store.key?(@name, @scope) + end + + test "return false when passed key doesn't exist" do + assert_true !@store.key?('unknown_key', @scope) + assert_true !@store.key?(@name, 'unkonwn_scope') + end + end + + sub_test_case 'delete' do + setup do + @store = Fluent::Counter::Store.new + data = { 'name' => @name, 'reset_interval' => 10 } + @init_value = @store.init(@name, @scope, data) + end + + test 'delete a value from the counter' do + v = @store.delete(@name, @scope) + assert_equal @init_value, v + assert_nil extract_value_from_counter(@store, @scope, @name) + end + + test "raise an error when passed key doesn't exist" do + assert_raise Fluent::Counter::UnknownKey do + @store.delete('unknown_key', @scope) + end + + assert_raise Fluent::Counter::UnknownKey do + @store.delete(@name, 'unknown_key') + end + end + end + + sub_test_case 'inc' do + setup do + @store = Fluent::Counter::Store.new + @init_data = { 'name' => @name, 'reset_interval' => 10 } + @travel_sec = 10 + end + + data( + positive: 10, + negative: -10 + ) + test 'increment or decrement a value in the counter' do |value| + @store.init(@name, @scope, @init_data) + Timecop.travel(@travel_sec) + v = @store.inc(@name, @scope, { 'value' => value }) + + assert_equal value, v.total + assert_equal value, v.current + assert_equal (@now + @travel_sec), v.last_modified_at + assert_equal @now, v.last_reset_at # last_reset_at doesn't change + + v1 = extract_value_from_counter(@store, @scope, @name) + assert_equal v, v1 + end + + test "raise an error when passed key doesn't exist" do + assert_raise Fluent::Counter::UnknownKey do + @store.inc('unknown_key', @scope, { 'value' => 1 }) + end + + assert_raise Fluent::Counter::UnknownKey do + @store.inc(@name, 'unknown_key', { 'value' => 1 }) + end + end + + test 'raise an error when a tyep of passed value is incompatible with a stored value' do + name2 = 'key_name2' + name3 = 'key_name3' + v = @store.init(@name, @scope, @init_data.merge('type' => 'integer')) + v2 = @store.init(name2, @scope, @init_data.merge('type' => 'float')) + v3 = @store.init(name3, @scope, @init_data.merge('type' => 'numeric')) + assert_equal 'integer', v.type + assert_equal 'float', v2.type + assert_equal 'numeric', v3.type + + assert_raise Fluent::Counter::InvalidParams do + @store.inc(@name, @scope, { 'value' => 1.1 }) + end + + assert_raise Fluent::Counter::InvalidParams do + @store.inc(name2, @scope, { 'value' => 1 }) + end + + assert_nothing_raised do + @store.inc(name3, @scope, { 'value' => 1 }) + @store.inc(name3, @scope, { 'value' => 1.0 }) + end + end + end + + sub_test_case 'reset' do + setup do + @store = Fluent::Counter::Store.new + @travel_sec = 10 + + @inc_value = 10 + @store.init(@name, @scope, { 'name' => @name, 'reset_interval' => 10 }) + @store.inc(@name, @scope, { 'value' => 10 }) + end + + test 'reset a value in the counter' do + Timecop.travel(@travel_sec) + + v = @store.reset(@name, @scope) + assert_equal @travel_sec, v['elapsed_time'] + assert_true v['success'] + counter = v['counter_data'] + + assert_equal @name, counter['name'] + assert_equal @inc_value, counter['total'] + assert_equal @inc_value, counter['current'] + assert_equal 'numeric', counter['type'] + assert_equal @now, counter['last_reset_at'] + assert_equal 10, counter['reset_interval'] + + v1 = extract_value_from_counter(@store, @scope, @name) + assert_equal 0, v1.current + assert_true v1.current.is_a?(Integer) + assert_equal @inc_value, v1.total + assert_equal (@now + @travel_sec), v1.last_reset_at + assert_equal (@now + @travel_sec), v1.last_modified_at + end + + test 'reset a value after `reset_interval` passed' do + first_travel_sec = 5 + Timecop.travel(first_travel_sec) # jump time less than reset_interval + v = @store.reset(@name, @scope) + + assert_equal false, v['success'] + assert_equal first_travel_sec, v['elapsed_time'] + store = extract_value_from_counter(@store, @scope, @name) + assert_equal 10, store.current + assert_equal @now, store.last_reset_at + + # time is passed greater than reset_interval + Timecop.travel(@travel_sec) + v = @store.reset(@name, @scope) + assert_true v['success'] + assert_equal @travel_sec + first_travel_sec, v['elapsed_time'] + + v1 = extract_value_from_counter(@store, @scope, @name) + assert_equal 0, v1.current + assert_equal (@now + @travel_sec + first_travel_sec), v1.last_reset_at + assert_equal (@now + @travel_sec + first_travel_sec), v1.last_modified_at + end + + test "raise an error when passed key doesn't exist" do + assert_raise Fluent::Counter::UnknownKey do + @store.reset('unknown_key', @scope) + end + + assert_raise Fluent::Counter::UnknownKey do + @store.reset(@name, 'unknown_key') + end + end + end +end diff --git a/test/counter/test_validator.rb b/test/counter/test_validator.rb new file mode 100644 index 0000000000..0833a4af90 --- /dev/null +++ b/test/counter/test_validator.rb @@ -0,0 +1,136 @@ +require_relative '../helper' +require 'fluent/counter/validator' + +class CounterValidatorTest < ::Test::Unit::TestCase + data( + invald_name1: '', + invald_name3: '_', + invald_name4: 'A', + invald_name5: 'a*', + invald_name6: "a\t", + invald_name7: "\n", + ) + test 'invalid name' do |invalid_name| + assert_nil(Fluent::Counter::Validator::VALID_NAME =~ invalid_name) + end + + sub_test_case 'request' do + test 'return an empty ary' do + data = { 'id' => 0, 'method' => 'init' } + errors = Fluent::Counter::Validator.request(data) + assert_empty errors + end + + data( + missing_id: [ + { 'method' => 'init' }, + { 'code' => 'invalid_request', 'message' => 'Request should include `id`' } + ], + missing_method: [ + { 'id' => 0 }, + { 'code' => 'invalid_request', 'message' => 'Request should include `method`' } + ], + invalid_method: [ + { 'id' => 0, 'method' => "A\t" }, + { 'code' => 'invalid_request', 'message' => '`method` is the invalid format' } + ], + missing_unknow_method: [ + { 'id' => 0, 'method' => 'unknown_method' }, + { 'code' => 'method_not_found', 'message' => 'Unknown Method name passed: unknown_method' } + ] + ) + test "return errors ary" do |(data, expected_error)| + errors = Fluent::Counter::Validator.request(data) + assert_equal [expected_error], errors + end + end + + sub_test_case 'call' do + test "return an error hash when passed method doesn't exist" do + v = Fluent::Counter::Validator.new(:unknow) + success, errors = v.call(['key1']) + assert_empty success + assert_equal 'internal_server_error', errors.first.to_hash['code'] + end + end + + test 'validate_empty!' do + v = Fluent::Counter::Validator.new(:empty) + success, errors = v.call([]) + assert_empty success + assert_equal [Fluent::Counter::InvalidParams.new('One or more `params` are required')], errors + end +end + +class CounterArrayValidatorTest < ::Test::Unit::TestCase + test 'validate_key!' do + ary = ['key', 100, '_'] + error_expected = [ + { 'code' => 'invalid_params', 'message' => 'The type of `key` should be String' }, + { 'code' => 'invalid_params', 'message' => '`key` is the invalid format' } + ] + v = Fluent::Counter::ArrayValidator.new(:key) + valid_params, errors = v.call(ary) + + assert_equal ['key'], valid_params + assert_equal error_expected, errors.map(&:to_hash) + end +end + +class CounterHashValidatorTest < ::Test::Unit::TestCase + test 'validate_name!' do + hash = [ + { 'name' => 'key' }, + {}, + { 'name' => 10 }, + { 'name' => '_' }] + error_expected = [ + { 'code' => 'invalid_params', 'message' => '`name` is required' }, + { 'code' => 'invalid_params', 'message' => 'The type of `name` should be String' }, + { 'code' => 'invalid_params', 'message' => '`name` is the invalid format' }, + ] + v = Fluent::Counter::HashValidator.new(:name) + success, errors = v.call(hash) + + assert_equal [{'name' => 'key'}], success + assert_equal error_expected, errors.map(&:to_hash) + end + + test 'validate_value!' do + hash = [ + { 'value' => 1 }, + { 'value' => -1 }, + {}, + { 'value' => 'str' } + ] + error_expected = [ + { 'code' => 'invalid_params', 'message' => '`value` is required' }, + { 'code' => 'invalid_params', 'message' => 'The type of `value` type should be Numeric' }, + ] + v = Fluent::Counter::HashValidator.new(:value) + valid_params, errors = v.call(hash) + + assert_equal [{ 'value' => 1 }, { 'value' => -1 }], valid_params + assert_equal error_expected, errors.map(&:to_hash) + end + + test 'validate_reset_interval!' do + hash = [ + { 'reset_interval' => 1 }, + { 'reset_interval' => 1.0 }, + {}, + { 'reset_interval' => -1 }, + { 'reset_interval' => 'str' } + ] + error_expected = [ + { 'code' => 'invalid_params', 'message' => '`reset_interval` is required' }, + { 'code' => 'invalid_params', 'message' => '`reset_interval` should be a positive number' }, + { 'code' => 'invalid_params', 'message' => 'The type of `reset_interval` should be Numeric' }, + ] + v = Fluent::Counter::HashValidator.new(:reset_interval) + valid_params, errors = v.call(hash) + + assert_equal [{ 'reset_interval' => 1 }, { 'reset_interval' => 1.0 }], valid_params + assert_equal error_expected.map(&:to_hash), errors.map(&:to_hash) + end +end From 2f0f110d2f2b4233253e7ebf31bf1f107c9c91f7 Mon Sep 17 00:00:00 2001 From: ganmacs Date: Mon, 26 Sep 2016 15:15:31 +0900 Subject: [PATCH 03/23] Pass server name as arguments --- lib/fluent/counter/server.rb | 14 ++++++++------ test/counter/test_client.rb | 7 ++++--- test/counter/test_server.rb | 13 ++++++++++--- 3 files changed, 22 insertions(+), 12 deletions(-) diff --git a/lib/fluent/counter/server.rb b/lib/fluent/counter/server.rb index f42e760fbb..cbba316b05 100644 --- a/lib/fluent/counter/server.rb +++ b/lib/fluent/counter/server.rb @@ -26,13 +26,14 @@ class Server DEFAULT_HOST = '127.0.0.1' DEFAULT_PORT = 4321 - def initialize(opt = {}) + def initialize(name, opt = {}) @opt = opt @host = @opt[:host] || DEFAULT_HOST @port = @opt[:port] || DEFAULT_PORT @loop = @opt[:loop] || Coolio::Loop.new - @counter = Fluent::Counter::Counter.new(opt) + @name = name + @counter = Fluent::Counter::Counter.new(name, opt) @server = Coolio::TCPServer.new(@host, @port, Handler, @counter.method(:on_message)) @thread = nil @run = false @@ -57,7 +58,9 @@ def stop end class Counter - def initialize(opt = {}) + def initialize(name, opt = {}) + @name = name + raise 'Counter server name is invalid' unless Validator::VALID_NAME =~ name @opt = opt @store = Fluent::Counter::Store.new @mutex_hash = MutexHash.new(@store) @@ -82,9 +85,8 @@ def establish(params, _scope, _options) valid_params, errors = validator.call(params) res = Response.new(errors) - valid_params.take(1).each do |param| - # TODO - res.push_data "somthing_name\t#{param}" + if scope = valid_params.first + res.push_data "#{@name}\t#{scope}" end res.to_hash diff --git a/test/counter/test_client.rb b/test/counter/test_client.rb index d47ac1c2a4..05259b376f 100644 --- a/test/counter/test_client.rb +++ b/test/counter/test_client.rb @@ -18,9 +18,10 @@ class CounterClientTest < ::Test::Unit::TestCase port: TEST_PORT, } - @scope = "server\tplugins" + @server_name = 'server1' + @scope = "worker1\tplugin1" @loop = Coolio::Loop.new - @server = Fluent::Counter::Server.new(@options).start + @server = Fluent::Counter::Server.new(@server_name, @options).start @client = Fluent::Counter::Client.new(@loop, @options).start end @@ -46,7 +47,7 @@ def extract_value_from_server(server, scope, name) sub_test_case 'establish' do test 'establish a scope' do @client.establish(@scope) - assert_equal "somthing_name\t#{@scope}", @client.instance_variable_get(:@scope) + assert_equal "#{@server_name}\t#{@scope}", @client.instance_variable_get(:@scope) end data( diff --git a/test/counter/test_server.rb b/test/counter/test_server.rb index 1879c13103..4d9513f104 100644 --- a/test/counter/test_server.rb +++ b/test/counter/test_server.rb @@ -12,7 +12,8 @@ class CounterCounterTest < ::Test::Unit::TestCase @now = Fluent::EventTime.now @scope = "server\tworker\tplugin" - @counter = Fluent::Counter::Counter.new + @server_name = 'server1' + @counter = Fluent::Counter::Counter.new(@server_name) end teardown do @@ -25,6 +26,12 @@ def extract_value_from_counter(counter, scope, name) store[key] end + test 'raise an error when server name is invalid' do + assert_raise do + Fluent::Counter::Counter.new("\tinvalid_name") + end + end + sub_test_case 'on_message' do data( establish: 'establish', @@ -88,7 +95,7 @@ def extract_value_from_counter(counter, scope, name) sub_test_case 'establish' do test 'establish a scope in a counter' do result = @counter.send('establish', ['key'], nil, nil) - expected = { 'data' => ["somthing_name\tkey"] } + expected = { 'data' => ["#{@server_name}\tkey"] } assert_equal expected, result end @@ -101,7 +108,7 @@ def extract_value_from_counter(counter, scope, name) result = @counter.send('establish', params, nil, nil) expected = { 'data' => [], - 'errors' => [{ 'code' => 'invalid_params', 'message' => msg }] + 'errors' => [{ 'code' => 'invalid_params', 'message' => msg }] } assert_equal expected, result end From 3f0c275d7ad88c7684d3811c5437b6ab7e6bb024 Mon Sep 17 00:00:00 2001 From: ganmacs Date: Mon, 26 Sep 2016 16:30:08 +0900 Subject: [PATCH 04/23] Enable a logger in Server and Client --- lib/fluent/counter/client.rb | 9 +++++---- lib/fluent/counter/server.rb | 22 +++++++++++++++------- lib/fluent/counter/validator.rb | 1 + test/counter/test_client.rb | 21 +++++++++++++++++++++ test/counter/test_server.rb | 8 +++++++- 5 files changed, 49 insertions(+), 12 deletions(-) diff --git a/lib/fluent/counter/client.rb b/lib/fluent/counter/client.rb index 80e12ea02c..117a8d4d4f 100644 --- a/lib/fluent/counter/client.rb +++ b/lib/fluent/counter/client.rb @@ -83,21 +83,22 @@ def reset(*params, options: {}) options[:async] ? res : res.get end + private + def on_message(data) if response = @responses.delete(data['id']) response.set(data) - else response.nil? - # logging or raise error + else + @log.warn("Receiving missing id data: #{data}") end end - private - def send_request(method, scope, params, opt = {}) id = generate_id res = Future.new(@loop, @loop_mutex) @responses[id] = res # set a response value to this future object at `on_message` request = build_request(method, id, scope, params, opt) + @log.debug(request) if @log @lsock.send_data request res end diff --git a/lib/fluent/counter/server.rb b/lib/fluent/counter/server.rb index cbba316b05..286c588e69 100644 --- a/lib/fluent/counter/server.rb +++ b/lib/fluent/counter/server.rb @@ -31,9 +31,10 @@ def initialize(name, opt = {}) @host = @opt[:host] || DEFAULT_HOST @port = @opt[:port] || DEFAULT_PORT @loop = @opt[:loop] || Coolio::Loop.new + @log = @opt[:log] || $log @name = name - @counter = Fluent::Counter::Counter.new(name, opt) + @counter = Fluent::Counter::Counter.new(name, @log) @server = Coolio::TCPServer.new(@host, @port, Handler, @counter.method(:on_message)) @thread = nil @run = false @@ -58,10 +59,10 @@ def stop end class Counter - def initialize(name, opt = {}) + def initialize(name, log) @name = name raise 'Counter server name is invalid' unless Validator::VALID_NAME =~ name - @opt = opt + @log = log @store = Fluent::Counter::Store.new @mutex_hash = MutexHash.new(@store) end @@ -76,6 +77,8 @@ def on_message(data) send(data['method'], data['params'], data['scope'], data['options']) end result.merge('id' => data['id']) + rescue => e + @log.error e.to_s end private @@ -86,7 +89,9 @@ def establish(params, _scope, _options) res = Response.new(errors) if scope = valid_params.first - res.push_data "#{@name}\t#{scope}" + new_scope = "#{@name}\t#{scope}" + res.push_data new_scope + @log.debug("Establish new key: #{new_scope}") if @log end res.to_hash @@ -102,6 +107,7 @@ def init(params, scope, options) begin param = valid_params.find { |par| par['name'] == key } v = store.init(key, scope, param, ignore: options['ignore']) + @log.debug("Create new key: #{param['name']}") if @log res.push_data v rescue => e res.push_error e @@ -125,6 +131,7 @@ def delete(params, scope, options) do_delete = lambda do |store, key| begin v = store.delete(key, scope) + @log.debug("delete a key: #{key}") if @log res.push_data v rescue => e res.push_error e @@ -153,6 +160,7 @@ def inc(params, scope, options) begin param = valid_params.find { |par| par['name'] == key } v = store.inc(key, scope, param, force: options['force']) + @log.debug("Increment #{key} by #{param['value']}") if @log res.push_data v rescue => e res.push_error e @@ -176,6 +184,7 @@ def reset(params, scope, options) do_reset = lambda do |store, key| begin v = store.reset(key, scope) + @log.debug("Reset #{key}'s' counter value") if @log res.push_data v rescue => e res.push_error e @@ -199,6 +208,7 @@ def get(params, scope, _options) valid_params.each do |key| begin v = @store.get(key, scope, raise_error: true) + @log.debug("Get counter value: #{key}") if @log res.push_data v rescue => e res.push_error e @@ -254,9 +264,7 @@ def initialize(io, on_message) def on_message(data) res = @on_message.call(data) - packed_write res - rescue => e - puts "server #{e}" + packed_write res if res end end end diff --git a/lib/fluent/counter/validator.rb b/lib/fluent/counter/validator.rb index 49b02185b1..e6fa9ed6a4 100644 --- a/lib/fluent/counter/validator.rb +++ b/lib/fluent/counter/validator.rb @@ -25,6 +25,7 @@ class Validator def self.request(data) errors = [] + raise "Recieved data is not Hash: #{data}" unless data.is_a?(Hash) if !data['id'] errors << Fluent::Counter::InvalidRequest.new('Request should include `id`') diff --git a/test/counter/test_client.rb b/test/counter/test_client.rb index 05259b376f..1b624f3e6d 100644 --- a/test/counter/test_client.rb +++ b/test/counter/test_client.rb @@ -1,6 +1,7 @@ require_relative '../helper' require 'fluent/counter/client' require 'fluent/counter/server' +require 'flexmock/test_unit' require 'timecop' class CounterClientTest < ::Test::Unit::TestCase @@ -16,6 +17,7 @@ class CounterClientTest < ::Test::Unit::TestCase @options = { host: TEST_HOST, port: TEST_PORT, + log: $log, } @server_name = 'server1' @@ -37,6 +39,25 @@ class CounterClientTest < ::Test::Unit::TestCase end end + sub_test_case 'on_message' do + setup do + @future = flexmock('future') + @client.instance_variable_set(:@responses, { 1 => @future }) + end + + test 'call a set method to a corresponding object' do + @future.should_receive(:set).once.with(Hash) + + @client.send(:on_message, {'id' => 1}) + end + + test "output a warning log when passed id doesn't exist" do + data = { 'id' => 2 } + mock($log).warn("Receiving missing id data: #{data}") + @client.send(:on_message, data) + end + end + def extract_value_from_server(server, scope, name) counter = server.instance_variable_get(:@counter) store = counter.instance_variable_get(:@store).instance_variable_get(:@store) diff --git a/test/counter/test_server.rb b/test/counter/test_server.rb index 4d9513f104..68b4e66574 100644 --- a/test/counter/test_server.rb +++ b/test/counter/test_server.rb @@ -13,7 +13,7 @@ class CounterCounterTest < ::Test::Unit::TestCase @scope = "server\tworker\tplugin" @server_name = 'server1' - @counter = Fluent::Counter::Counter.new(@server_name) + @counter = Fluent::Counter::Counter.new(@server_name, $log) end teardown do @@ -90,6 +90,12 @@ def extract_value_from_counter(counter, scope, name) } assert_equal expected, @counter.on_message(request) end + + test 'output an error log when passed data is not Hash' do + data = 'this is not a hash' + mock($log).error("Recieved data is not Hash: #{data}") + @counter.on_message(data) + end end sub_test_case 'establish' do From a49eb7834c624fc8b9c02856900ce32406f66b5c Mon Sep 17 00:00:00 2001 From: ganmacs Date: Mon, 26 Sep 2016 18:02:18 +0900 Subject: [PATCH 05/23] Fix misc * types, Description or etc --- lib/fluent/counter/base_socket.rb | 4 +- lib/fluent/counter/client.rb | 19 ++-- lib/fluent/counter/server.rb | 16 +-- lib/fluent/counter/store.rb | 16 ++- lib/fluent/counter/validator.rb | 6 +- test/counter/test_client.rb | 158 +++++++++++++++++------------- test/counter/test_error.rb | 2 +- test/counter/test_mutex_hash.rb | 4 +- test/counter/test_server.rb | 12 +-- test/counter/test_store.rb | 8 +- test/counter/test_validator.rb | 27 ++--- 11 files changed, 148 insertions(+), 124 deletions(-) diff --git a/lib/fluent/counter/base_socket.rb b/lib/fluent/counter/base_socket.rb index 483c826ad9..7d088d660a 100644 --- a/lib/fluent/counter/base_socket.rb +++ b/lib/fluent/counter/base_socket.rb @@ -32,10 +32,12 @@ def on_read(data) end end - def on_message + def on_message(data) raise NotImplementedError end + private + def pack(data) msgpack_packer.pack(data) end diff --git a/lib/fluent/counter/client.rb b/lib/fluent/counter/client.rb index 117a8d4d4f..7b49bb2328 100644 --- a/lib/fluent/counter/client.rb +++ b/lib/fluent/counter/client.rb @@ -28,37 +28,37 @@ def initialize(loop = Coolio::Loop.new, opt = {}) @loop = loop @port = opt[:port] || DEFAULT_PORT @host = opt[:host] || DEFAULT_HOST - @lsock = Connection.connect(@host, @port, method(:on_message)) - @id_mutex = Mutex.new @log = opt[:log] || $log + @conn = Connection.connect(@host, @port, method(:on_message)) @responses = {} @id = 0 + @id_mutex = Mutex.new @loop_mutex = Mutex.new end def start - @loop.attach(@lsock) + @loop.attach(@conn) self rescue => e @log.error e end def stop - @lsock.close + @conn.close end def establish(scope) response = send_request('establish', nil, [scope]) - raise response.errors if response.errors? + raise response.errors.first if response.errors? data = response.data @scope = data.first end # if `async` is true, return a Future object (nonblocking). - # if `async` is false, block at this method and return a Hash object. + # if `async` is false or missing, block at this method and return a Hash object. def init(*params, options: {}) - # raise 'call `estabslish` method to set a scope before call this method' unless @scope + # raise 'call `establish` method to set a scope before call this method' unless @scope res = send_request('init', @scope, params, options) options[:async] ? res : res.get end @@ -96,10 +96,10 @@ def on_message(data) def send_request(method, scope, params, opt = {}) id = generate_id res = Future.new(@loop, @loop_mutex) - @responses[id] = res # set a response value to this future object at `on_message` + @responses[id] = res # set a response value to this future object at `on_message` request = build_request(method, id, scope, params, opt) @log.debug(request) if @log - @lsock.send_data request + @conn.send_data request res end @@ -180,6 +180,7 @@ def data end def get + # Block until `set` method is called and @result is set a value join if @result.nil? @result end diff --git a/lib/fluent/counter/server.rb b/lib/fluent/counter/server.rb index 286c588e69..0a07e77cab 100644 --- a/lib/fluent/counter/server.rb +++ b/lib/fluent/counter/server.rb @@ -27,13 +27,13 @@ class Server DEFAULT_PORT = 4321 def initialize(name, opt = {}) + @name = name @opt = opt @host = @opt[:host] || DEFAULT_HOST @port = @opt[:port] || DEFAULT_PORT @loop = @opt[:loop] || Coolio::Loop.new @log = @opt[:log] || $log - @name = name @counter = Fluent::Counter::Counter.new(name, @log) @server = Coolio::TCPServer.new(@host, @port, Handler, @counter.method(:on_message)) @thread = nil @@ -60,8 +60,8 @@ def stop class Counter def initialize(name, log) - @name = name raise 'Counter server name is invalid' unless Validator::VALID_NAME =~ name + @name = name @log = log @store = Fluent::Counter::Store.new @mutex_hash = MutexHash.new(@store) @@ -101,7 +101,7 @@ def init(params, scope, options) validator = Fluent::Counter::HashValidator.new(:empty, :name, :reset_interval) valid_params, errors = validator.call(params) res = Response.new(errors) - vp = valid_params.map { |e| e['name'] } + keys = valid_params.map { |e| e['name'] } do_init = lambda do |store, key| begin @@ -115,9 +115,9 @@ def init(params, scope, options) end if options['random'] - @mutex_hash.synchronize_keys(*vp, &do_init) + @mutex_hash.synchronize_keys(*keys, &do_init) else - @mutex_hash.synchronize(*vp, &do_init) + @mutex_hash.synchronize(*keys, &do_init) end res.to_hash @@ -154,7 +154,7 @@ def inc(params, scope, options) valid_params, errors = validator.call(params) res = Response.new(errors) - vp = valid_params.map { |par| par['name'] } + keys = valid_params.map { |par| par['name'] } do_inc = lambda do |store, key| begin @@ -168,9 +168,9 @@ def inc(params, scope, options) end if options['random'] - @mutex_hash.synchronize_keys(*vp, &do_inc) + @mutex_hash.synchronize_keys(*keys, &do_inc) else - @mutex_hash.synchronize(*vp, &do_inc) + @mutex_hash.synchronize(*keys, &do_inc) end res.to_hash diff --git a/lib/fluent/counter/store.rb b/lib/fluent/counter/store.rb index 6876026382..677770f0bd 100644 --- a/lib/fluent/counter/store.rb +++ b/lib/fluent/counter/store.rb @@ -55,7 +55,6 @@ def self.gen_key(scope, key) end def initialize - @mutex = Mutex.new @store = {} end @@ -92,7 +91,7 @@ def inc(name, scope, data, force: false) init(name, scope, data) if force v = get(name, scope, raise_error: true) value = data['value'] - valida_type!(v, value) + valid_type!(v, value) v.total += value v.current += value @@ -106,7 +105,7 @@ def reset(name, scope) success = false old_data = v.to_response_hash - # Check it is need reset or not + # Does it need reset? if (v.last_reset_at + v.reset_interval) <= now success = true v.current = Value.initial_value(v.type) @@ -123,18 +122,17 @@ def reset(name, scope) private - def valida_type!(v, value) - if (v.type != 'numeric') && (extract_type(value) != v.type) - raise InvalidParams.new("`type` is #{v.type}. You should pass #{v.type} value as a `value`") - end + def valid_type!(v, value) + return unless (v.type != 'numeric') && (type_str(value) != v.type) + raise InvalidParams.new("`type` is #{v.type}. You should pass #{v.type} value as a `value`") end - def extract_type(v) + def type_str(v) case v when Integer 'integer' when Float - 'flaot' + 'float' when Numeric 'numeric' else diff --git a/lib/fluent/counter/validator.rb b/lib/fluent/counter/validator.rb index e6fa9ed6a4..94ab6d2465 100644 --- a/lib/fluent/counter/validator.rb +++ b/lib/fluent/counter/validator.rb @@ -25,9 +25,9 @@ class Validator def self.request(data) errors = [] - raise "Recieved data is not Hash: #{data}" unless data.is_a?(Hash) + raise "Received data is not Hash: #{data}" unless data.is_a?(Hash) - if !data['id'] + unless data['id'] errors << Fluent::Counter::InvalidRequest.new('Request should include `id`') end @@ -36,7 +36,7 @@ def self.request(data) elsif !(VALID_NAME =~ data['method']) errors << Fluent::Counter::InvalidRequest.new('`method` is the invalid format') elsif !VALID_METHODS.include?(data['method']) - errors << Fluent::Counter::MethodNotFound.new("Unknown Method name passed: #{data['method']}") + errors << Fluent::Counter::MethodNotFound.new("Unknown method name passed: #{data['method']}") end errors.map(&:to_hash) diff --git a/test/counter/test_client.rb b/test/counter/test_client.rb index 1b624f3e6d..6390254997 100644 --- a/test/counter/test_client.rb +++ b/test/counter/test_client.rb @@ -133,21 +133,32 @@ def extract_value_from_server(server, scope, name) { 'name' => 'key' }, { 'code' => 'invalid_params', 'message' => '`reset_interval` is required' }, ], - invalid_key: [ + invalid_name: [ { 'name' => '\tkey' }, { 'code' => 'invalid_params', 'message' => '`name` is the invalid format' } ] ) test 'return an error object' do |(param, expected_error)| @client.init({ 'name' => 'key1', 'reset_interval' => 10 }) - resopnse = @client.init(param) + response = @client.init(param) - errors = resopnse['errors'].first + errors = response['errors'].first - assert_empty resopnse['data'] + assert_empty response['data'] assert_equal expected_error, errors end + test 'return an existing value when passed key already exists and ignore option is true' do + res1 = @client.init({ 'name' => 'key1', 'reset_interval' => 10 }) + + res2 = nil + assert_nothing_raised do + res2 = @client.init({ 'name' => 'key1', 'reset_interval' => 10 }, options: { ignore: true }) + end + + assert_equal res1['data'], res2['data'] + end + test 'return an error object and data object' do param = { 'name' => 'key1', 'reset_interval' => 10 } param2 = { 'name' => 'key2', 'reset_interval' => 10 } @@ -164,10 +175,11 @@ def extract_value_from_server(server, scope, name) assert_equal "#{param['name']} already exists in counter", error['message'] end - test 'return a future object when asyn option is true' do + test 'return a future object when async option is true' do param = { 'name' => 'key', 'reset_interval' => 10 } r = @client.init(param, options: { async: true }) assert_true r.is_a?(Fluent::Counter::Future) + assert_nil r.errors end end @@ -187,9 +199,9 @@ def extract_value_from_server(server, scope, name) v = response['data'].first assert_nil response['errors'] - assert_equal @name, v['name'] - assert_equal 20, v['reset_interval'] - assert_equal 'numeric', v['type'] + assert_equal @init_obj['name'], v['name'] + assert_equal @init_obj['type'], v['type'] + assert_equal @init_obj['reset_interval'], v['reset_interval'] assert_nil extract_value_from_server(@server, @scope, @name) end @@ -205,18 +217,18 @@ def extract_value_from_server(server, scope, name) ] ) test 'return an error object' do |(param, expected_error)| - resopnse = @client.delete(param) + response = @client.delete(param) - errors = resopnse['errors'].first + errors = response['errors'].first - assert_empty resopnse['data'] + assert_empty response['data'] assert_equal expected_error, errors end test 'return an error object and data object' do - unknow_name = 'key2' + unknown_name = 'key2' - response = @client.delete(@name, unknow_name) + response = @client.delete(@name, unknown_name) data = response['data'].first error = response['errors'].first @@ -224,14 +236,15 @@ def extract_value_from_server(server, scope, name) assert_equal @init_obj['reset_interval'], data['reset_interval'] assert_equal 'unknown_key', error['code'] - assert_equal "`key2` doesn't exist in counter", error['message'] + assert_equal "`#{unknown_name}` doesn't exist in counter", error['message'] assert_nil extract_value_from_server(@server, @scope, @name) end - test 'return a future object when asyn option is true' do + test 'return a future object when async option is true' do r = @client.delete(@name, options: { async: true }) assert_true r.is_a?(Fluent::Counter::Future) + assert_nil r.errors end end @@ -249,12 +262,14 @@ def extract_value_from_server(server, scope, name) assert_equal 0, v.total assert_equal 0, v.current - @client.inc({ 'name' => @name, 'value' => 10 }) + Timecop.travel(1) + inc_obj = { 'name' => @name, 'value' => 10 } + @client.inc(inc_obj) v = extract_value_from_server(@server, @scope, @name) - assert_equal 10, v.total - assert_equal 10, v.current - assert_equal @now, v.last_modified_at + assert_equal inc_obj['value'], v.total + assert_equal inc_obj['value'], v.current + assert_equal (@now + 1), v.last_modified_at end test 'create and increment a value when force option is true' do @@ -266,10 +281,11 @@ def extract_value_from_server(server, scope, name) @client.inc(param, options: { force: true }) v = extract_value_from_server(@server, @scope, name) - assert_equal name, v.name + assert v + assert_equal param['name'], v.name assert_equal 1, v.reset_interval - assert_equal 11, v.total - assert_equal 11, v.total + assert_equal param['value'], v.current + assert_equal param['value'], v.total end data( @@ -285,16 +301,16 @@ def extract_value_from_server(server, scope, name) { 'name' => 'key' }, { 'code' => 'invalid_params', 'message' => '`value` is required' }, ], - invalid_key: [ + invalid_name: [ { 'name' => '\tkey' }, { 'code' => 'invalid_params', 'message' => '`name` is the invalid format' } ] ) test 'return an error object' do |(param, expected_error)| - resopnse = @client.inc(param) + response = @client.inc(param) - errors = resopnse['errors'].first - assert_empty resopnse['data'] + errors = response['errors'].first + assert_empty response['data'] assert_equal expected_error, errors end @@ -316,10 +332,11 @@ def extract_value_from_server(server, scope, name) assert_equal "`unknown_key` doesn't exist in counter", error['message'] end - test 'return a future object when asyn option is true' do + test 'return a future object when async option is true' do param = { 'name' => 'key', 'value' => 10 } r = @client.inc(param, options: { async: true }) assert_true r.is_a?(Fluent::Counter::Future) + assert_nil r.errors end end @@ -353,18 +370,18 @@ def extract_value_from_server(server, scope, name) ] ) test 'return an error object' do |(param, expected_error)| - resopnse = @client.get(param) + response = @client.get(param) - errors = resopnse['errors'].first + errors = response['errors'].first - assert_empty resopnse['data'] + assert_empty response['data'] assert_equal expected_error, errors end test 'return an error object and data object' do - unknow_name = 'key2' + unknown_name = 'key2' - response = @client.get(@name, unknow_name) + response = @client.get(@name, unknown_name) data = response['data'].first error = response['errors'].first @@ -372,76 +389,79 @@ def extract_value_from_server(server, scope, name) assert_equal @init_obj['reset_interval'], data['reset_interval'] assert_equal 'unknown_key', error['code'] - assert_equal "`key2` doesn't exist in counter", error['message'] + assert_equal "`#{unknown_name}` doesn't exist in counter", error['message'] end - test 'return a future object when asyn option is true' do + test 'return a future object when async option is true' do r = @client.get(@name, options: { async: true }) assert_true r.is_a?(Fluent::Counter::Future) + assert_nil r.errors end end - sub_test_case 'rseet' do + sub_test_case 'reset' do setup do @client.instance_variable_set(:@scope, @scope) @name = 'key' @init_obj = { 'name' => @name, 'reset_interval' => 5, 'type' => 'numeric' } @client.init(@init_obj) - @client.inc({ 'name' => @name, 'value' => 10 }) + @inc_obj = { 'name' => @name, 'value' => 10 } + @client.inc(@inc_obj) end test 'reset a value after `reset_interval` passed' do v1 = extract_value_from_server(@server, @scope, @name) - assert_equal 10, v1.total - assert_equal 10, v1.current + assert_equal @inc_obj['value'], v1.total + assert_equal @inc_obj['value'], v1.current assert_equal @now, v1.last_reset_at - Timecop.travel(6) # greater than reset_interval + travel_sec = 6 # greater than reset_interval + Timecop.travel(travel_sec) v2 = @client.reset(@name) data = v2['data'].first c = data['counter_data'] - assert_equal 6, data['elapsed_time'] + assert_equal travel_sec, data['elapsed_time'] assert_true data['success'] - assert_equal 10, c['current'] - assert_equal 10, c['total'] + assert_equal @inc_obj['value'], c['current'] + assert_equal @inc_obj['value'], c['total'] assert_equal @now, c['last_reset_at'] v1 = extract_value_from_server(@server, @scope, @name) assert_equal 0, v1.current - assert_equal 10, v1.total - assert_equal (@now + 6), v1.last_reset_at - assert_equal (@now + 6), v1.last_modified_at + assert_equal @inc_obj['value'], v1.total + assert_equal (@now + travel_sec), v1.last_reset_at + assert_equal (@now + travel_sec), v1.last_modified_at end test 'return a value object before `reset_interval` passed' do v1 = extract_value_from_server(@server, @scope, @name) - assert_equal 10, v1.total - assert_equal 10, v1.current + assert_equal @inc_obj['value'], v1.total + assert_equal @inc_obj['value'], v1.current assert_equal @now, v1.last_reset_at - Timecop.travel(4) # less than reset_interval + travel_sec = 4 # less than reset_interval + Timecop.travel(travel_sec) v2 = @client.reset(@name) data = v2['data'].first c = data['counter_data'] - assert_equal 4, data['elapsed_time'] + assert_equal travel_sec, data['elapsed_time'] assert_equal false, data['success'] - assert_equal 10, c['current'] - assert_equal 10, c['total'] + assert_equal @inc_obj['value'], c['current'] + assert_equal @inc_obj['value'], c['total'] assert_equal @now, c['last_reset_at'] v1 = extract_value_from_server(@server, @scope, @name) - assert_equal 10, v1.current - assert_equal 10, v1.total - assert_equal @now, v1.last_reset_at + assert_equal @inc_obj['value'], v1.current + assert_equal @inc_obj['value'], v1.total assert_equal @now, v1.last_reset_at end @@ -456,44 +476,46 @@ def extract_value_from_server(server, scope, name) ] ) test 'return an error object' do |(param, expected_error)| - resopnse = @client.reset(param) + response = @client.reset(param) - errors = resopnse['errors'].first + errors = response['errors'].first - assert_empty resopnse['data'] + assert_empty response['data'] assert_equal expected_error, errors end test 'return an error object and data object' do - unknow_name = 'key2' + unknown_name = 'key2' - Timecop.travel(6) # greater than reset_interval + travel_sec = 6 # greater than reset_interval + Timecop.travel(travel_sec) - response = @client.reset(@name, unknow_name) + response = @client.reset(@name, unknown_name) data = response['data'].first error = response['errors'].first counter = data['counter_data'] assert_true data['success'] - assert_equal 6, data['elapsed_time'] + assert_equal travel_sec, data['elapsed_time'] assert_equal @name, counter['name'] assert_equal @init_obj['reset_interval'], counter['reset_interval'] - assert_equal 10, counter['total'] - assert_equal 10, counter['current'] + assert_equal @inc_obj['value'], counter['total'] + assert_equal @inc_obj['value'], counter['current'] assert_equal 'unknown_key', error['code'] - assert_equal "`key2` doesn't exist in counter", error['message'] + assert_equal "`#{unknown_name}` doesn't exist in counter", error['message'] v1 = extract_value_from_server(@server, @scope, @name) assert_equal 0, v1.current - assert_equal 10, v1.total - assert_equal (@now + 6), v1.last_reset_at - assert_equal (@now + 6), v1.last_modified_at + assert_equal @inc_obj['value'], v1.total + assert_equal (@now + travel_sec), v1.last_reset_at + assert_equal (@now + travel_sec), v1.last_modified_at end - test 'return a future object when asyn option is true' do + test 'return a future object when async option is true' do r = @client.reset(@name, options: { async: true }) assert_true r.is_a?(Fluent::Counter::Future) + assert_nil r.errors end end end diff --git a/test/counter/test_error.rb b/test/counter/test_error.rb index c0d0eae874..47c41cab8d 100644 --- a/test/counter/test_error.rb +++ b/test/counter/test_error.rb @@ -18,7 +18,7 @@ class CounterErrorTest < ::Test::Unit::TestCase assert_equal expected, error.to_hash end - test 'parase_error' do + test 'parse_error' do error = Fluent::Counter::ParseError.new(@message) expected = { 'code' => 'parse_error', 'message' => @message } assert_equal expected, error.to_hash diff --git a/test/counter/test_mutex_hash.rb b/test/counter/test_mutex_hash.rb index d03c14c60b..04b7897b82 100644 --- a/test/counter/test_mutex_hash.rb +++ b/test/counter/test_mutex_hash.rb @@ -34,7 +34,7 @@ class MutexHashTest < ::Test::Unit::TestCase assert_true @store.empty? end - test 'use a one mutex value when the same key specifed' do + test 'use a one mutex value when the same key specified' do key = 'key' @counter_store_mutex.synchronize(key) do |store, k| store[k] = @value @@ -85,7 +85,7 @@ class MutexHashTest < ::Test::Unit::TestCase assert_true @store.empty? end - test 'use a one mutex value when the same key specifed' do + test 'use a one mutex value when the same key specified' do key = 'key' @counter_store_mutex.synchronize_keys(key) do |store, k| store[k] = @value diff --git a/test/counter/test_server.rb b/test/counter/test_server.rb index 68b4e66574..eebb167eff 100644 --- a/test/counter/test_server.rb +++ b/test/counter/test_server.rb @@ -56,13 +56,13 @@ def extract_value_from_counter(counter, scope, name) { 'method' => 'init' }, { 'code' => 'invalid_request', 'message' => 'Request should include `id`' } ], - missing_mehtod: [ + missing_method: [ { 'id' => 0 }, { 'code' => 'invalid_request', 'message' => 'Request should include `method`' } ], - invalid_mehtod: [ - { 'id' => 0, 'method' => 'invalid_method'}, - { 'code' => 'method_not_found', 'message' => 'Unknown Method name passed: invalid_method'} + invalid_method: [ + { 'id' => 0, 'method' => 'invalid_method' }, + { 'code' => 'method_not_found', 'message' => 'Unknown method name passed: invalid_method' } ] ) test 'invalid request' do |(request, error)| @@ -93,7 +93,7 @@ def extract_value_from_counter(counter, scope, name) test 'output an error log when passed data is not Hash' do data = 'this is not a hash' - mock($log).error("Recieved data is not Hash: #{data}") + mock($log).error("Received data is not Hash: #{data}") @counter.on_message(data) end end @@ -401,8 +401,8 @@ def extract_value_from_counter(counter, scope, name) v = extract_value_from_counter(@counter, @scope, @name) assert_equal 0, v.current assert_equal 10, v.total + assert_equal (@now + @travel_sec), v.last_reset_at assert_equal (@now + @travel_sec), v.last_modified_at - assert_equal (@now + @travel_sec) , v.last_modified_at end test 'reset a value after `reset_interval` passed' do diff --git a/test/counter/test_store.rb b/test/counter/test_store.rb index 111b24745f..aab5dc5d06 100644 --- a/test/counter/test_store.rb +++ b/test/counter/test_store.rb @@ -32,7 +32,7 @@ class CounterStoreValueTest < ::Test::Unit::TestCase data( integer: ['integer', 0, Integer], numeric: ['numeric', 0, Numeric], - flaot: ['float', 0.0, Float] + float: ['float', 0.0, Float] ) test 'set a type' do |(type_name, value, type)| v = Fluent::Counter::Store::Value.init( @@ -166,7 +166,7 @@ def extract_value_from_counter(counter, scope, name) test "raise a error when when a passed key doesn't exist and raise_error option is true" do assert_raise Fluent::Counter::UnknownKey do - @store.get('unkonwn_key', @scope, raise_error: true) + @store.get('unknown_key', @scope, raise_error: true) end assert_raise Fluent::Counter::UnknownKey do @@ -188,7 +188,7 @@ def extract_value_from_counter(counter, scope, name) test "return false when passed key doesn't exist" do assert_true !@store.key?('unknown_key', @scope) - assert_true !@store.key?(@name, 'unkonwn_scope') + assert_true !@store.key?(@name, 'unknown_scope') end end @@ -251,7 +251,7 @@ def extract_value_from_counter(counter, scope, name) end end - test 'raise an error when a tyep of passed value is incompatible with a stored value' do + test 'raise an error when a type of passed value is incompatible with a stored value' do name2 = 'key_name2' name3 = 'key_name3' v = @store.init(@name, @scope, @init_data.merge('type' => 'integer')) diff --git a/test/counter/test_validator.rb b/test/counter/test_validator.rb index 0833a4af90..c66a22af93 100644 --- a/test/counter/test_validator.rb +++ b/test/counter/test_validator.rb @@ -3,19 +3,19 @@ class CounterValidatorTest < ::Test::Unit::TestCase data( - invald_name1: '', - invald_name3: '_', - invald_name4: 'A', - invald_name5: 'a*', - invald_name6: "a\t", - invald_name7: "\n", + invalid_name1: '', + invalid_name3: '_', + invalid_name4: 'A', + invalid_name5: 'a*', + invalid_name6: "a\t", + invalid_name7: "\n", ) test 'invalid name' do |invalid_name| assert_nil(Fluent::Counter::Validator::VALID_NAME =~ invalid_name) end sub_test_case 'request' do - test 'return an empty ary' do + test 'return an empty array' do data = { 'id' => 0, 'method' => 'init' } errors = Fluent::Counter::Validator.request(data) assert_empty errors @@ -34,12 +34,12 @@ class CounterValidatorTest < ::Test::Unit::TestCase { 'id' => 0, 'method' => "A\t" }, { 'code' => 'invalid_request', 'message' => '`method` is the invalid format' } ], - missing_unknow_method: [ + unknown_method: [ { 'id' => 0, 'method' => 'unknown_method' }, - { 'code' => 'method_not_found', 'message' => 'Unknown Method name passed: unknown_method' } + { 'code' => 'method_not_found', 'message' => 'Unknown method name passed: unknown_method' } ] ) - test "return errors ary" do |(data, expected_error)| + test 'return an error array' do |(data, expected_error)| errors = Fluent::Counter::Validator.request(data) assert_equal [expected_error], errors end @@ -47,7 +47,7 @@ class CounterValidatorTest < ::Test::Unit::TestCase sub_test_case 'call' do test "return an error hash when passed method doesn't exist" do - v = Fluent::Counter::Validator.new(:unknow) + v = Fluent::Counter::Validator.new(:unknown) success, errors = v.call(['key1']) assert_empty success assert_equal 'internal_server_error', errors.first.to_hash['code'] @@ -83,7 +83,8 @@ class CounterHashValidatorTest < ::Test::Unit::TestCase { 'name' => 'key' }, {}, { 'name' => 10 }, - { 'name' => '_' }] + { 'name' => '_' } + ] error_expected = [ { 'code' => 'invalid_params', 'message' => '`name` is required' }, { 'code' => 'invalid_params', 'message' => 'The type of `name` should be String' }, @@ -92,7 +93,7 @@ class CounterHashValidatorTest < ::Test::Unit::TestCase v = Fluent::Counter::HashValidator.new(:name) success, errors = v.call(hash) - assert_equal [{'name' => 'key'}], success + assert_equal [{ 'name' => 'key' }], success assert_equal error_expected, errors.map(&:to_hash) end From 61c32877b98723a9128b13fbb159d7e2203f7528 Mon Sep 17 00:00:00 2001 From: ganmacs Date: Tue, 27 Sep 2016 16:03:11 +0900 Subject: [PATCH 06/23] Implement CleanupThread which removes unused mutex --- lib/fluent/counter/mutex_hash.rb | 80 +++++++++++++++++++--- lib/fluent/counter/server.rb | 54 +++++++++------ lib/fluent/counter/store.rb | 30 ++++----- test/counter/test_client.rb | 24 ++++--- test/counter/test_server.rb | 26 ++++--- test/counter/test_store.rb | 112 ++++++++++++++----------------- 6 files changed, 199 insertions(+), 127 deletions(-) diff --git a/lib/fluent/counter/mutex_hash.rb b/lib/fluent/counter/mutex_hash.rb index 49138ea78a..b28d76d65f 100644 --- a/lib/fluent/counter/mutex_hash.rb +++ b/lib/fluent/counter/mutex_hash.rb @@ -14,6 +14,8 @@ # limitations under the License. # +require 'timeout' + module Fluent module Counter class MutexHash @@ -21,17 +23,17 @@ def initialize(data_store) @mutex = Mutex.new @data_store = data_store @mutex_hash = {} - # @thread = nil - # @cleaup_thread = CleaupThread.new(@store, @dict, @dict_mutex) + @thread = nil + @cleanup_thread = CleanupThread.new(@data_store, @mutex_hash, @mutex) end - # def start - # @cleaup_thread.start - # end + def start + @cleanup_thread.start + end - # def stop - # @cleaup_thread.stop - # end + def stop + @cleanup_thread.stop + end def synchronize(*keys) return if keys.empty? @@ -93,5 +95,67 @@ def synchronize_keys(*keys) end end end + + class CleanupThread + CLEANUP_INTERVAL = 60 * 15 # 15 min + + def initialize(store, mutex_hash, mutex) + @store = store + @mutex_hash = mutex_hash + @mutex = mutex + @thread = nil + @running = false + end + + def start + @running = true + @thread = Thread.new do + while @running + sleep CLEANUP_INTERVAL + run_once + end + end + end + + def stop + return unless @running + @running = false + begin + # Avoid waiting CLEANUP_INTERVAL + Timeout.timeout(1) do + @thread.join + end + rescue Timeout::Error + @thread.kill + end + end + + private + + def run_once + @mutex.synchronize do + last_cleanup_at = (Time.now - CLEANUP_INTERVAL).to_i + @mutex_hash.each do |(key, mutex)| + v = @store.get(key) + next unless v + next if last_cleanup_at < v.last_modified_at.to_i + next unless mutex.try_lock + + @mutex_hash[key] = nil + mutex.unlock + + # Check that a waiting thread is in a lock queue. + # Can't get a lock here means this key is used in other places. + # So restore a mutex value to a corresponding key. + if mutex.try_lock + @mutex_hash.delete(key) + mutex.unlock + else + @mutex_hash[key] = mutex + end + end + end + end + end end end diff --git a/lib/fluent/counter/server.rb b/lib/fluent/counter/server.rb index 0a07e77cab..d609ed52f4 100644 --- a/lib/fluent/counter/server.rb +++ b/lib/fluent/counter/server.rb @@ -46,6 +46,7 @@ def start @loop.run(0.5) @run = true end + @counter.start self end @@ -54,6 +55,7 @@ def stop sleep 0.1 unless @run @server.close @loop.stop + @counter.stop @thread.join if @thread end end @@ -67,6 +69,14 @@ def initialize(name, log) @mutex_hash = MutexHash.new(@store) end + def start + @mutex_hash.start + end + + def stop + @mutex_hash.stop + end + def on_message(data) errors = Validator.request(data) unless errors.empty? @@ -101,12 +111,14 @@ def init(params, scope, options) validator = Fluent::Counter::HashValidator.new(:empty, :name, :reset_interval) valid_params, errors = validator.call(params) res = Response.new(errors) - keys = valid_params.map { |e| e['name'] } + key_hash = valid_params.reduce({}) do |acc, vp| + acc.merge(Store.gen_key(scope, vp['name']) => vp) + end do_init = lambda do |store, key| begin - param = valid_params.find { |par| par['name'] == key } - v = store.init(key, scope, param, ignore: options['ignore']) + param = key_hash[key] + v = store.init(key, param, ignore: options['ignore']) @log.debug("Create new key: #{param['name']}") if @log res.push_data v rescue => e @@ -115,9 +127,9 @@ def init(params, scope, options) end if options['random'] - @mutex_hash.synchronize_keys(*keys, &do_init) + @mutex_hash.synchronize_keys(*(key_hash.keys), &do_init) else - @mutex_hash.synchronize(*keys, &do_init) + @mutex_hash.synchronize(*(key_hash.keys), &do_init) end res.to_hash @@ -127,10 +139,11 @@ def delete(params, scope, options) validator = Fluent::Counter::ArrayValidator.new(:empty, :key) valid_params, errors = validator.call(params) res = Response.new(errors) + keys = valid_params.map { |vp| Store.gen_key(scope, vp) } do_delete = lambda do |store, key| begin - v = store.delete(key, scope) + v = store.delete(key) @log.debug("delete a key: #{key}") if @log res.push_data v rescue => e @@ -139,9 +152,9 @@ def delete(params, scope, options) end if options['random'] - @mutex_hash.synchronize_keys(*valid_params, &do_delete) + @mutex_hash.synchronize_keys(*keys, &do_delete) else - @mutex_hash.synchronize(*valid_params, &do_delete) + @mutex_hash.synchronize(*keys, &do_delete) end res.to_hash @@ -151,15 +164,16 @@ def inc(params, scope, options) validate_param = [:empty, :name, :value] validate_param << :reset_interval if options['force'] validator = Fluent::Counter::HashValidator.new(*validate_param) - valid_params, errors = validator.call(params) res = Response.new(errors) - keys = valid_params.map { |par| par['name'] } + key_hash = valid_params.reduce({}) do |acc, vp| + acc.merge(Store.gen_key(scope, vp['name']) => vp) + end do_inc = lambda do |store, key| begin - param = valid_params.find { |par| par['name'] == key } - v = store.inc(key, scope, param, force: options['force']) + param = key_hash[key] + v = store.inc(key, param, force: options['force']) @log.debug("Increment #{key} by #{param['value']}") if @log res.push_data v rescue => e @@ -168,9 +182,9 @@ def inc(params, scope, options) end if options['random'] - @mutex_hash.synchronize_keys(*keys, &do_inc) + @mutex_hash.synchronize_keys(*(key_hash.keys), &do_inc) else - @mutex_hash.synchronize(*keys, &do_inc) + @mutex_hash.synchronize(*(key_hash.keys), &do_inc) end res.to_hash @@ -180,10 +194,11 @@ def reset(params, scope, options) validator = Fluent::Counter::ArrayValidator.new(:empty, :key) valid_params, errors = validator.call(params) res = Response.new(errors) + keys = valid_params.map { |vp| Store.gen_key(scope, vp) } do_reset = lambda do |store, key| begin - v = store.reset(key, scope) + v = store.reset(key) @log.debug("Reset #{key}'s' counter value") if @log res.push_data v rescue => e @@ -192,9 +207,9 @@ def reset(params, scope, options) end if options['random'] - @mutex_hash.synchronize_keys(*valid_params, &do_reset) + @mutex_hash.synchronize_keys(*keys, &do_reset) else - @mutex_hash.synchronize(*valid_params, &do_reset) + @mutex_hash.synchronize(*keys, &do_reset) end res.to_hash @@ -205,9 +220,10 @@ def get(params, scope, _options) valid_params, errors = validator.call(params) res = Response.new(errors) - valid_params.each do |key| + keys = valid_params.map { |vp| Store.gen_key(scope, vp) } + keys.each do |key| begin - v = @store.get(key, scope, raise_error: true) + v = @store.get(key, raise_error: true) @log.debug("Get counter value: #{key}") if @log res.push_data v rescue => e diff --git a/lib/fluent/counter/store.rb b/lib/fluent/counter/store.rb index 677770f0bd..2336750e7c 100644 --- a/lib/fluent/counter/store.rb +++ b/lib/fluent/counter/store.rb @@ -58,38 +58,34 @@ def initialize @store = {} end - def init(name, scope, data, ignore: false) - if v = get(name, scope) - raise InvalidParams.new("#{name} already exists in counter") unless ignore + def init(key, data, ignore: false) + if v = get(key) + raise InvalidParams.new("#{key} already exists in counter") unless ignore v else - key = Store.gen_key(scope, name) @store[key] = Value.init(data) end end - def get(name, scope, raise_error: false) - key = Store.gen_key(scope, name) + def get(key, raise_error: false) if raise_error - @store[key] or raise UnknownKey.new("`#{name}` doesn't exist in counter") + @store[key] or raise UnknownKey.new("`#{key}` doesn't exist in counter") else @store[key] end end - def key?(name, scope) - key = Store.gen_key(scope, name) + def key?(key) @store.key?(key) end - def delete(name, scope) - key = Store.gen_key(scope, name) - @store.delete(key) or raise UnknownKey.new("`#{name}` doesn't exist in counter") + def delete(key) + @store.delete(key) or raise UnknownKey.new("`#{key}` doesn't exist in counter") end - def inc(name, scope, data, force: false) - init(name, scope, data) if force - v = get(name, scope, raise_error: true) + def inc(key, data, force: false) + init(key, data) if force + v = get(key, raise_error: true) value = data['value'] valid_type!(v, value) @@ -99,8 +95,8 @@ def inc(name, scope, data, force: false) v end - def reset(name, scope) - v = get(name, scope, raise_error: true) + def reset(key) + v = get(key, raise_error: true) now = EventTime.now success = false old_data = v.to_response_hash diff --git a/test/counter/test_client.rb b/test/counter/test_client.rb index 6390254997..24693d1ef2 100644 --- a/test/counter/test_client.rb +++ b/test/counter/test_client.rb @@ -1,5 +1,6 @@ require_relative '../helper' require 'fluent/counter/client' +require 'fluent/counter/store' require 'fluent/counter/server' require 'flexmock/test_unit' require 'timecop' @@ -123,7 +124,7 @@ def extract_value_from_server(server, scope, name) data( already_exist_key: [ { 'name' => 'key1', 'reset_interval' => 10 }, - { 'code' => 'invalid_params', 'message' => 'key1 already exists in counter' } + { 'code' => 'invalid_params', 'message' => "worker1\tplugin1\tkey1 already exists in counter" } ], missing_name: [ { 'reset_interval' => 10 }, @@ -172,7 +173,7 @@ def extract_value_from_server(server, scope, name) assert_equal param2['reset_interval'], data['reset_interval'] assert_equal 'invalid_params', error['code'] - assert_equal "#{param['name']} already exists in counter", error['message'] + assert_equal "#{@scope}\t#{param['name']} already exists in counter", error['message'] end test 'return a future object when async option is true' do @@ -187,6 +188,7 @@ def extract_value_from_server(server, scope, name) setup do @client.instance_variable_set(:@scope, @scope) @name = 'key' + @key = Fluent::Counter::Store.gen_key(@scope, @name) @init_obj = { 'name' => @name, 'reset_interval' => 20, 'type' => 'numeric' } @client.init(@init_obj) @@ -209,7 +211,7 @@ def extract_value_from_server(server, scope, name) data( key_not_found: [ 'key2', - { 'code' => 'unknown_key', 'message' => "`key2` doesn't exist in counter" } + { 'code' => 'unknown_key', 'message' => "`worker1\tplugin1\tkey2` doesn't exist in counter" } ], invalid_key: [ '\tkey', @@ -236,7 +238,7 @@ def extract_value_from_server(server, scope, name) assert_equal @init_obj['reset_interval'], data['reset_interval'] assert_equal 'unknown_key', error['code'] - assert_equal "`#{unknown_name}` doesn't exist in counter", error['message'] + assert_equal "`#{@scope}\t#{unknown_name}` doesn't exist in counter", error['message'] assert_nil extract_value_from_server(@server, @scope, @name) end @@ -252,6 +254,7 @@ def extract_value_from_server(server, scope, name) setup do @client.instance_variable_set(:@scope, @scope) @name = 'key' + @key = Fluent::Counter::Store.gen_key(@scope, @name) @init_obj = { 'name' => @name, 'reset_interval' => 20, 'type' => 'numeric' } @client.init(@init_obj) @@ -291,7 +294,7 @@ def extract_value_from_server(server, scope, name) data( not_exist_key: [ { 'name' => 'key2', 'value' => 10 }, - { 'code' => 'unknown_key', 'message' => "`key2` doesn't exist in counter" } + { 'code' => 'unknown_key', 'message' => "`worker1\tplugin1\tkey2` doesn't exist in counter" } ], missing_name: [ { 'value' => 10 }, @@ -329,7 +332,7 @@ def extract_value_from_server(server, scope, name) assert_equal 10, data['total'] assert_equal 'unknown_key', error['code'] - assert_equal "`unknown_key` doesn't exist in counter", error['message'] + assert_equal "`#{@scope}\tunknown_key` doesn't exist in counter", error['message'] end test 'return a future object when async option is true' do @@ -362,7 +365,7 @@ def extract_value_from_server(server, scope, name) data( key_not_found: [ 'key2', - { 'code' => 'unknown_key', 'message' => "`key2` doesn't exist in counter" } + { 'code' => 'unknown_key', 'message' => "`worker1\tplugin1\tkey2` doesn't exist in counter" } ], invalid_key: [ '\tkey', @@ -389,7 +392,7 @@ def extract_value_from_server(server, scope, name) assert_equal @init_obj['reset_interval'], data['reset_interval'] assert_equal 'unknown_key', error['code'] - assert_equal "`#{unknown_name}` doesn't exist in counter", error['message'] + assert_equal "`#{@scope}\t#{unknown_name}` doesn't exist in counter", error['message'] end test 'return a future object when async option is true' do @@ -403,6 +406,7 @@ def extract_value_from_server(server, scope, name) setup do @client.instance_variable_set(:@scope, @scope) @name = 'key' + @key = Fluent::Counter::Store.gen_key(@scope, @name) @init_obj = { 'name' => @name, 'reset_interval' => 5, 'type' => 'numeric' } @client.init(@init_obj) @@ -468,7 +472,7 @@ def extract_value_from_server(server, scope, name) data( key_not_found: [ 'key2', - { 'code' => 'unknown_key', 'message' => "`key2` doesn't exist in counter" } + { 'code' => 'unknown_key', 'message' => "`worker1\tplugin1\tkey2` doesn't exist in counter" } ], invalid_key: [ '\tkey', @@ -503,7 +507,7 @@ def extract_value_from_server(server, scope, name) assert_equal @inc_obj['value'], counter['current'] assert_equal 'unknown_key', error['code'] - assert_equal "`#{unknown_name}` doesn't exist in counter", error['message'] + assert_equal "`#{@scope}\t#{unknown_name}` doesn't exist in counter", error['message'] v1 = extract_value_from_server(@server, @scope, @name) assert_equal 0, v1.current diff --git a/test/counter/test_server.rb b/test/counter/test_server.rb index eebb167eff..6503f8d45f 100644 --- a/test/counter/test_server.rb +++ b/test/counter/test_server.rb @@ -1,5 +1,6 @@ require_relative '../helper' require 'fluent/counter/server' +require 'fluent/counter/store' require 'fluent/time' require 'flexmock/test_unit' require 'timecop' @@ -22,7 +23,7 @@ class CounterCounterTest < ::Test::Unit::TestCase def extract_value_from_counter(counter, scope, name) store = counter.instance_variable_get(:@store).instance_variable_get(:@store) - key = "#{scope}\t#{name}" + key = Fluent::Counter::Store.gen_key(scope, name) store[key] end @@ -123,6 +124,7 @@ def extract_value_from_counter(counter, scope, name) sub_test_case 'init' do setup do @name = 'key1' + @key = Fluent::Counter::Store.gen_key(@scope, @name) end test 'create new value in a counter' do @@ -192,7 +194,7 @@ def extract_value_from_counter(counter, scope, name) assert_empty result['data'] error = result['errors'].first - expected = { 'code' => 'invalid_params', 'message' => "#{@name} already exists in counter" } + expected = { 'code' => 'invalid_params', 'message' => "#{@key} already exists in counter" } assert_equal expected, error end @@ -207,11 +209,11 @@ def extract_value_from_counter(counter, scope, name) test 'call `synchronize_keys` when random option is true' do mhash = @counter.instance_variable_get(:@mutex_hash) - mock(mhash).synchronize(@name).once + mock(mhash).synchronize(@key).once @counter.send('init', [{ 'name' => @name, 'reset_interval' => 1 }], @scope, {}) mhash = @counter.instance_variable_get(:@mutex_hash) - mock(mhash).synchronize_keys(@name).once + mock(mhash).synchronize_keys(@key).once @counter.send('init', [{ 'name' => @name, 'reset_interval' => 1 }], @scope, { 'random' => true }) end end @@ -219,6 +221,7 @@ def extract_value_from_counter(counter, scope, name) sub_test_case 'delete' do setup do @name = 'key1' + @key = Fluent::Counter::Store.gen_key(@scope, @name) @counter.send('init', [{ 'name' => @name, 'reset_interval' => 20 }], @scope, {}) end @@ -259,16 +262,16 @@ def extract_value_from_counter(counter, scope, name) assert_empty result['data'] error = result['errors'].first assert_equal unknown_key, error['code'] - assert_equal "`#{unknown_key}` doesn't exist in counter", error['message'] + assert_equal "`#{@scope}\t#{unknown_key}` doesn't exist in counter", error['message'] end test 'call `synchronize_keys` when random option is true' do mhash = @counter.instance_variable_get(:@mutex_hash) - mock(mhash).synchronize(@name).once + mock(mhash).synchronize(@key).once @counter.send('delete', [@name], @scope, {}) mhash = @counter.instance_variable_get(:@mutex_hash) - mock(mhash).synchronize_keys(@name).once + mock(mhash).synchronize_keys(@key).once @counter.send('delete', [@name], @scope, { 'random' => true }) end end @@ -277,6 +280,7 @@ def extract_value_from_counter(counter, scope, name) setup do @name1 = 'key1' @name2 = 'key2' + @key1 = Fluent::Counter::Store.gen_key(@scope, @name1) inc_objects = [ { 'name' => @name1, 'reset_interval' => 20 }, { 'name' => @name2, 'type' => 'integer', 'reset_interval' => 20 } @@ -363,12 +367,12 @@ def extract_value_from_counter(counter, scope, name) test 'call `synchronize_keys` when random option is true' do mhash = @counter.instance_variable_get(:@mutex_hash) - mock(mhash).synchronize(@name1).once + mock(mhash).synchronize(@key1).once params = [{ 'name' => @name1, 'value' => 1 }] @counter.send('inc', params, @scope, {}) mhash = @counter.instance_variable_get(:@mutex_hash) - mock(mhash).synchronize_keys(@name1).once + mock(mhash).synchronize_keys(@key1).once @counter.send('inc', params, @scope, { 'random' => true }) end end @@ -451,7 +455,7 @@ def extract_value_from_counter(counter, scope, name) assert_empty result['data'] error = result['errors'].first assert_equal unknown_key, error['code'] - assert_equal "`#{unknown_key}` doesn't exist in counter", error['message'] + assert_equal "`#{@scope}\t#{unknown_key}` doesn't exist in counter", error['message'] end end @@ -514,7 +518,7 @@ def extract_value_from_counter(counter, scope, name) assert_empty result['data'] error = result['errors'].first assert_equal unknown_key, error['code'] - assert_equal "`#{unknown_key}` doesn't exist in counter", error['message'] + assert_equal "`#{@scope}\t#{unknown_key}` doesn't exist in counter", error['message'] end end end diff --git a/test/counter/test_store.rb b/test/counter/test_store.rb index aab5dc5d06..35c737a8e0 100644 --- a/test/counter/test_store.rb +++ b/test/counter/test_store.rb @@ -107,9 +107,8 @@ class CounterStoreTest < ::Test::Unit::TestCase Timecop.return end - def extract_value_from_counter(counter, scope, name) + def extract_value_from_counter(counter, key) store = counter.instance_variable_get(:@store) - key = Fluent::Counter::Store.gen_key(scope, name) store[key] end @@ -118,30 +117,31 @@ def extract_value_from_counter(counter, scope, name) @reset_interval = 10 @store = Fluent::Counter::Store.new @data = { 'name' => @name, 'reset_interval' => @reset_interval } + @key = Fluent::Counter::Store.gen_key(@scope, @name) end test 'create new value in the counter' do - v = @store.init(@name, @scope, @data) + v = @store.init(@key, @data) assert_equal @name, v.name assert_equal @reset_interval, v.reset_interval - v2 = extract_value_from_counter(@store, @scope, @name) + v2 = extract_value_from_counter(@store, @key) assert_equal v, v2 end test 'raise an error when a passed key already exists' do - @store.init(@name, @scope, @data) + @store.init(@key, @data) assert_raise Fluent::Counter::InvalidParams do - @store.init(@name, @scope, @data) + @store.init(@key, @data) end end test 'return a value when passed key already exists and a ignore option is true' do - v = @store.init(@name, @scope, @data) - v1 = extract_value_from_counter(@store, @scope, @name) - v2 = @store.init(@name, @scope, @data, ignore: true) + v = @store.init(@key, @data) + v1 = extract_value_from_counter(@store, @key) + v2 = @store.init(@key, @data, ignore: true) assert_equal v, v2 assert_equal v1, v2 end @@ -151,26 +151,22 @@ def extract_value_from_counter(counter, scope, name) setup do @store = Fluent::Counter::Store.new data = { 'name' => @name, 'reset_interval' => 10 } - @store.init(@name, @scope, data) + @key = Fluent::Counter::Store.gen_key(@scope, @name) + @store.init(@key, data) end test 'return a value from the counter' do - v = extract_value_from_counter(@store, @scope, @name) - assert_equal v, @store.get(@name, @scope) + v = extract_value_from_counter(@store, @key) + assert_equal v, @store.get(@key) end test "return nil when a passed key doesn't exist" do - assert_equal nil, @store.get('unknown_key', @scope) - assert_equal nil, @store.get(@name, 'unknown_scope') + assert_equal nil, @store.get('unknown_key') end test "raise a error when when a passed key doesn't exist and raise_error option is true" do assert_raise Fluent::Counter::UnknownKey do - @store.get('unknown_key', @scope, raise_error: true) - end - - assert_raise Fluent::Counter::UnknownKey do - @store.get(@name, 'unknown_key', raise_error: true) + @store.get('unknown_key', raise_error: true) end end end @@ -179,16 +175,16 @@ def extract_value_from_counter(counter, scope, name) setup do @store = Fluent::Counter::Store.new data = { 'name' => @name, 'reset_interval' => 10 } - @store.init(@name, @scope, data) + @key = Fluent::Counter::Store.gen_key(@scope, @name) + @store.init(@key, data) end test 'return true when passed key exists' do - assert_true @store.key?(@name, @scope) + assert_true @store.key?(@key) end test "return false when passed key doesn't exist" do - assert_true !@store.key?('unknown_key', @scope) - assert_true !@store.key?(@name, 'unknown_scope') + assert_true !@store.key?('unknown_key') end end @@ -196,22 +192,19 @@ def extract_value_from_counter(counter, scope, name) setup do @store = Fluent::Counter::Store.new data = { 'name' => @name, 'reset_interval' => 10 } - @init_value = @store.init(@name, @scope, data) + @key = Fluent::Counter::Store.gen_key(@scope, @name) + @init_value = @store.init(@key, data) end test 'delete a value from the counter' do - v = @store.delete(@name, @scope) + v = @store.delete(@key) assert_equal @init_value, v - assert_nil extract_value_from_counter(@store, @scope, @name) + assert_nil extract_value_from_counter(@store, @key) end test "raise an error when passed key doesn't exist" do assert_raise Fluent::Counter::UnknownKey do - @store.delete('unknown_key', @scope) - end - - assert_raise Fluent::Counter::UnknownKey do - @store.delete(@name, 'unknown_key') + @store.delete('unknown_key') end end end @@ -228,50 +221,48 @@ def extract_value_from_counter(counter, scope, name) negative: -10 ) test 'increment or decrement a value in the counter' do |value| - @store.init(@name, @scope, @init_data) + key = Fluent::Counter::Store.gen_key(@scope, @name) + @store.init(key, @init_data) Timecop.travel(@travel_sec) - v = @store.inc(@name, @scope, { 'value' => value }) + v = @store.inc(key, { 'value' => value }) assert_equal value, v.total assert_equal value, v.current assert_equal (@now + @travel_sec), v.last_modified_at assert_equal @now, v.last_reset_at # last_reset_at doesn't change - v1 = extract_value_from_counter(@store, @scope, @name) + v1 = extract_value_from_counter(@store, key) assert_equal v, v1 end test "raise an error when passed key doesn't exist" do assert_raise Fluent::Counter::UnknownKey do - @store.inc('unknown_key', @scope, { 'value' => 1 }) - end - - assert_raise Fluent::Counter::UnknownKey do - @store.inc(@name, 'unknown_key', { 'value' => 1 }) + @store.inc('unknown_key', { 'value' => 1 }) end end test 'raise an error when a type of passed value is incompatible with a stored value' do - name2 = 'key_name2' - name3 = 'key_name3' - v = @store.init(@name, @scope, @init_data.merge('type' => 'integer')) - v2 = @store.init(name2, @scope, @init_data.merge('type' => 'float')) - v3 = @store.init(name3, @scope, @init_data.merge('type' => 'numeric')) - assert_equal 'integer', v.type + key1 = Fluent::Counter::Store.gen_key(@scope, @name) + key2 = Fluent::Counter::Store.gen_key(@scope, 'name2') + key3 = Fluent::Counter::Store.gen_key(@scope, 'name3') + v1 = @store.init(key1, @init_data.merge('type' => 'integer')) + v2 = @store.init(key2, @init_data.merge('type' => 'float')) + v3 = @store.init(key3, @init_data.merge('type' => 'numeric')) + assert_equal 'integer', v1.type assert_equal 'float', v2.type assert_equal 'numeric', v3.type assert_raise Fluent::Counter::InvalidParams do - @store.inc(@name, @scope, { 'value' => 1.1 }) + @store.inc(key1, { 'value' => 1.1 }) end assert_raise Fluent::Counter::InvalidParams do - @store.inc(name2, @scope, { 'value' => 1 }) + @store.inc(key2, { 'value' => 1 }) end assert_nothing_raised do - @store.inc(name3, @scope, { 'value' => 1 }) - @store.inc(name3, @scope, { 'value' => 1.0 }) + @store.inc(key3, { 'value' => 1 }) + @store.inc(key3, { 'value' => 1.0 }) end end end @@ -282,14 +273,15 @@ def extract_value_from_counter(counter, scope, name) @travel_sec = 10 @inc_value = 10 - @store.init(@name, @scope, { 'name' => @name, 'reset_interval' => 10 }) - @store.inc(@name, @scope, { 'value' => 10 }) + @key = Fluent::Counter::Store.gen_key(@scope, @name) + @store.init(@key, { 'name' => @name, 'reset_interval' => 10 }) + @store.inc(@key, { 'value' => 10 }) end test 'reset a value in the counter' do Timecop.travel(@travel_sec) - v = @store.reset(@name, @scope) + v = @store.reset(@key) assert_equal @travel_sec, v['elapsed_time'] assert_true v['success'] counter = v['counter_data'] @@ -301,7 +293,7 @@ def extract_value_from_counter(counter, scope, name) assert_equal @now, counter['last_reset_at'] assert_equal 10, counter['reset_interval'] - v1 = extract_value_from_counter(@store, @scope, @name) + v1 = extract_value_from_counter(@store, @key) assert_equal 0, v1.current assert_true v1.current.is_a?(Integer) assert_equal @inc_value, v1.total @@ -312,21 +304,21 @@ def extract_value_from_counter(counter, scope, name) test 'reset a value after `reset_interval` passed' do first_travel_sec = 5 Timecop.travel(first_travel_sec) # jump time less than reset_interval - v = @store.reset(@name, @scope) + v = @store.reset(@key) assert_equal false, v['success'] assert_equal first_travel_sec, v['elapsed_time'] - store = extract_value_from_counter(@store, @scope, @name) + store = extract_value_from_counter(@store, @key) assert_equal 10, store.current assert_equal @now, store.last_reset_at # time is passed greater than reset_interval Timecop.travel(@travel_sec) - v = @store.reset(@name, @scope) + v = @store.reset(@key) assert_true v['success'] assert_equal @travel_sec + first_travel_sec, v['elapsed_time'] - v1 = extract_value_from_counter(@store, @scope, @name) + v1 = extract_value_from_counter(@store, @key) assert_equal 0, v1.current assert_equal (@now + @travel_sec + first_travel_sec), v1.last_reset_at assert_equal (@now + @travel_sec + first_travel_sec), v1.last_modified_at @@ -334,11 +326,7 @@ def extract_value_from_counter(counter, scope, name) test "raise an error when passed key doesn't exist" do assert_raise Fluent::Counter::UnknownKey do - @store.reset('unknown_key', @scope) - end - - assert_raise Fluent::Counter::UnknownKey do - @store.reset(@name, 'unknown_key') + @store.reset('unknown_key') end end end From 9d62168fb6c34c28f095ae20b6c6ba5d47687e5b Mon Sep 17 00:00:00 2001 From: ganmacs Date: Tue, 27 Sep 2016 16:04:10 +0900 Subject: [PATCH 07/23] Add CleanupThread test --- test/counter/test_mutex_hash.rb | 67 +++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/test/counter/test_mutex_hash.rb b/test/counter/test_mutex_hash.rb index 04b7897b82..0422915b59 100644 --- a/test/counter/test_mutex_hash.rb +++ b/test/counter/test_mutex_hash.rb @@ -1,5 +1,7 @@ require_relative '../helper' require 'fluent/counter/mutex_hash' +require 'fluent/counter/store' +require 'flexmock/test_unit' require 'timecop' class MutexHashTest < ::Test::Unit::TestCase @@ -111,3 +113,68 @@ class MutexHashTest < ::Test::Unit::TestCase end end end + +class CleanupThreadTest < ::Test::Unit::TestCase + StoreValue = Struct.new(:last_modified_at) + + setup do + # timecop isn't compatible with EventTime + t = Time.parse('2016-09-22 16:59:59 +0900') + Timecop.freeze(t) + + + @store = Fluent::Counter::Store.new + @mhash = Fluent::Counter::MutexHash.new(@store) + + # stub sleep method to avoid waiting CLEANUP_INTERVAL + ct = @mhash.instance_variable_get(:@cleanup_thread) + flexstub(ct).should_receive(:sleep) + end + + teardown do + @mhash.stop + Timecop.return + end + + test 'clean up unused mutex' do + name = 'key1' + init_obj = { 'name' => name, 'reset_interval' => 2 } + + @mhash.synchronize(init_obj['name']) do + @store.init(name, init_obj) + end + + ct = @mhash.instance_variable_get(:@mutex_hash) + assert ct[name] + + Timecop.travel(15 * 60 + 1) # 15 min + + @mhash.start # start cleanup + sleep 1 + + ct = @mhash.instance_variable_get(:@mutex_hash) + assert_empty ct + + @mhash.stop + end + + test "don't remove when `last_modified_at` is greater than (Time.now - CLEANUP_INTERVAL)" do + name = 'key1' + init_obj = { 'name' => name, 'reset_interval' => 2 } + + @mhash.synchronize(init_obj['name']) do + @store.init(name, init_obj) + end + + ct = @mhash.instance_variable_get(:@mutex_hash) + assert ct[name] + + @mhash.start # start cleanup + sleep 1 + + ct = @mhash.instance_variable_get(:@mutex_hash) + assert ct[name] + + @mhash.stop + end +end From 73b2f66f32e8887b1090ce69ec2f8ee1361b906c Mon Sep 17 00:00:00 2001 From: ganmacs Date: Wed, 28 Sep 2016 00:25:21 +0900 Subject: [PATCH 08/23] Change init and inc interface * enable to use keyword argument at method arguments --- lib/fluent/counter/client.rb | 26 +++++-- test/counter/test_client.rb | 132 +++++++++++++++++------------------ 2 files changed, 87 insertions(+), 71 deletions(-) diff --git a/lib/fluent/counter/client.rb b/lib/fluent/counter/client.rb index 7b49bb2328..619c1571dd 100644 --- a/lib/fluent/counter/client.rb +++ b/lib/fluent/counter/client.rb @@ -55,11 +55,19 @@ def establish(scope) @scope = data.first end - # if `async` is true, return a Future object (nonblocking). - # if `async` is false or missing, block at this method and return a Hash object. - def init(*params, options: {}) - # raise 'call `establish` method to set a scope before call this method' unless @scope + # === Example + # `init` receives various arguments. + # + # 1. init(name: 'name') + # 2. init({ name: 'name',reset_interval: 20 }, options: {}) + # 3. init([{ name: 'name1',reset_interval: 20 }, { name: 'name2',reset_interval: 20 }]) + # 4. init([{ name: 'name1',reset_interval: 20 }, { name: 'name2',reset_interval: 20 }], options: {}) + def init(params, options: {}) + params = [params] unless params.is_a?(Array) res = send_request('init', @scope, params, options) + + # if `async` is true, return a Future object (non blocking). + # if `async` is false or missing, block at this method and return a Hash object. options[:async] ? res : res.get end @@ -68,7 +76,15 @@ def delete(*params, options: {}) options[:async] ? res : res.get end - def inc(*params, options: {}) + # === Example + # `inc` receives various arguments. + # + # 1. init(name: 'name') + # 2. init({ name: 'name',value: 20 }, options: {}) + # 3. init([{ name: 'name1',value: 20 }, { name: 'name2',value: 20 }]) + # 4. init([{ name: 'name1',value: 20 }, { name: 'name2',value: 20 }], options: {}) + def inc(params, options: {}) + params = [params] unless params.is_a?(Array) res = send_request('inc', @scope, params, options) options[:async] ? res : res.get end diff --git a/test/counter/test_client.rb b/test/counter/test_client.rb index 24693d1ef2..c42c532237 100644 --- a/test/counter/test_client.rb +++ b/test/counter/test_client.rb @@ -49,7 +49,7 @@ class CounterClientTest < ::Test::Unit::TestCase test 'call a set method to a corresponding object' do @future.should_receive(:set).once.with(Hash) - @client.send(:on_message, {'id' => 1}) + @client.send(:on_message, { 'id' => 1 }) end test "output a warning log when passed id doesn't exist" do @@ -91,56 +91,56 @@ def extract_value_from_server(server, scope, name) data( numeric_type: [ - { 'name' => 'key', 'reset_interval' => 20, 'type' => 'numeric' }, 0 + { name: 'key', reset_interval: 20, type: 'numeric' }, 0 ], float_type: [ - { 'name' => 'key', 'reset_interval' => 20, 'type' => 'float' }, 0.0 + { name: 'key', reset_interval: 20, type: 'float' }, 0.0 ], integer_type: [ - { 'name' => 'key', 'reset_interval' => 20, 'type' => 'integer' }, 0 + { name: 'key', reset_interval: 20, type: 'integer' }, 0 ] ) test 'create a value' do |(param, initial_value)| - assert_nil extract_value_from_server(@server, @scope, 'key') + assert_nil extract_value_from_server(@server, @scope, param[:name]) response = @client.init(param) data = response['data'].first assert_nil response['errors'] - assert_equal 'key', data['name'] - assert_equal 20, data['reset_interval'] - assert_equal param['type'], data['type'] + assert_equal param[:name], data['name'] + assert_equal param[:reset_interval], data['reset_interval'] + assert_equal param[:type], data['type'] assert_equal initial_value, data['current'] assert_equal initial_value, data['total'] - v = extract_value_from_server(@server, @scope, 'key') - assert_equal 'key', v.name - assert_equal 20, v.reset_interval - assert_equal param['type'], v.type + v = extract_value_from_server(@server, @scope, param[:name]) + assert_equal param[:name], v.name + assert_equal param[:reset_interval], v.reset_interval + assert_equal param[:type], v.type assert_equal initial_value, v.total assert_equal initial_value, v.current end data( already_exist_key: [ - { 'name' => 'key1', 'reset_interval' => 10 }, + { name: 'key1', reset_interval: 10 }, { 'code' => 'invalid_params', 'message' => "worker1\tplugin1\tkey1 already exists in counter" } ], missing_name: [ - { 'reset_interval' => 10 }, + { reset_interval: 10 }, { 'code' => 'invalid_params', 'message' => '`name` is required' }, ], missing_reset_interval: [ - { 'name' => 'key' }, + { name: 'key' }, { 'code' => 'invalid_params', 'message' => '`reset_interval` is required' }, ], invalid_name: [ - { 'name' => '\tkey' }, + { name: '\tkey' }, { 'code' => 'invalid_params', 'message' => '`name` is the invalid format' } ] ) test 'return an error object' do |(param, expected_error)| - @client.init({ 'name' => 'key1', 'reset_interval' => 10 }) + @client.init(:name => 'key1', :reset_interval => 10) response = @client.init(param) errors = response['errors'].first @@ -150,34 +150,34 @@ def extract_value_from_server(server, scope, name) end test 'return an existing value when passed key already exists and ignore option is true' do - res1 = @client.init({ 'name' => 'key1', 'reset_interval' => 10 }) + res1 = @client.init(name: 'key1', reset_interval: 10) res2 = nil assert_nothing_raised do - res2 = @client.init({ 'name' => 'key1', 'reset_interval' => 10 }, options: { ignore: true }) + res2 = @client.init({ name: 'key1', reset_interval: 10 }, options: { ignore: true }) end assert_equal res1['data'], res2['data'] end test 'return an error object and data object' do - param = { 'name' => 'key1', 'reset_interval' => 10 } - param2 = { 'name' => 'key2', 'reset_interval' => 10 } + param = { name: 'key1', reset_interval: 10 } + param2 = { name: 'key2', reset_interval: 10 } @client.init(param) - response = @client.init(param2, param) + response = @client.init([param2, param]) data = response['data'].first error = response['errors'].first - assert_equal param2['name'], data['name'] - assert_equal param2['reset_interval'], data['reset_interval'] + assert_equal param2[:name], data['name'] + assert_equal param2[:reset_interval], data['reset_interval'] assert_equal 'invalid_params', error['code'] - assert_equal "#{@scope}\t#{param['name']} already exists in counter", error['message'] + assert_equal "#{@scope}\t#{param[:name]} already exists in counter", error['message'] end test 'return a future object when async option is true' do - param = { 'name' => 'key', 'reset_interval' => 10 } + param = { name: 'key', reset_interval: 10 } r = @client.init(param, options: { async: true }) assert_true r.is_a?(Fluent::Counter::Future) assert_nil r.errors @@ -190,7 +190,7 @@ def extract_value_from_server(server, scope, name) @name = 'key' @key = Fluent::Counter::Store.gen_key(@scope, @name) - @init_obj = { 'name' => @name, 'reset_interval' => 20, 'type' => 'numeric' } + @init_obj = { name: @name, reset_interval: 20, type: 'numeric' } @client.init(@init_obj) end @@ -201,9 +201,9 @@ def extract_value_from_server(server, scope, name) v = response['data'].first assert_nil response['errors'] - assert_equal @init_obj['name'], v['name'] - assert_equal @init_obj['type'], v['type'] - assert_equal @init_obj['reset_interval'], v['reset_interval'] + assert_equal @init_obj[:name], v['name'] + assert_equal @init_obj[:type], v['type'] + assert_equal @init_obj[:reset_interval], v['reset_interval'] assert_nil extract_value_from_server(@server, @scope, @name) end @@ -235,7 +235,7 @@ def extract_value_from_server(server, scope, name) error = response['errors'].first assert_equal @name, data['name'] - assert_equal @init_obj['reset_interval'], data['reset_interval'] + assert_equal @init_obj[:reset_interval], data['reset_interval'] assert_equal 'unknown_key', error['code'] assert_equal "`#{@scope}\t#{unknown_name}` doesn't exist in counter", error['message'] @@ -256,7 +256,7 @@ def extract_value_from_server(server, scope, name) @name = 'key' @key = Fluent::Counter::Store.gen_key(@scope, @name) - @init_obj = { 'name' => @name, 'reset_interval' => 20, 'type' => 'numeric' } + @init_obj = { name: @name, reset_interval: 20, type: 'numeric' } @client.init(@init_obj) end @@ -266,18 +266,18 @@ def extract_value_from_server(server, scope, name) assert_equal 0, v.current Timecop.travel(1) - inc_obj = { 'name' => @name, 'value' => 10 } + inc_obj = { name: @name, value: 10 } @client.inc(inc_obj) v = extract_value_from_server(@server, @scope, @name) - assert_equal inc_obj['value'], v.total - assert_equal inc_obj['value'], v.current + assert_equal inc_obj[:value], v.total + assert_equal inc_obj[:value], v.current assert_equal (@now + 1), v.last_modified_at end test 'create and increment a value when force option is true' do name = 'new_key' - param = { 'name' => name, 'value' => 11, 'reset_interval' => 1 } + param = { name: name, value: 11, reset_interval: 1 } assert_nil extract_value_from_server(@server, @scope, name) @@ -285,27 +285,27 @@ def extract_value_from_server(server, scope, name) v = extract_value_from_server(@server, @scope, name) assert v - assert_equal param['name'], v.name + assert_equal param[:name], v.name assert_equal 1, v.reset_interval - assert_equal param['value'], v.current - assert_equal param['value'], v.total + assert_equal param[:value], v.current + assert_equal param[:value], v.total end data( not_exist_key: [ - { 'name' => 'key2', 'value' => 10 }, + { name: 'key2', value: 10 }, { 'code' => 'unknown_key', 'message' => "`worker1\tplugin1\tkey2` doesn't exist in counter" } ], missing_name: [ - { 'value' => 10 }, + { value: 10 }, { 'code' => 'invalid_params', 'message' => '`name` is required' }, ], missing_value: [ - { 'name' => 'key' }, + { name: 'key' }, { 'code' => 'invalid_params', 'message' => '`value` is required' }, ], invalid_name: [ - { 'name' => '\tkey' }, + { name: '\tkey' }, { 'code' => 'invalid_params', 'message' => '`name` is the invalid format' } ] ) @@ -319,10 +319,10 @@ def extract_value_from_server(server, scope, name) test 'return an error object and data object' do parmas = [ - { 'name' => @name, 'value' => 10 }, - { 'name' => 'unknown_key', 'value' => 9 } + { name: @name, value: 10 }, + { name: 'unknown_key', value: 9 }, ] - response = @client.inc(*parmas) + response = @client.inc(parmas) data = response['data'].first error = response['errors'].first @@ -336,7 +336,7 @@ def extract_value_from_server(server, scope, name) end test 'return a future object when async option is true' do - param = { 'name' => 'key', 'value' => 10 } + param = { name: 'key', value: 10 } r = @client.inc(param, options: { async: true }) assert_true r.is_a?(Fluent::Counter::Future) assert_nil r.errors @@ -348,7 +348,7 @@ def extract_value_from_server(server, scope, name) @client.instance_variable_set(:@scope, @scope) @name = 'key' - @init_obj = { 'name' => @name, 'reset_interval' => 20, 'type' => 'numeric' } + @init_obj = { name: @name, reset_interval: 20, type: 'numeric' } @client.init(@init_obj) end @@ -389,7 +389,7 @@ def extract_value_from_server(server, scope, name) error = response['errors'].first assert_equal @name, data['name'] - assert_equal @init_obj['reset_interval'], data['reset_interval'] + assert_equal @init_obj[:reset_interval], data['reset_interval'] assert_equal 'unknown_key', error['code'] assert_equal "`#{@scope}\t#{unknown_name}` doesn't exist in counter", error['message'] @@ -408,16 +408,16 @@ def extract_value_from_server(server, scope, name) @name = 'key' @key = Fluent::Counter::Store.gen_key(@scope, @name) - @init_obj = { 'name' => @name, 'reset_interval' => 5, 'type' => 'numeric' } + @init_obj = { name: @name, reset_interval: 5, type: 'numeric' } @client.init(@init_obj) - @inc_obj = { 'name' => @name, 'value' => 10 } + @inc_obj = { name: @name, value: 10 } @client.inc(@inc_obj) end test 'reset a value after `reset_interval` passed' do v1 = extract_value_from_server(@server, @scope, @name) - assert_equal @inc_obj['value'], v1.total - assert_equal @inc_obj['value'], v1.current + assert_equal @inc_obj[:value], v1.total + assert_equal @inc_obj[:value], v1.current assert_equal @now, v1.last_reset_at travel_sec = 6 # greater than reset_interval @@ -431,21 +431,21 @@ def extract_value_from_server(server, scope, name) assert_equal travel_sec, data['elapsed_time'] assert_true data['success'] - assert_equal @inc_obj['value'], c['current'] - assert_equal @inc_obj['value'], c['total'] + assert_equal @inc_obj[:value], c['current'] + assert_equal @inc_obj[:value], c['total'] assert_equal @now, c['last_reset_at'] v1 = extract_value_from_server(@server, @scope, @name) assert_equal 0, v1.current - assert_equal @inc_obj['value'], v1.total + assert_equal @inc_obj[:value], v1.total assert_equal (@now + travel_sec), v1.last_reset_at assert_equal (@now + travel_sec), v1.last_modified_at end test 'return a value object before `reset_interval` passed' do v1 = extract_value_from_server(@server, @scope, @name) - assert_equal @inc_obj['value'], v1.total - assert_equal @inc_obj['value'], v1.current + assert_equal @inc_obj[:value], v1.total + assert_equal @inc_obj[:value], v1.current assert_equal @now, v1.last_reset_at travel_sec = 4 # less than reset_interval @@ -459,13 +459,13 @@ def extract_value_from_server(server, scope, name) assert_equal travel_sec, data['elapsed_time'] assert_equal false, data['success'] - assert_equal @inc_obj['value'], c['current'] - assert_equal @inc_obj['value'], c['total'] + assert_equal @inc_obj[:value], c['current'] + assert_equal @inc_obj[:value], c['total'] assert_equal @now, c['last_reset_at'] v1 = extract_value_from_server(@server, @scope, @name) - assert_equal @inc_obj['value'], v1.current - assert_equal @inc_obj['value'], v1.total + assert_equal @inc_obj[:value], v1.current + assert_equal @inc_obj[:value], v1.total assert_equal @now, v1.last_reset_at end @@ -502,16 +502,16 @@ def extract_value_from_server(server, scope, name) assert_true data['success'] assert_equal travel_sec, data['elapsed_time'] assert_equal @name, counter['name'] - assert_equal @init_obj['reset_interval'], counter['reset_interval'] - assert_equal @inc_obj['value'], counter['total'] - assert_equal @inc_obj['value'], counter['current'] + assert_equal @init_obj[:reset_interval], counter['reset_interval'] + assert_equal @inc_obj[:value], counter['total'] + assert_equal @inc_obj[:value], counter['current'] assert_equal 'unknown_key', error['code'] assert_equal "`#{@scope}\t#{unknown_name}` doesn't exist in counter", error['message'] v1 = extract_value_from_server(@server, @scope, @name) assert_equal 0, v1.current - assert_equal @inc_obj['value'], v1.total + assert_equal @inc_obj[:value], v1.total assert_equal (@now + travel_sec), v1.last_reset_at assert_equal (@now + travel_sec), v1.last_modified_at end From 3b4306c04c2d190d2a46b7fe956ce4fab596f688 Mon Sep 17 00:00:00 2001 From: ganmacs Date: Wed, 28 Sep 2016 00:34:18 +0900 Subject: [PATCH 09/23] Check scope is not nil before calling methods --- lib/fluent/counter/client.rb | 9 +++++++++ test/counter/test_client.rb | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/lib/fluent/counter/client.rb b/lib/fluent/counter/client.rb index 619c1571dd..7fdee50049 100644 --- a/lib/fluent/counter/client.rb +++ b/lib/fluent/counter/client.rb @@ -63,6 +63,7 @@ def establish(scope) # 3. init([{ name: 'name1',reset_interval: 20 }, { name: 'name2',reset_interval: 20 }]) # 4. init([{ name: 'name1',reset_interval: 20 }, { name: 'name2',reset_interval: 20 }], options: {}) def init(params, options: {}) + exist_scope! params = [params] unless params.is_a?(Array) res = send_request('init', @scope, params, options) @@ -72,6 +73,7 @@ def init(params, options: {}) end def delete(*params, options: {}) + exist_scope! res = send_request('delete', @scope, params, options) options[:async] ? res : res.get end @@ -84,23 +86,30 @@ def delete(*params, options: {}) # 3. init([{ name: 'name1',value: 20 }, { name: 'name2',value: 20 }]) # 4. init([{ name: 'name1',value: 20 }, { name: 'name2',value: 20 }], options: {}) def inc(params, options: {}) + exist_scope! params = [params] unless params.is_a?(Array) res = send_request('inc', @scope, params, options) options[:async] ? res : res.get end def get(*params, options: {}) + exist_scope! res = send_request('get', @scope, params, options) options[:async] ? res : res.get end def reset(*params, options: {}) + exist_scope! res = send_request('reset', @scope, params, options) options[:async] ? res : res.get end private + def exist_scope! + raise 'Call `establish` method to get a `scope` before calling this method' unless @scope + end + def on_message(data) if response = @responses.delete(data['id']) response.set(data) diff --git a/test/counter/test_client.rb b/test/counter/test_client.rb index c42c532237..1e9fd70702 100644 --- a/test/counter/test_client.rb +++ b/test/counter/test_client.rb @@ -121,6 +121,13 @@ def extract_value_from_server(server, scope, name) assert_equal initial_value, v.current end + test 'raise an error when @scope is nil' do + @client.instance_variable_set(:@scope, nil) + assert_raise 'Call `establish` method to get a `scope` before calling this method' do + @client.init(name: 'key1', reset_interval: 10) + end + end + data( already_exist_key: [ { name: 'key1', reset_interval: 10 }, @@ -208,6 +215,13 @@ def extract_value_from_server(server, scope, name) assert_nil extract_value_from_server(@server, @scope, @name) end + test 'raise an error when @scope is nil' do + @client.instance_variable_set(:@scope, nil) + assert_raise 'Call `establish` method to get a `scope` before calling this method' do + @client.delete(@name) + end + end + data( key_not_found: [ 'key2', @@ -291,6 +305,13 @@ def extract_value_from_server(server, scope, name) assert_equal param[:value], v.total end + test 'raise an error when @scope is nil' do + @client.instance_variable_set(:@scope, nil) + assert_raise 'Call `establish` method to get a `scope` before calling this method' do + @client.inc(name: 'name', value: 1) + end + end + data( not_exist_key: [ { name: 'key2', value: 10 }, @@ -362,6 +383,13 @@ def extract_value_from_server(server, scope, name) assert_equal v1.type, v2['type'] end + test 'raise an error when @scope is nil' do + @client.instance_variable_set(:@scope, nil) + assert_raise 'Call `establish` method to get a `scope` before calling this method' do + @client.get(@name) + end + end + data( key_not_found: [ 'key2', @@ -469,6 +497,13 @@ def extract_value_from_server(server, scope, name) assert_equal @now, v1.last_reset_at end + test 'raise an error when @scope is nil' do + @client.instance_variable_set(:@scope, nil) + assert_raise 'Call `establish` method to get a `scope` before calling this method' do + @client.reset(@name) + end + end + data( key_not_found: [ 'key2', From 0c9a4e865630c7389fb1a8f7920cbabdac7a5c9d Mon Sep 17 00:00:00 2001 From: ganmacs Date: Wed, 28 Sep 2016 11:31:57 +0900 Subject: [PATCH 10/23] Set Timeout --- lib/fluent/counter/client.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/fluent/counter/client.rb b/lib/fluent/counter/client.rb index 7fdee50049..60e1f5ca9f 100644 --- a/lib/fluent/counter/client.rb +++ b/lib/fluent/counter/client.rb @@ -215,7 +215,7 @@ def get def join until @set @mutex.synchronize do - @loop.run_once + @loop.run_once(0.0001) # retun a lock as soon as possible end end end From a429cf0c4e6436e3e3d1ac6e3136e608b8f9295a Mon Sep 17 00:00:00 2001 From: ganmacs Date: Wed, 28 Sep 2016 17:04:53 +0900 Subject: [PATCH 11/23] Use storage plugin --- lib/fluent/counter/mutex_hash.rb | 6 +- lib/fluent/counter/server.rb | 44 +++----- lib/fluent/counter/store.rb | 153 ++++++++++++++++---------- test/counter/test_client.rb | 75 +++++++------ test/counter/test_mutex_hash.rb | 1 - test/counter/test_server.rb | 180 +++++++++++++++---------------- test/counter/test_store.rb | 141 ++++++------------------ 7 files changed, 268 insertions(+), 332 deletions(-) diff --git a/lib/fluent/counter/mutex_hash.rb b/lib/fluent/counter/mutex_hash.rb index b28d76d65f..9c66286458 100644 --- a/lib/fluent/counter/mutex_hash.rb +++ b/lib/fluent/counter/mutex_hash.rb @@ -28,10 +28,12 @@ def initialize(data_store) end def start + @data_store.start @cleanup_thread.start end def stop + @data_store.stop @cleanup_thread.stop end @@ -136,9 +138,9 @@ def run_once @mutex.synchronize do last_cleanup_at = (Time.now - CLEANUP_INTERVAL).to_i @mutex_hash.each do |(key, mutex)| - v = @store.get(key) + v = @store.get(key, raw: true) next unless v - next if last_cleanup_at < v.last_modified_at.to_i + next if last_cleanup_at < v['last_modified_at'][0] # v['last_modified_at'] = [sec, nsec] next unless mutex.try_lock @mutex_hash[key] = nil diff --git a/lib/fluent/counter/server.rb b/lib/fluent/counter/server.rb index d609ed52f4..d68bf1d106 100644 --- a/lib/fluent/counter/server.rb +++ b/lib/fluent/counter/server.rb @@ -27,6 +27,7 @@ class Server DEFAULT_PORT = 4321 def initialize(name, opt = {}) + raise 'Counter server name is invalid' unless Validator::VALID_NAME =~ name @name = name @opt = opt @host = @opt[:host] || DEFAULT_HOST @@ -34,47 +35,32 @@ def initialize(name, opt = {}) @loop = @opt[:loop] || Coolio::Loop.new @log = @opt[:log] || $log - @counter = Fluent::Counter::Counter.new(name, @log) - @server = Coolio::TCPServer.new(@host, @port, Handler, @counter.method(:on_message)) + @store = Fluent::Counter::Store.new(opt) + @mutex_hash = MutexHash.new(@store) + + @server = Coolio::TCPServer.new(@host, @port, Handler, method(:on_message)) @thread = nil - @run = false + @running = false end def start @server.attach(@loop) @thread = Thread.new do + @running = true @loop.run(0.5) - @run = true + @running = false end - @counter.start + @mutex_hash.start self end def stop # This `sleep` for a test to wait for a `@loop` to begin to run - sleep 0.1 unless @run + sleep 0.1 @server.close - @loop.stop - @counter.stop - @thread.join if @thread - end - end - - class Counter - def initialize(name, log) - raise 'Counter server name is invalid' unless Validator::VALID_NAME =~ name - @name = name - @log = log - @store = Fluent::Counter::Store.new - @mutex_hash = MutexHash.new(@store) - end - - def start - @mutex_hash.start - end - - def stop + @loop.stop if @running @mutex_hash.stop + @thread.join if @thread end def on_message(data) @@ -257,16 +243,14 @@ def push_data(data) end def to_hash - data = @data.map { |d| d.respond_to?(:to_response_hash) ? d.to_response_hash : d } - if @errors.empty? - { 'data' => data } + { 'data' => @data } else errors = @errors.map do |e| error = e.respond_to?(:to_hash) ? e : InternalServerError.new(e.to_s) error.to_hash end - { 'data' => data, 'errors' => errors } + { 'data' => @data, 'errors' => errors } end end end diff --git a/lib/fluent/counter/store.rb b/lib/fluent/counter/store.rb index 2336750e7c..9e18ca4183 100644 --- a/lib/fluent/counter/store.rb +++ b/lib/fluent/counter/store.rb @@ -16,111 +16,146 @@ require 'fluent/time' require 'fluent/counter/error' +require 'fluent/plugin/storage_local' module Fluent module Counter class Store - Value = Struct.new(:name, :total, :current, :type, :reset_interval, :last_reset_at, :last_modified_at) do - class << self - def init(data) - type = data['type'] || 'numeric' - now = EventTime.now - v = initial_value(type) - Value.new(data['name'], v, v, type, data['reset_interval'], now, now) - end - - def initial_value(type) - case type - when 'numeric', 'integer' then 0 - when 'float' then 0.0 - else raise InvalidParams.new('`type` should be integer, float, or numeric') - end - end - end + def self.gen_key(scope, key) + "#{scope}\t#{key}" + end - def to_response_hash - { - 'name' => name, - 'total' => total, - 'current' => current, - 'type' => type, - 'reset_interval' => reset_interval, - 'last_reset_at' => last_reset_at, - } - end + def initialize(opt = {}) + # Notice: This storage is not be implemented auto save. + @storage = Fluent::Plugin::LocalStorage.new + @storage.configure( + Fluent::Config::Element.new('storage', {}, {'persistent' => true, 'path' => opt[:path] }, []) + ) end - def self.gen_key(scope, key) - "#{scope}\t#{key}" + def start + @storage.load end - def initialize - @store = {} + def stop + @storage.save end def init(key, data, ignore: false) - if v = get(key) - raise InvalidParams.new("#{key} already exists in counter") unless ignore - v - else - @store[key] = Value.init(data) - end + ret = if v = get(key) + raise InvalidParams.new("#{key} already exists in counter") unless ignore + v + else + @storage.put(key, build_value(data)) + end + + build_response(ret) end - def get(key, raise_error: false) - if raise_error - @store[key] or raise UnknownKey.new("`#{key}` doesn't exist in counter") + def get(key, raise_error: false, raw: false) + ret = if raise_error + @storage.get(key) or raise UnknownKey.new("`#{key}` doesn't exist in counter") + else + @storage.get(key) + end + if raw + ret else - @store[key] + ret && build_response(ret) end end def key?(key) - @store.key?(key) + !!@storage.get(key) end def delete(key) - @store.delete(key) or raise UnknownKey.new("`#{key}` doesn't exist in counter") + ret = @storage.delete(key) or raise UnknownKey.new("`#{key}` doesn't exist in counter") + build_response(ret) end def inc(key, data, force: false) init(key, data) if force - v = get(key, raise_error: true) + v = get(key, raise_error: true, raw: true) value = data['value'] valid_type!(v, value) - v.total += value - v.current += value - v.last_modified_at = EventTime.now - v + v['total'] += value + v['current'] += value + t = EventTime.now + v['last_modified_at'] = [t.sec, t.nsec] + @storage.put(key, v) + + build_response(v) end def reset(key) - v = get(key, raise_error: true) - now = EventTime.now + v = get(key, raise_error: true, raw: true) success = false - old_data = v.to_response_hash + old_data = v.dup + now = EventTime.now + last_reset_at = EventTime.new(*v['last_reset_at']) # Does it need reset? - if (v.last_reset_at + v.reset_interval) <= now + if (last_reset_at + v['reset_interval']) <= now success = true - v.current = Value.initial_value(v.type) - v.last_reset_at = now - v.last_modified_at = now + v['current'] = initial_value(v['type']) + t = [now.sec, now.nsec] + v['last_reset_at'] = t + v['last_modified_at'] = t + @storage.put(key, v) end { - 'elapsed_time' => now - old_data['last_reset_at'], + 'elapsed_time' => now - last_reset_at, 'success' => success, - 'counter_data' => old_data + 'counter_data' => build_response(old_data) } end private + def build_response(d) + { + 'name' => d['name'], + 'total' => d['total'], + 'current' => d['current'], + 'type' => d['type'], + 'reset_interval' => d['reset_interval'], + 'last_reset_at' => EventTime.new(*d['last_reset_at']), + } + end + + # value is Hash. value requires these fileds. + # :name, :total, :current, :type, :reset_interval, :last_reset_at, :last_modified_at + def build_value(data) + type = data['type'] || 'numeric' + now = EventTime.now + t = [now.sec, now.nsec] + + v = initial_value(type) + + data.merge( + 'type' => type, + 'last_reset_at' => t, + 'last_modified_at' => t, + 'current' => v, + 'total' => v, + ) + end + + def initial_value(type) + case type + when 'numeric', 'integer' then 0 + when 'float' then 0.0 + else raise InvalidParams.new('`type` should be integer, float, or numeric') + end + end + def valid_type!(v, value) - return unless (v.type != 'numeric') && (type_str(value) != v.type) - raise InvalidParams.new("`type` is #{v.type}. You should pass #{v.type} value as a `value`") + type = v['type'] + return unless (type != 'numeric') && (type_str(value) != type) + raise InvalidParams.new("`type` is #{type}. You should pass #{type} value as a `value`") end def type_str(v) diff --git a/test/counter/test_client.rb b/test/counter/test_client.rb index 1e9fd70702..fb0e979015 100644 --- a/test/counter/test_client.rb +++ b/test/counter/test_client.rb @@ -60,8 +60,7 @@ class CounterClientTest < ::Test::Unit::TestCase end def extract_value_from_server(server, scope, name) - counter = server.instance_variable_get(:@counter) - store = counter.instance_variable_get(:@store).instance_variable_get(:@store) + store = server.instance_variable_get(:@store).instance_variable_get(:@storage).instance_variable_get(:@store) key = Fluent::Counter::Store.gen_key(scope, name) store[key] end @@ -114,11 +113,11 @@ def extract_value_from_server(server, scope, name) assert_equal initial_value, data['total'] v = extract_value_from_server(@server, @scope, param[:name]) - assert_equal param[:name], v.name - assert_equal param[:reset_interval], v.reset_interval - assert_equal param[:type], v.type - assert_equal initial_value, v.total - assert_equal initial_value, v.current + assert_equal param[:name], v['name'] + assert_equal param[:reset_interval], v['reset_interval'] + assert_equal param[:type], v['type'] + assert_equal initial_value, v['total'] + assert_equal initial_value, v['current'] end test 'raise an error when @scope is nil' do @@ -276,17 +275,17 @@ def extract_value_from_server(server, scope, name) test 'increment a value' do v = extract_value_from_server(@server, @scope, @name) - assert_equal 0, v.total - assert_equal 0, v.current + assert_equal 0, v['total'] + assert_equal 0, v['current'] Timecop.travel(1) inc_obj = { name: @name, value: 10 } @client.inc(inc_obj) v = extract_value_from_server(@server, @scope, @name) - assert_equal inc_obj[:value], v.total - assert_equal inc_obj[:value], v.current - assert_equal (@now + 1), v.last_modified_at + assert_equal inc_obj[:value], v['total'] + assert_equal inc_obj[:value], v['current'] + assert_equal (@now + 1), Fluent::EventTime.new(*v['last_modified_at']) end test 'create and increment a value when force option is true' do @@ -299,10 +298,10 @@ def extract_value_from_server(server, scope, name) v = extract_value_from_server(@server, @scope, name) assert v - assert_equal param[:name], v.name - assert_equal 1, v.reset_interval - assert_equal param[:value], v.current - assert_equal param[:value], v.total + assert_equal param[:name], v['name'] + assert_equal 1, v['reset_interval'] + assert_equal param[:value], v['current'] + assert_equal param[:value], v['total'] end test 'raise an error when @scope is nil' do @@ -377,10 +376,10 @@ def extract_value_from_server(server, scope, name) v1 = extract_value_from_server(@server, @scope, @name) v2 = @client.get(@name)['data'].first - assert_equal v1.name, v2['name'] - assert_equal v1.current, v2['current'] - assert_equal v1.total, v2['total'] - assert_equal v1.type, v2['type'] + assert_equal v1['name'], v2['name'] + assert_equal v1['current'], v2['current'] + assert_equal v1['total'], v2['total'] + assert_equal v1['type'], v2['type'] end test 'raise an error when @scope is nil' do @@ -444,9 +443,9 @@ def extract_value_from_server(server, scope, name) test 'reset a value after `reset_interval` passed' do v1 = extract_value_from_server(@server, @scope, @name) - assert_equal @inc_obj[:value], v1.total - assert_equal @inc_obj[:value], v1.current - assert_equal @now, v1.last_reset_at + assert_equal @inc_obj[:value], v1['total'] + assert_equal @inc_obj[:value], v1['current'] + assert_equal @now, Fluent::EventTime.new(*v1['last_reset_at']) travel_sec = 6 # greater than reset_interval Timecop.travel(travel_sec) @@ -464,17 +463,17 @@ def extract_value_from_server(server, scope, name) assert_equal @now, c['last_reset_at'] v1 = extract_value_from_server(@server, @scope, @name) - assert_equal 0, v1.current - assert_equal @inc_obj[:value], v1.total - assert_equal (@now + travel_sec), v1.last_reset_at - assert_equal (@now + travel_sec), v1.last_modified_at + assert_equal 0, v1['current'] + assert_equal @inc_obj[:value], v1['total'] + assert_equal (@now + travel_sec), Fluent::EventTime.new(*v1['last_reset_at']) + assert_equal (@now + travel_sec), Fluent::EventTime.new(*v1['last_modified_at']) end - test 'return a value object before `reset_interval` passed' do + test 'areturn a value object before `reset_interval` passed' do v1 = extract_value_from_server(@server, @scope, @name) - assert_equal @inc_obj[:value], v1.total - assert_equal @inc_obj[:value], v1.current - assert_equal @now, v1.last_reset_at + assert_equal @inc_obj[:value], v1['total'] + assert_equal @inc_obj[:value], v1['current'] + assert_equal @now, Fluent::EventTime.new(*v1['last_reset_at']) travel_sec = 4 # less than reset_interval Timecop.travel(travel_sec) @@ -492,9 +491,9 @@ def extract_value_from_server(server, scope, name) assert_equal @now, c['last_reset_at'] v1 = extract_value_from_server(@server, @scope, @name) - assert_equal @inc_obj[:value], v1.current - assert_equal @inc_obj[:value], v1.total - assert_equal @now, v1.last_reset_at + assert_equal @inc_obj[:value], v1['current'] + assert_equal @inc_obj[:value], v1['total'] + assert_equal @now, Fluent::EventTime.new(*v1['last_reset_at']) end test 'raise an error when @scope is nil' do @@ -545,10 +544,10 @@ def extract_value_from_server(server, scope, name) assert_equal "`#{@scope}\t#{unknown_name}` doesn't exist in counter", error['message'] v1 = extract_value_from_server(@server, @scope, @name) - assert_equal 0, v1.current - assert_equal @inc_obj[:value], v1.total - assert_equal (@now + travel_sec), v1.last_reset_at - assert_equal (@now + travel_sec), v1.last_modified_at + assert_equal 0, v1['current'] + assert_equal @inc_obj[:value], v1['total'] + assert_equal (@now + travel_sec), Fluent::EventTime.new(*v1['last_reset_at']) + assert_equal (@now + travel_sec), Fluent::EventTime.new(*v1['last_modified_at']) end test 'return a future object when async option is true' do diff --git a/test/counter/test_mutex_hash.rb b/test/counter/test_mutex_hash.rb index 0422915b59..aea0ed70cd 100644 --- a/test/counter/test_mutex_hash.rb +++ b/test/counter/test_mutex_hash.rb @@ -122,7 +122,6 @@ class CleanupThreadTest < ::Test::Unit::TestCase t = Time.parse('2016-09-22 16:59:59 +0900') Timecop.freeze(t) - @store = Fluent::Counter::Store.new @mhash = Fluent::Counter::MutexHash.new(@store) diff --git a/test/counter/test_server.rb b/test/counter/test_server.rb index 6503f8d45f..f3c81eae75 100644 --- a/test/counter/test_server.rb +++ b/test/counter/test_server.rb @@ -5,7 +5,7 @@ require 'flexmock/test_unit' require 'timecop' -class CounterCounterTest < ::Test::Unit::TestCase +class CounterServerTest < ::Test::Unit::TestCase setup do # timecop isn't compatible with EventTime t = Time.parse('2016-09-22 16:59:59 +0900') @@ -14,7 +14,8 @@ class CounterCounterTest < ::Test::Unit::TestCase @scope = "server\tworker\tplugin" @server_name = 'server1' - @counter = Fluent::Counter::Counter.new(@server_name, $log) + @server = Fluent::Counter::Server.new(@server_name, opt: { log: $log }) + @server.instance_eval { @server.close } end teardown do @@ -22,14 +23,14 @@ class CounterCounterTest < ::Test::Unit::TestCase end def extract_value_from_counter(counter, scope, name) - store = counter.instance_variable_get(:@store).instance_variable_get(:@store) + store = counter.instance_variable_get(:@store).instance_variable_get(:@storage).instance_variable_get(:@store) key = Fluent::Counter::Store.gen_key(scope, name) store[key] end test 'raise an error when server name is invalid' do assert_raise do - Fluent::Counter::Counter.new("\tinvalid_name") + Fluent::Counter::Server.new("\tinvalid_name") end end @@ -43,13 +44,13 @@ def extract_value_from_counter(counter, scope, name) reset: 'reset', ) test 'call valid methods' do |method| - stub(@counter).send do |_m, params, scope, options| + stub(@server).send do |_m, params, scope, options| { 'data' => [params, scope, options] } end request = { 'id' => 0, 'method' => method } expected = { 'id' => 0, 'data' => [nil, nil, nil] } - assert_equal expected, @counter.on_message(request) + assert_equal expected, @server.on_message(request) end data( @@ -73,11 +74,11 @@ def extract_value_from_counter(counter, scope, name) 'errors' => [error], } - assert_equal expected, @counter.on_message(request) + assert_equal expected, @server.on_message(request) end test 'return an `internal_server_error` error object when an error raises in safe_run' do - stub(@counter).send do |_m, _params, _scope, _options| + stub(@server).send do |_m, _params, _scope, _options| raise 'Error in safe_run' end @@ -89,19 +90,19 @@ def extract_value_from_counter(counter, scope, name) { 'code' => 'internal_server_error', 'message' => 'Error in safe_run' } ] } - assert_equal expected, @counter.on_message(request) + assert_equal expected, @server.on_message(request) end test 'output an error log when passed data is not Hash' do data = 'this is not a hash' mock($log).error("Received data is not Hash: #{data}") - @counter.on_message(data) + @server.on_message(data) end end sub_test_case 'establish' do test 'establish a scope in a counter' do - result = @counter.send('establish', ['key'], nil, nil) + result = @server.send('establish', ['key'], nil, nil) expected = { 'data' => ["#{@server_name}\tkey"] } assert_equal expected, result end @@ -112,7 +113,7 @@ def extract_value_from_counter(counter, scope, name) invalid_key: [['_key'], '`scope` is the invalid format'], ) test 'raise an error: invalid_params' do |(params, msg)| - result = @counter.send('establish', params, nil, nil) + result = @server.send('establish', params, nil, nil) expected = { 'data' => [], 'errors' => [{ 'code' => 'invalid_params', 'message' => msg }] @@ -128,8 +129,8 @@ def extract_value_from_counter(counter, scope, name) end test 'create new value in a counter' do - assert_nil extract_value_from_counter(@counter, @scope, @name) - result = @counter.send('init', [{ 'name' => @name, 'reset_interval' => 1 }], @scope, {}) + assert_nil extract_value_from_counter(@server, @scope, @name) + result = @server.send('init', [{ 'name' => @name, 'reset_interval' => 1 }], @scope, {}) assert_nil result['errors'] counter = result['data'].first @@ -138,7 +139,7 @@ def extract_value_from_counter(counter, scope, name) assert_equal 0, counter['current'] assert_equal 0, counter['total'] assert_equal @now, counter['last_reset_at'] - assert extract_value_from_counter(@counter, @scope, @name) + assert extract_value_from_counter(@server, @scope, @name) end data( @@ -147,12 +148,12 @@ def extract_value_from_counter(counter, scope, name) float: 'float' ) test 'set the type of a counter value' do |type| - result = @counter.send('init', [{ 'name' => @name, 'reset_interval' => 1, 'type' => type }], @scope, {}) + result = @server.send('init', [{ 'name' => @name, 'reset_interval' => 1, 'type' => type }], @scope, {}) counter = result['data'].first assert_equal type, counter['type'] - v = extract_value_from_counter(@counter, @scope, @name) - assert_equal type, v.type + v = extract_value_from_counter(@server, @scope, @name) + assert_equal type, v['type'] end data( @@ -179,7 +180,7 @@ def extract_value_from_counter(counter, scope, name) ] ) test 'return an error object: invalid_params' do |(params, msg)| - result = @counter.send('init', params, @scope, {}) + result = @server.send('init', params, @scope, {}) assert_empty result['data'] error = result['errors'].first assert_equal 'invalid_params', error['code'] @@ -187,10 +188,10 @@ def extract_value_from_counter(counter, scope, name) end test 'return error objects when passed key already exists' do - @counter.send('init', [{ 'name' => @name, 'reset_interval' => 1 }], @scope, {}) + @server.send('init', [{ 'name' => @name, 'reset_interval' => 1 }], @scope, {}) # call `init` to same key twice - result = @counter.send('init', [{ 'name' => @name, 'reset_interval' => 1 }], @scope, {}) + result = @server.send('init', [{ 'name' => @name, 'reset_interval' => 1 }], @scope, {}) assert_empty result['data'] error = result['errors'].first @@ -199,22 +200,22 @@ def extract_value_from_counter(counter, scope, name) end test 'return existing value when passed key already exists and ignore option is true' do - v1 = @counter.send('init', [{ 'name' => @name, 'reset_interval' => 1 }], @scope, {}) + v1 = @server.send('init', [{ 'name' => @name, 'reset_interval' => 1 }], @scope, {}) # call `init` to same key twice - v2 = @counter.send('init', [{ 'name' => @name, 'reset_interval' => 1 }], @scope, { 'ignore' => true }) + v2 = @server.send('init', [{ 'name' => @name, 'reset_interval' => 1 }], @scope, { 'ignore' => true }) assert_nil v2['errors'] assert_equal v1['data'], v2['data'] end test 'call `synchronize_keys` when random option is true' do - mhash = @counter.instance_variable_get(:@mutex_hash) + mhash = @server.instance_variable_get(:@mutex_hash) mock(mhash).synchronize(@key).once - @counter.send('init', [{ 'name' => @name, 'reset_interval' => 1 }], @scope, {}) + @server.send('init', [{ 'name' => @name, 'reset_interval' => 1 }], @scope, {}) - mhash = @counter.instance_variable_get(:@mutex_hash) + mhash = @server.instance_variable_get(:@mutex_hash) mock(mhash).synchronize_keys(@key).once - @counter.send('init', [{ 'name' => @name, 'reset_interval' => 1 }], @scope, { 'random' => true }) + @server.send('init', [{ 'name' => @name, 'reset_interval' => 1 }], @scope, { 'random' => true }) end end @@ -222,13 +223,13 @@ def extract_value_from_counter(counter, scope, name) setup do @name = 'key1' @key = Fluent::Counter::Store.gen_key(@scope, @name) - @counter.send('init', [{ 'name' => @name, 'reset_interval' => 20 }], @scope, {}) + @server.send('init', [{ 'name' => @name, 'reset_interval' => 20 }], @scope, {}) end test 'delete a value from a counter' do - assert extract_value_from_counter(@counter, @scope, @name) + assert extract_value_from_counter(@server, @scope, @name) - result = @counter.send('delete', [@name], @scope, {}) + result = @server.send('delete', [@name], @scope, {}) assert_nil result['errors'] counter = result['data'].first @@ -238,7 +239,7 @@ def extract_value_from_counter(counter, scope, name) assert_equal @name, counter['name'] assert_equal @now, counter['last_reset_at'] - assert_nil extract_value_from_counter(@counter, @scope, @name) + assert_nil extract_value_from_counter(@server, @scope, @name) end data( @@ -247,7 +248,7 @@ def extract_value_from_counter(counter, scope, name) invalid_key: [['_key'], '`key` is the invalid format'], ) test 'return an error object: invalid_params' do |(params, msg)| - result = @counter.send('delete', params, @scope, {}) + result = @server.send('delete', params, @scope, {}) assert_empty result['data'] error = result['errors'].first @@ -257,7 +258,7 @@ def extract_value_from_counter(counter, scope, name) test 'return an error object: unknown_key' do unknown_key = 'unknown_key' - result = @counter.send('delete', [unknown_key], @scope, {}) + result = @server.send('delete', [unknown_key], @scope, {}) assert_empty result['data'] error = result['errors'].first @@ -266,13 +267,13 @@ def extract_value_from_counter(counter, scope, name) end test 'call `synchronize_keys` when random option is true' do - mhash = @counter.instance_variable_get(:@mutex_hash) + mhash = @server.instance_variable_get(:@mutex_hash) mock(mhash).synchronize(@key).once - @counter.send('delete', [@name], @scope, {}) + @server.send('delete', [@name], @scope, {}) - mhash = @counter.instance_variable_get(:@mutex_hash) + mhash = @server.instance_variable_get(:@mutex_hash) mock(mhash).synchronize_keys(@key).once - @counter.send('delete', [@name], @scope, { 'random' => true }) + @server.send('delete', [@name], @scope, { 'random' => true }) end end @@ -285,11 +286,11 @@ def extract_value_from_counter(counter, scope, name) { 'name' => @name1, 'reset_interval' => 20 }, { 'name' => @name2, 'type' => 'integer', 'reset_interval' => 20 } ] - @counter.send('init', inc_objects, @scope, {}) + @server.send('init', inc_objects, @scope, {}) end test 'increment or decrement a value in counter' do - result = @counter.send('inc', [{ 'name' => @name1, 'value' => 10 }], @scope, {}) + result = @server.send('inc', [{ 'name' => @name1, 'value' => 10 }], @scope, {}) assert_nil result['errors'] counter = result['data'].first @@ -299,25 +300,25 @@ def extract_value_from_counter(counter, scope, name) assert_equal @name1, counter['name'] assert_equal @now, counter['last_reset_at'] - c = extract_value_from_counter(@counter, @scope, @name1) - assert_equal 10, c.current - assert_equal 10, c.total - assert_equal @now, c.last_reset_at - assert_equal @now, c.last_modified_at + c = extract_value_from_counter(@server, @scope, @name1) + assert_equal 10, c['current'] + assert_equal 10, c['total'] + assert_equal @now, Fluent::EventTime.new(*c['last_reset_at']) + assert_equal @now, Fluent::EventTime.new(*c['last_modified_at']) end test 'create new value and increment/decrement its value when `force` option is true' do new_name = 'new_key' - assert_nil extract_value_from_counter(@counter, @scope, new_name) + assert_nil extract_value_from_counter(@server, @scope, new_name) - v1 = @counter.send('inc', [{ 'name' => new_name, 'value' => 10 }], @scope, {}) + v1 = @server.send('inc', [{ 'name' => new_name, 'value' => 10 }], @scope, {}) assert_empty v1['data'] error = v1['errors'].first assert_equal 'unknown_key', error['code'] - assert_nil extract_value_from_counter(@counter, @scope, new_name) + assert_nil extract_value_from_counter(@server, @scope, new_name) - v2 = @counter.send( + v2 = @server.send( 'inc', [{ 'name' => new_name, 'value' => 10, 'reset_interval' => 20 }], @scope, @@ -333,7 +334,7 @@ def extract_value_from_counter(counter, scope, name) assert_equal new_name, counter['name'] assert_equal @now, counter['last_reset_at'] - assert extract_value_from_counter(@counter, @scope, new_name) + assert extract_value_from_counter(@server, @scope, new_name) end data( @@ -357,7 +358,7 @@ def extract_value_from_counter(counter, scope, name) ] ) test 'return an error object: invalid_params' do |(params, msg, opt)| - result = @counter.send('inc', params, @scope, opt) + result = @server.send('inc', params, @scope, opt) assert_empty result['data'] error = result['errors'].first @@ -366,14 +367,14 @@ def extract_value_from_counter(counter, scope, name) end test 'call `synchronize_keys` when random option is true' do - mhash = @counter.instance_variable_get(:@mutex_hash) + mhash = @server.instance_variable_get(:@mutex_hash) mock(mhash).synchronize(@key1).once params = [{ 'name' => @name1, 'value' => 1 }] - @counter.send('inc', params, @scope, {}) + @server.send('inc', params, @scope, {}) - mhash = @counter.instance_variable_get(:@mutex_hash) + mhash = @server.instance_variable_get(:@mutex_hash) mock(mhash).synchronize_keys(@key1).once - @counter.send('inc', params, @scope, { 'random' => true }) + @server.send('inc', params, @scope, { 'random' => true }) end end @@ -381,14 +382,14 @@ def extract_value_from_counter(counter, scope, name) setup do @name = 'key' @travel_sec = 10 - @counter.send('init', [{ 'name' => @name, 'reset_interval' => 10 }], @scope, {}) - @counter.send('inc', [{ 'name' => @name, 'value' => 10 }], @scope, {}) + @server.send('init', [{ 'name' => @name, 'reset_interval' => 10 }], @scope, {}) + @server.send('inc', [{ 'name' => @name, 'value' => 10 }], @scope, {}) end test 'reset a value in the counter' do Timecop.travel(@travel_sec) - result = @counter.send('reset', [@name], @scope, {}) + result = @server.send('reset', [@name], @scope, {}) assert_nil result['errors'] data = result['data'].first @@ -402,38 +403,38 @@ def extract_value_from_counter(counter, scope, name) assert_equal @name, counter['name'] assert_equal @now, counter['last_reset_at'] - v = extract_value_from_counter(@counter, @scope, @name) - assert_equal 0, v.current - assert_equal 10, v.total - assert_equal (@now + @travel_sec), v.last_reset_at - assert_equal (@now + @travel_sec), v.last_modified_at + v = extract_value_from_counter(@server, @scope, @name) + assert_equal 0, v['current'] + assert_equal 10, v['total'] + assert_equal (@now + @travel_sec), Fluent::EventTime.new(*v['last_reset_at']) + assert_equal (@now + @travel_sec), Fluent::EventTime.new(*v['last_modified_at']) end test 'reset a value after `reset_interval` passed' do first_travel_sec = 5 Timecop.travel(first_travel_sec) # jump time less than reset_interval - result = @counter.send('reset', [@name], @scope, {}) + result = @server.send('reset', [@name], @scope, {}) v = result['data'].first assert_equal false, v['success'] assert_equal first_travel_sec, v['elapsed_time'] - store = extract_value_from_counter(@counter, @scope, @name) - assert_equal 10, store.current - assert_equal @now, store.last_reset_at + store = extract_value_from_counter(@server, @scope, @name) + assert_equal 10, store['current'] + assert_equal @now, Fluent::EventTime.new(*store['last_reset_at']) # time is passed greater than reset_interval Timecop.travel(@travel_sec) - result = @counter.send('reset', [@name], @scope, {}) + result = @server.send('reset', [@name], @scope, {}) v = result['data'].first assert_true v['success'] assert_equal @travel_sec + first_travel_sec, v['elapsed_time'] - v1 = extract_value_from_counter(@counter, @scope, @name) - assert_equal 0, v1.current - assert_equal (@now + @travel_sec + first_travel_sec), v1.last_reset_at - assert_equal (@now + @travel_sec + first_travel_sec), v1.last_modified_at + v1 = extract_value_from_counter(@server, @scope, @name) + assert_equal 0, v1['current'] + assert_equal (@now + @travel_sec + first_travel_sec), Fluent::EventTime.new(*v1['last_reset_at']) + assert_equal (@now + @travel_sec + first_travel_sec), Fluent::EventTime.new(*v1['last_modified_at']) end data( @@ -442,7 +443,7 @@ def extract_value_from_counter(counter, scope, name) invalid_key: [['_key'], '`key` is the invalid format'], ) test 'return an error object: invalid_params' do |(params, msg)| - result = @counter.send('reset', params, @scope, {}) + result = @server.send('reset', params, @scope, {}) assert_empty result['data'] assert_equal 'invalid_params', result['errors'].first['code'] assert_equal msg, result['errors'].first['message'] @@ -450,7 +451,7 @@ def extract_value_from_counter(counter, scope, name) test 'return an error object: unknown_key' do unknown_key = 'unknown_key' - result = @counter.send('reset', [unknown_key], @scope, {}) + result = @server.send('reset', [unknown_key], @scope, {}) assert_empty result['data'] error = result['errors'].first @@ -467,12 +468,12 @@ def extract_value_from_counter(counter, scope, name) { 'name' => @name1, 'reset_interval' => 0 }, { 'name' => @name2, 'reset_interval' => 0 }, ] - @counter.send('init', init_objects, @scope, {}) + @server.send('init', init_objects, @scope, {}) end test 'get a counter value' do key = @name1 - result = @counter.send('get', [key], @scope, {}) + result = @server.send('get', [key], @scope, {}) assert_nil result['errors'] counter = result['data'].first @@ -483,7 +484,7 @@ def extract_value_from_counter(counter, scope, name) end test 'get counter values' do - result = @counter.send('get', [@name1, @name2], @scope, {}) + result = @server.send('get', [@name1, @name2], @scope, {}) assert_nil result['errors'] counter1 = result['data'][0] @@ -505,7 +506,7 @@ def extract_value_from_counter(counter, scope, name) invalid_key: [['_key'], '`key` is the invalid format'], ) test 'return an error object: invalid_params' do |(params, msg)| - result = @counter.send('get', params, @scope, {}) + result = @server.send('get', params, @scope, {}) assert_empty result['data'] assert_equal 'invalid_params', result['errors'].first['code'] assert_equal msg, result['errors'].first['message'] @@ -513,7 +514,7 @@ def extract_value_from_counter(counter, scope, name) test 'return an error object: unknown_key' do unknown_key = 'unknown_key' - result = @counter.send('get', [unknown_key], @scope, {}) + result = @server.send('get', [unknown_key], @scope, {}) assert_empty result['data'] error = result['errors'].first @@ -525,13 +526,20 @@ def extract_value_from_counter(counter, scope, name) class CounterCounterResponseTest < ::Test::Unit::TestCase setup do - @response = Fluent::Counter::Counter::Response.new + @response = Fluent::Counter::Server::Response.new @errors = [ StandardError.new('standard error'), Fluent::Counter::InternalServerError.new('internal server error') ] @now = Fluent::EventTime.now - value = Fluent::Counter::Store::Value.new('name', 100, 11, 'numeric', 10, @now, @now) + value = { + 'name' => 'name', + 'total' => 100, + 'current' => 11, + 'type' => 'numeric', + 'reset_interval' => 10, + 'last_reset_at' => @now, + } @values = [value, 'test'] end @@ -566,17 +574,7 @@ class CounterCounterResponseTest < ::Test::Unit::TestCase { 'code' => 'internal_server_error', 'message' => 'standard error' }, { 'code' => 'internal_server_error', 'message' => 'internal server error' } ] - expected_data = [ - { - 'name' => 'name', - 'total' => 100, - 'current' => 11, - 'type' => 'numeric', - 'reset_interval' => 10, - 'last_reset_at' => @now, - }, - 'test' - ] + expected_data = @values hash = @response.to_hash assert_equal expected_errors, hash['errors'] diff --git a/test/counter/test_store.rb b/test/counter/test_store.rb index 35c737a8e0..67ba231926 100644 --- a/test/counter/test_store.rb +++ b/test/counter/test_store.rb @@ -1,97 +1,8 @@ require_relative '../helper' require 'fluent/counter/store' +require 'fluent/time' require 'timecop' -class CounterStoreValueTest < ::Test::Unit::TestCase - setup do - # timecop isn't compatible with EventTime - t = Time.parse('2016-09-22 16:59:59 +0900') - Timecop.freeze(t) - @now = Fluent::EventTime.now - end - - teardown do - Timecop.return - end - - sub_test_case 'init' do - test 'default values' do - v = Fluent::Counter::Store::Value.init( - 'name' => 'name', - 'reset_interval' => 10 - ) - assert_equal 'name', v.name - assert_equal 10, v.reset_interval - assert_equal 'numeric', v.type - assert_equal @now, v.last_reset_at - assert_equal @now, v.last_modified_at - assert_equal 0, v.current - assert_equal 0, v.total - end - - data( - integer: ['integer', 0, Integer], - numeric: ['numeric', 0, Numeric], - float: ['float', 0.0, Float] - ) - test 'set a type' do |(type_name, value, type)| - v = Fluent::Counter::Store::Value.init( - 'type' => type_name, - 'name' => 'name', - 'reset_interval' => 10 - ) - - assert_true v.current.is_a?(type) - assert_equal value, v.current - assert_equal value, v.total - assert_equal type_name, v.type - end - - test 'raise an error when an invalid type has passed' do - assert_raise do - Fluent::Counter::Store::Value.init( - 'type' => 'invalid_type', - 'name' => 'name', - 'reset_interval' => 10 - ) - end - end - end - - sub_test_case 'initial_value' do - data( - integer: ['integer', Integer], - numeric: ['numeric', Integer], - float: ['float', Float], - ) - test 'return a initial value' do |(type, type_class)| - assert_true Fluent::Counter::Store::Value.initial_value(type).is_a?(type_class) - end - - test 'raise an error when passed string is invalid' do - assert_raise Fluent::Counter::InvalidParams do - Fluent::Counter::Store::Value.initial_value('invalid value') - end - end - end - - test 'to_response_hash' do - v = Fluent::Counter::Store::Value.init( - 'name' => 'name', - 'reset_interval' => 10 - ) - expected = { - 'name' => 'name', - 'total' => 0, - 'current' => 0, - 'type' => 'numeric', - 'reset_interval' => 10, - 'last_reset_at' => @now, - } - assert_equal expected, v.to_response_hash - end -end - class CounterStoreTest < ::Test::Unit::TestCase setup do @name = 'key_name' @@ -108,7 +19,7 @@ class CounterStoreTest < ::Test::Unit::TestCase end def extract_value_from_counter(counter, key) - store = counter.instance_variable_get(:@store) + store = counter.instance_variable_get(:@storage).instance_variable_get(:@store) store[key] end @@ -123,10 +34,11 @@ def extract_value_from_counter(counter, key) test 'create new value in the counter' do v = @store.init(@key, @data) - assert_equal @name, v.name - assert_equal @reset_interval, v.reset_interval + assert_equal @name, v['name'] + assert_equal @reset_interval, v['reset_interval'] v2 = extract_value_from_counter(@store, @key) + v2 = @store.send(:build_response, v2) assert_equal v, v2 end @@ -141,6 +53,7 @@ def extract_value_from_counter(counter, key) test 'return a value when passed key already exists and a ignore option is true' do v = @store.init(@key, @data) v1 = extract_value_from_counter(@store, @key) + v1 = @store.send(:build_response, v1) v2 = @store.init(@key, @data, ignore: true) assert_equal v, v2 assert_equal v1, v2 @@ -157,7 +70,13 @@ def extract_value_from_counter(counter, key) test 'return a value from the counter' do v = extract_value_from_counter(@store, @key) - assert_equal v, @store.get(@key) + expected = @store.send(:build_response, v) + assert_equal expected, @store.get(@key) + end + + test 'return a raw value from the counter when raw option is true' do + v = extract_value_from_counter(@store, @key) + assert_equal v, @store.get(@key, raw: true) end test "return nil when a passed key doesn't exist" do @@ -226,12 +145,12 @@ def extract_value_from_counter(counter, key) Timecop.travel(@travel_sec) v = @store.inc(key, { 'value' => value }) - assert_equal value, v.total - assert_equal value, v.current - assert_equal (@now + @travel_sec), v.last_modified_at - assert_equal @now, v.last_reset_at # last_reset_at doesn't change + assert_equal value, v['total'] + assert_equal value, v['current'] + assert_equal @now, v['last_reset_at'] # last_reset_at doesn't change v1 = extract_value_from_counter(@store, key) + v1 = @store.send(:build_response, v1) assert_equal v, v1 end @@ -248,9 +167,9 @@ def extract_value_from_counter(counter, key) v1 = @store.init(key1, @init_data.merge('type' => 'integer')) v2 = @store.init(key2, @init_data.merge('type' => 'float')) v3 = @store.init(key3, @init_data.merge('type' => 'numeric')) - assert_equal 'integer', v1.type - assert_equal 'float', v2.type - assert_equal 'numeric', v3.type + assert_equal 'integer', v1['type'] + assert_equal 'float', v2['type'] + assert_equal 'numeric', v3['type'] assert_raise Fluent::Counter::InvalidParams do @store.inc(key1, { 'value' => 1.1 }) @@ -294,11 +213,11 @@ def extract_value_from_counter(counter, key) assert_equal 10, counter['reset_interval'] v1 = extract_value_from_counter(@store, @key) - assert_equal 0, v1.current - assert_true v1.current.is_a?(Integer) - assert_equal @inc_value, v1.total - assert_equal (@now + @travel_sec), v1.last_reset_at - assert_equal (@now + @travel_sec), v1.last_modified_at + assert_equal 0, v1['current'] + assert_true v1['current'].is_a?(Integer) + assert_equal @inc_value, v1['total'] + assert_equal (@now + @travel_sec), Fluent::EventTime.new(*v1['last_reset_at']) + assert_equal (@now + @travel_sec), Fluent::EventTime.new(*v1['last_modified_at']) end test 'reset a value after `reset_interval` passed' do @@ -309,8 +228,8 @@ def extract_value_from_counter(counter, key) assert_equal false, v['success'] assert_equal first_travel_sec, v['elapsed_time'] store = extract_value_from_counter(@store, @key) - assert_equal 10, store.current - assert_equal @now, store.last_reset_at + assert_equal 10, store['current'] + assert_equal @now, Fluent::EventTime.new(*store['last_reset_at']) # time is passed greater than reset_interval Timecop.travel(@travel_sec) @@ -319,9 +238,9 @@ def extract_value_from_counter(counter, key) assert_equal @travel_sec + first_travel_sec, v['elapsed_time'] v1 = extract_value_from_counter(@store, @key) - assert_equal 0, v1.current - assert_equal (@now + @travel_sec + first_travel_sec), v1.last_reset_at - assert_equal (@now + @travel_sec + first_travel_sec), v1.last_modified_at + assert_equal 0, v1['current'] + assert_equal (@now + @travel_sec + first_travel_sec), Fluent::EventTime.new(*v1['last_reset_at']) + assert_equal (@now + @travel_sec + first_travel_sec), Fluent::EventTime.new(*v1['last_modified_at']) end test "raise an error when passed key doesn't exist" do From cbcb9682428987540fcba68e0a228d08044a6397 Mon Sep 17 00:00:00 2001 From: ganmacs Date: Wed, 28 Sep 2016 19:25:51 +0900 Subject: [PATCH 12/23] Add system_config to set endpoint or etc --- example/counter.conf | 17 ++++++++++++++++ lib/fluent/counter/client.rb | 20 +++++++++++------- lib/fluent/counter/server.rb | 20 +++++++++--------- lib/fluent/counter/store.rb | 39 +++++++++++++++++++++++++++++------- lib/fluent/supervisor.rb | 24 ++++++++++++++++++++++ lib/fluent/system_config.rb | 22 ++++++++++++++++++-- test/counter/test_client.rb | 4 ++-- test/test_supervisor.rb | 9 +++++++++ 8 files changed, 128 insertions(+), 27 deletions(-) create mode 100644 example/counter.conf diff --git a/example/counter.conf b/example/counter.conf new file mode 100644 index 0000000000..ff5958fc04 --- /dev/null +++ b/example/counter.conf @@ -0,0 +1,17 @@ + + + scope server1 + endpoint 127.0.0.1:4321 + path tmp/back + + + + + @type dummy + tag "test.data" + auto_increment_key number + + + + @type stdout + diff --git a/lib/fluent/counter/client.rb b/lib/fluent/counter/client.rb index 60e1f5ca9f..c0546c770b 100644 --- a/lib/fluent/counter/client.rb +++ b/lib/fluent/counter/client.rb @@ -21,15 +21,15 @@ module Fluent module Counter class Client DEFAULT_PORT = 4321 - DEFAULT_HOST = '127.0.0.1' + DEFAULT_ADDR = '127.0.0.1' ID_LIMIT_COUNT = 1 << 31 - def initialize(loop = Coolio::Loop.new, opt = {}) - @loop = loop + def initialize(loop = nil, opt = {}) + @loop = loop || Coolio::Loop.new @port = opt[:port] || DEFAULT_PORT - @host = opt[:host] || DEFAULT_HOST + @addr = opt[:addr] || DEFAULT_ADDR @log = opt[:log] || $log - @conn = Connection.connect(@host, @port, method(:on_message)) + @conn = Connection.connect(@addr, @port, method(:on_message)) @responses = {} @id = 0 @id_mutex = Mutex.new @@ -38,13 +38,19 @@ def initialize(loop = Coolio::Loop.new, opt = {}) def start @loop.attach(@conn) + @log.debug("starting counter client: #{@addr}:#{@port}") self rescue => e - @log.error e + if @log + @log.error e + else + STDERR.puts e + end end def stop @conn.close + @log.debug("calling stop in counter client: #{@addr}:#{@port}") end def establish(scope) @@ -123,7 +129,7 @@ def send_request(method, scope, params, opt = {}) res = Future.new(@loop, @loop_mutex) @responses[id] = res # set a response value to this future object at `on_message` request = build_request(method, id, scope, params, opt) - @log.debug(request) if @log + @log.debug(request) @conn.send_data request res end diff --git a/lib/fluent/counter/server.rb b/lib/fluent/counter/server.rb index d68bf1d106..d242b3868b 100644 --- a/lib/fluent/counter/server.rb +++ b/lib/fluent/counter/server.rb @@ -23,14 +23,14 @@ module Fluent module Counter class Server - DEFAULT_HOST = '127.0.0.1' + DEFAULT_ADDR = '127.0.0.1' DEFAULT_PORT = 4321 def initialize(name, opt = {}) raise 'Counter server name is invalid' unless Validator::VALID_NAME =~ name @name = name @opt = opt - @host = @opt[:host] || DEFAULT_HOST + @addr = @opt[:addr] || DEFAULT_ADDR @port = @opt[:port] || DEFAULT_PORT @loop = @opt[:loop] || Coolio::Loop.new @log = @opt[:log] || $log @@ -38,7 +38,7 @@ def initialize(name, opt = {}) @store = Fluent::Counter::Store.new(opt) @mutex_hash = MutexHash.new(@store) - @server = Coolio::TCPServer.new(@host, @port, Handler, method(:on_message)) + @server = Coolio::TCPServer.new(@addr, @port, Handler, method(:on_message)) @thread = nil @running = false end @@ -50,6 +50,7 @@ def start @loop.run(0.5) @running = false end + @log.debug("starting counter server #{@addr}:#{@port}") @mutex_hash.start self end @@ -61,6 +62,7 @@ def stop @loop.stop if @running @mutex_hash.stop @thread.join if @thread + @log.debug("calling stop in counter server #{@addr}:#{@port}") end def on_message(data) @@ -87,7 +89,7 @@ def establish(params, _scope, _options) if scope = valid_params.first new_scope = "#{@name}\t#{scope}" res.push_data new_scope - @log.debug("Establish new key: #{new_scope}") if @log + @log.debug("Establish new key: #{new_scope}") end res.to_hash @@ -105,7 +107,7 @@ def init(params, scope, options) begin param = key_hash[key] v = store.init(key, param, ignore: options['ignore']) - @log.debug("Create new key: #{param['name']}") if @log + @log.debug("Create new key: #{param['name']}") res.push_data v rescue => e res.push_error e @@ -130,7 +132,7 @@ def delete(params, scope, options) do_delete = lambda do |store, key| begin v = store.delete(key) - @log.debug("delete a key: #{key}") if @log + @log.debug("delete a key: #{key}") res.push_data v rescue => e res.push_error e @@ -160,7 +162,7 @@ def inc(params, scope, options) begin param = key_hash[key] v = store.inc(key, param, force: options['force']) - @log.debug("Increment #{key} by #{param['value']}") if @log + @log.debug("Increment #{key} by #{param['value']}") res.push_data v rescue => e res.push_error e @@ -185,7 +187,7 @@ def reset(params, scope, options) do_reset = lambda do |store, key| begin v = store.reset(key) - @log.debug("Reset #{key}'s' counter value") if @log + @log.debug("Reset #{key}'s' counter value") res.push_data v rescue => e res.push_error e @@ -210,7 +212,7 @@ def get(params, scope, _options) keys.each do |key| begin v = @store.get(key, raise_error: true) - @log.debug("Get counter value: #{key}") if @log + @log.debug("Get counter value: #{key}") res.push_data v rescue => e res.push_error e diff --git a/lib/fluent/counter/store.rb b/lib/fluent/counter/store.rb index 9e18ca4183..e8423438c8 100644 --- a/lib/fluent/counter/store.rb +++ b/lib/fluent/counter/store.rb @@ -14,9 +14,10 @@ # limitations under the License. # -require 'fluent/time' +require 'fluent/config' require 'fluent/counter/error' require 'fluent/plugin/storage_local' +require 'fluent/time' module Fluent module Counter @@ -26,11 +27,35 @@ def self.gen_key(scope, key) end def initialize(opt = {}) + @log = opt[:log] || $log + # Notice: This storage is not be implemented auto save. - @storage = Fluent::Plugin::LocalStorage.new - @storage.configure( - Fluent::Config::Element.new('storage', {}, {'persistent' => true, 'path' => opt[:path] }, []) - ) + @storage = Plugin.new_storage('local', parent: DummyParent.new(@log)) + conf = if opt[:path] + {'persistent' => true, 'path' => opt[:path] } + else + {'persistent' => false } + end + @storage.configure(Fluent::Config::Element.new('storage', {}, conf, [])) + end + + # This class behaves as a configurable plugin for using in storage (OwnedByMixin). + class DummyParent + include Configurable + + attr_reader :log + + def initialize(log) + @log = log + end + + def plugin_id + 'dummy_parent_store' + end + + def plugin_id_configured? + false + end end def start @@ -75,9 +100,9 @@ def delete(key) end def inc(key, data, force: false) - init(key, data) if force + value = data.delete('value') + init(key, data) if !key?(key) && force v = get(key, raise_error: true, raw: true) - value = data['value'] valid_type!(v, value) v['total'] += value diff --git a/lib/fluent/supervisor.rb b/lib/fluent/supervisor.rb index 6cae2801c5..d7f5a94bb2 100644 --- a/lib/fluent/supervisor.rb +++ b/lib/fluent/supervisor.rb @@ -17,6 +17,7 @@ require 'fileutils' require 'fluent/config' +require 'fluent/counter' require 'fluent/env' require 'fluent/engine' require 'fluent/error' @@ -54,6 +55,13 @@ def before_run install_windows_event_handler end + if counter = config[:counter_server] + @counter_endpoint = counter.endpoint + @counter_scope = counter.scope + @counter_path = counter.path + run_counter_server + end + socket_manager_path = ServerEngine::SocketManager::Server.generate_path ServerEngine::SocketManager::Server.open(socket_manager_path) ENV['SERVERENGINE_SOCKETMANAGER_PATH'] = socket_manager_path.to_s @@ -61,6 +69,7 @@ def before_run def after_run stop_rpc_server if @rpc_endpoint + stop_counter_server if @counter_endpoint Fluent::Supervisor.cleanup_resources end @@ -126,6 +135,19 @@ def stop_rpc_server @rpc_server.shutdown end + def run_counter_server + host, port = @counter_endpoint.split(':') + @counter = Fluent::Counter::Server.new( + @counter_scope, + { host: host, port: port, log: $log, path: @counter_path } + ) + @counter.start + end + + def stop_counter_server + @counter.stop + end + def install_supervisor_signal_handlers trap :HUP do $log.debug "fluentd supervisor process get SIGHUP" @@ -249,6 +271,7 @@ def self.load_config(path, params = {}) log_rotate_size = params['log_rotate_size'] rpc_endpoint = system_config.rpc_endpoint enable_get_dump = system_config.enable_get_dump + counter_server = system_config.counter_server log_opts = {suppress_repeated_stacktrace: suppress_repeated_stacktrace} logger_initializer = Supervisor::LoggerInitializer.new( @@ -290,6 +313,7 @@ def self.load_config(path, params = {}) suppress_repeated_stacktrace: suppress_repeated_stacktrace, daemonize: daemonize, rpc_endpoint: rpc_endpoint, + counter_server: counter_server, enable_get_dump: enable_get_dump, windows_daemon_cmdline: [ServerEngine.ruby_bin_path, File.join(File.dirname(__FILE__), 'daemon.rb'), diff --git a/lib/fluent/system_config.rb b/lib/fluent/system_config.rb index 0dbd6731e5..de99a63de5 100644 --- a/lib/fluent/system_config.rb +++ b/lib/fluent/system_config.rb @@ -26,7 +26,7 @@ class SystemConfig :suppress_repeated_stacktrace, :emit_error_log_interval, :suppress_config_dump, :log_event_verbose, :without_source, :rpc_endpoint, :enable_get_dump, :process_name, - :file_permission, :dir_permission, + :file_permission, :dir_permission, :counter_server, :counter_client, ] config_param :workers, :integer, default: 1 @@ -51,6 +51,22 @@ class SystemConfig config_param :time_format, :string, default: '%Y-%m-%d %H:%M:%S %z' end + config_section :counter_server, multi: false do + desc 'scope name of counter server' + config_param :scope, :string + + desc 'endpoint of counter server' + config_param :endpoint, :string # host:port + + desc 'backup file path of counter values' + config_param :path, :string + end + + config_section :counter_client, multi: false do + desc 'endpoint of counter client' + config_param :endpoint, :string # host:port + end + def self.create(conf) systems = conf.elements(name: 'system') return SystemConfig.new if systems.empty? @@ -98,7 +114,7 @@ def attach(supervisor) supervisor.instance_eval { SYSTEM_CONFIG_PARAMETERS.each do |param| case param - when :rpc_endpoint, :enable_get_dump, :process_name, :file_permission, :dir_permission + when :rpc_endpoint, :enable_get_dump, :process_name, :file_permission, :dir_permission, :counter_server, :counter_client next # doesn't exist in command line options when :emit_error_log_interval system.emit_error_log_interval = @suppress_interval if @suppress_interval @@ -136,6 +152,8 @@ def apply(supervisor) instance_variable_set("@#{param}", param_value) end end + #@counter_server = system.counter_server unless system.counter_server.nil? + #@counter_client = system.counter_client unless system.counter_client.nil? } end diff --git a/test/counter/test_client.rb b/test/counter/test_client.rb index fb0e979015..010dac2c01 100644 --- a/test/counter/test_client.rb +++ b/test/counter/test_client.rb @@ -6,7 +6,7 @@ require 'timecop' class CounterClientTest < ::Test::Unit::TestCase - TEST_HOST = '127.0.0.1' + TEST_ADDR = '127.0.0.1' TEST_PORT = '8277' setup do @@ -16,7 +16,7 @@ class CounterClientTest < ::Test::Unit::TestCase @now = Fluent::EventTime.now @options = { - host: TEST_HOST, + addr: TEST_ADDR, port: TEST_PORT, log: $log, } diff --git a/test/test_supervisor.rb b/test/test_supervisor.rb index 1bc3649455..f50effd4a9 100644 --- a/test/test_supervisor.rb +++ b/test/test_supervisor.rb @@ -134,6 +134,11 @@ def test_system_config format json time_format %Y + + endpoint 127.0.0.1:4321 + scope server1 + path /tmp/backup + EOC conf = Fluent::Config.parse(conf_data, "(test)", "(test_dir)", true) @@ -151,6 +156,10 @@ def test_system_config assert_equal TMP_ROOT_DIR, sys_conf.root_dir assert_equal :json, sys_conf.log.format assert_equal '%Y', sys_conf.log.time_format + counter_server = sys_conf.counter_server + assert_equal '127.0.0.1:4321', counter_server.endpoint + assert_equal 'server1', counter_server.scope + assert_equal '/tmp/backup', counter_server.path end def test_main_process_signal_handlers From bc997ac9c1149acf739d427357f41087c9eb5511 Mon Sep 17 00:00:00 2001 From: Masahiro Nakagawa Date: Fri, 30 Mar 2018 12:29:54 +0900 Subject: [PATCH 13/23] Fix plugin_root_dir related test errors --- lib/fluent/counter/store.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/fluent/counter/store.rb b/lib/fluent/counter/store.rb index e8423438c8..6ab98eb835 100644 --- a/lib/fluent/counter/store.rb +++ b/lib/fluent/counter/store.rb @@ -56,6 +56,11 @@ def plugin_id def plugin_id_configured? false end + + # storage_local calls PluginId#plugin_root_dir + def plugin_root_dir + nil + end end def start From 056a344ad81eb71694e46095e15055c32b2e0301 Mon Sep 17 00:00:00 2001 From: Masahiro Nakagawa Date: Fri, 30 Mar 2018 12:32:35 +0900 Subject: [PATCH 14/23] Change counter parameters to follow other plugins --- lib/fluent/supervisor.rb | 12 ++++-------- lib/fluent/system_config.rb | 14 +++++++++----- test/test_supervisor.rb | 17 +++++++++++++---- 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/lib/fluent/supervisor.rb b/lib/fluent/supervisor.rb index d7f5a94bb2..fa773f3818 100644 --- a/lib/fluent/supervisor.rb +++ b/lib/fluent/supervisor.rb @@ -56,10 +56,7 @@ def before_run end if counter = config[:counter_server] - @counter_endpoint = counter.endpoint - @counter_scope = counter.scope - @counter_path = counter.path - run_counter_server + run_counter_server(counter) end socket_manager_path = ServerEngine::SocketManager::Server.generate_path @@ -135,11 +132,10 @@ def stop_rpc_server @rpc_server.shutdown end - def run_counter_server - host, port = @counter_endpoint.split(':') + def run_counter_server(counter_conf) @counter = Fluent::Counter::Server.new( - @counter_scope, - { host: host, port: port, log: $log, path: @counter_path } + counter_conf.scope, + {host: counter_conf.bind, port: counter_conf.port, log: $log, path: counter_conf.backup_path} ) @counter.start end diff --git a/lib/fluent/system_config.rb b/lib/fluent/system_config.rb index de99a63de5..edf975d0f2 100644 --- a/lib/fluent/system_config.rb +++ b/lib/fluent/system_config.rb @@ -55,16 +55,20 @@ class SystemConfig desc 'scope name of counter server' config_param :scope, :string - desc 'endpoint of counter server' - config_param :endpoint, :string # host:port + desc 'the port of counter server to listen to' + config_param :port, :integer, default: nil + desc 'the bind address of counter server to listen to' + config_param :bind, :string, default: nil desc 'backup file path of counter values' - config_param :path, :string + config_param :backup_path, :string end config_section :counter_client, multi: false do - desc 'endpoint of counter client' - config_param :endpoint, :string # host:port + desc 'the port of counter server' + config_param :port, :integer, default: nil + desc 'the IP address or hostname of counter server' + config_param :host, :string end def self.create(conf) diff --git a/test/test_supervisor.rb b/test/test_supervisor.rb index f50effd4a9..02cbd72593 100644 --- a/test/test_supervisor.rb +++ b/test/test_supervisor.rb @@ -135,10 +135,15 @@ def test_system_config time_format %Y - endpoint 127.0.0.1:4321 + bind 127.0.0.1 + port 4321 scope server1 - path /tmp/backup + backup_path /tmp/backup + + host 127.0.0.1 + port 4321 + EOC conf = Fluent::Config.parse(conf_data, "(test)", "(test_dir)", true) @@ -157,9 +162,13 @@ def test_system_config assert_equal :json, sys_conf.log.format assert_equal '%Y', sys_conf.log.time_format counter_server = sys_conf.counter_server - assert_equal '127.0.0.1:4321', counter_server.endpoint + assert_equal '127.0.0.1', counter_server.bind + assert_equal 4321, counter_server.port assert_equal 'server1', counter_server.scope - assert_equal '/tmp/backup', counter_server.path + assert_equal '/tmp/backup', counter_server.backup_path + counter_client = sys_conf.counter_client + assert_equal '127.0.0.1', counter_client.host + assert_equal 4321, counter_client.port end def test_main_process_signal_handlers From 158da25e4f16c64d10e7bf6b43d339f016881573 Mon Sep 17 00:00:00 2001 From: Masahiro Nakagawa Date: Fri, 30 Mar 2018 12:34:43 +0900 Subject: [PATCH 15/23] Change default port to 24321 --- lib/fluent/counter/client.rb | 2 +- lib/fluent/counter/server.rb | 2 +- test/test_supervisor.rb | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/fluent/counter/client.rb b/lib/fluent/counter/client.rb index c0546c770b..e89c5e025e 100644 --- a/lib/fluent/counter/client.rb +++ b/lib/fluent/counter/client.rb @@ -20,7 +20,7 @@ module Fluent module Counter class Client - DEFAULT_PORT = 4321 + DEFAULT_PORT = 24321 DEFAULT_ADDR = '127.0.0.1' ID_LIMIT_COUNT = 1 << 31 diff --git a/lib/fluent/counter/server.rb b/lib/fluent/counter/server.rb index d242b3868b..c35d3073d2 100644 --- a/lib/fluent/counter/server.rb +++ b/lib/fluent/counter/server.rb @@ -24,7 +24,7 @@ module Fluent module Counter class Server DEFAULT_ADDR = '127.0.0.1' - DEFAULT_PORT = 4321 + DEFAULT_PORT = 24321 def initialize(name, opt = {}) raise 'Counter server name is invalid' unless Validator::VALID_NAME =~ name diff --git a/test/test_supervisor.rb b/test/test_supervisor.rb index 02cbd72593..5f094e5ceb 100644 --- a/test/test_supervisor.rb +++ b/test/test_supervisor.rb @@ -136,13 +136,13 @@ def test_system_config bind 127.0.0.1 - port 4321 + port 24321 scope server1 backup_path /tmp/backup host 127.0.0.1 - port 4321 + port 24321 EOC @@ -163,12 +163,12 @@ def test_system_config assert_equal '%Y', sys_conf.log.time_format counter_server = sys_conf.counter_server assert_equal '127.0.0.1', counter_server.bind - assert_equal 4321, counter_server.port + assert_equal 24321, counter_server.port assert_equal 'server1', counter_server.scope assert_equal '/tmp/backup', counter_server.backup_path counter_client = sys_conf.counter_client assert_equal '127.0.0.1', counter_client.host - assert_equal 4321, counter_client.port + assert_equal 24321, counter_client.port end def test_main_process_signal_handlers From bb91b2ca16c4e817f025487b74c495ebe3c694f5 Mon Sep 17 00:00:00 2001 From: Masahiro Nakagawa Date: Fri, 30 Mar 2018 14:07:13 +0900 Subject: [PATCH 16/23] Add timeout optino to counter client --- lib/fluent/counter/client.rb | 25 ++++++++++++++++--------- lib/fluent/system_config.rb | 2 ++ test/test_supervisor.rb | 2 ++ 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/lib/fluent/counter/client.rb b/lib/fluent/counter/client.rb index e89c5e025e..a780baa52b 100644 --- a/lib/fluent/counter/client.rb +++ b/lib/fluent/counter/client.rb @@ -16,20 +16,23 @@ require 'cool.io' require 'fluent/counter/base_socket' +require 'timeout' module Fluent module Counter class Client DEFAULT_PORT = 24321 DEFAULT_ADDR = '127.0.0.1' + DEFAULT_TIMEOUT = 5 ID_LIMIT_COUNT = 1 << 31 def initialize(loop = nil, opt = {}) @loop = loop || Coolio::Loop.new @port = opt[:port] || DEFAULT_PORT - @addr = opt[:addr] || DEFAULT_ADDR + @host = opt[:host] || DEFAULT_ADDR @log = opt[:log] || $log - @conn = Connection.connect(@addr, @port, method(:on_message)) + @timeout = opt[:timeout] || DEFAULT_TIMEOUT + @conn = Connection.connect(@host, @port, method(:on_message)) @responses = {} @id = 0 @id_mutex = Mutex.new @@ -38,7 +41,7 @@ def initialize(loop = nil, opt = {}) def start @loop.attach(@conn) - @log.debug("starting counter client: #{@addr}:#{@port}") + @log.debug("starting counter client: #{@host}:#{@port}") self rescue => e if @log @@ -50,15 +53,19 @@ def start def stop @conn.close - @log.debug("calling stop in counter client: #{@addr}:#{@port}") + @log.debug("calling stop in counter client: #{@host}:#{@port}") end def establish(scope) - response = send_request('establish', nil, [scope]) - - raise response.errors.first if response.errors? - data = response.data - @scope = data.first + scope = Timeout.timeout(@timeout) { + response = send_request('establish', nil, [scope]) + raise response.errors.first if response.errors? + data = response.data + data.first + } + @scope = scope + rescue Timeout::Error + raise "Can't establish the connection to counter server due to timeout" end # === Example diff --git a/lib/fluent/system_config.rb b/lib/fluent/system_config.rb index edf975d0f2..97f875a208 100644 --- a/lib/fluent/system_config.rb +++ b/lib/fluent/system_config.rb @@ -69,6 +69,8 @@ class SystemConfig config_param :port, :integer, default: nil desc 'the IP address or hostname of counter server' config_param :host, :string + desc 'the timeout of each operation' + config_param :timeout, :time, default: nil end def self.create(conf) diff --git a/test/test_supervisor.rb b/test/test_supervisor.rb index 5f094e5ceb..0d6f2be2c3 100644 --- a/test/test_supervisor.rb +++ b/test/test_supervisor.rb @@ -143,6 +143,7 @@ def test_system_config host 127.0.0.1 port 24321 + timeout 2 EOC @@ -169,6 +170,7 @@ def test_system_config counter_client = sys_conf.counter_client assert_equal '127.0.0.1', counter_client.host assert_equal 24321, counter_client.port + assert_equal 2, counter_client.timeout end def test_main_process_signal_handlers From 77deb70d73599130fc57f107afa8f6aff2d86882 Mon Sep 17 00:00:00 2001 From: Masahiro Nakagawa Date: Fri, 30 Mar 2018 14:07:29 +0900 Subject: [PATCH 17/23] Add counter plugin helper to manage counter client --- lib/fluent/plugin_helper.rb | 1 + lib/fluent/plugin_helper/counter.rb | 51 +++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 lib/fluent/plugin_helper/counter.rb diff --git a/lib/fluent/plugin_helper.rb b/lib/fluent/plugin_helper.rb index 83fa874d1a..363b1866f5 100644 --- a/lib/fluent/plugin_helper.rb +++ b/lib/fluent/plugin_helper.rb @@ -26,6 +26,7 @@ require 'fluent/plugin_helper/extract' require 'fluent/plugin_helper/socket' require 'fluent/plugin_helper/server' +require 'fluent/plugin_helper/counter' require 'fluent/plugin_helper/retry_state' require 'fluent/plugin_helper/record_accessor' require 'fluent/plugin_helper/compat_parameters' diff --git a/lib/fluent/plugin_helper/counter.rb b/lib/fluent/plugin_helper/counter.rb new file mode 100644 index 0000000000..05cc786209 --- /dev/null +++ b/lib/fluent/plugin_helper/counter.rb @@ -0,0 +1,51 @@ +# +# Fluentd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +require 'fluent/plugin' +require 'fluent/counter/client' + +module Fluent + module PluginHelper + module Counter + def counter_client_create(scope:, loop: Coolio::Loop.new) + client_conf = system_config.counter_client + raise Fluent::ConfigError, ' is required in ' unless client_conf + counter_client = Fluent::Counter::Client.new(loop, port: client_conf.port, host: client_conf.host, log: log, timeout: client_conf.timeout) + counter_client.start + counter_client.establish(scope) + @_counter_client = counter_client + counter_client + end + + attr_reader :_counter_client + + def initialize + super + @_counter_client = nil + end + + def stop + super + @_counter_client.stop + end + + def terminate + @_counter_client = nil + super + end + end + end +end From 6095ad7212c2d36f5de2c069f52cfee6e1ac58d0 Mon Sep 17 00:00:00 2001 From: Masahiro Nakagawa Date: Fri, 30 Mar 2018 14:11:18 +0900 Subject: [PATCH 18/23] Fix example conf --- example/counter.conf | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/example/counter.conf b/example/counter.conf index ff5958fc04..e8fcbaf6a8 100644 --- a/example/counter.conf +++ b/example/counter.conf @@ -1,7 +1,8 @@ scope server1 - endpoint 127.0.0.1:4321 + bind 127.0.0.1 + port 24321 path tmp/back From 44f341353ea109804ce15aa0361c79d414af1dd2 Mon Sep 17 00:00:00 2001 From: Masahiro Nakagawa Date: Thu, 5 Apr 2018 04:50:19 +0900 Subject: [PATCH 19/23] Use class instead of raw Hash for client result --- lib/fluent/counter/client.rb | 25 +++++++++++++--- test/counter/test_client.rb | 56 ++++++++++++++++++------------------ 2 files changed, 49 insertions(+), 32 deletions(-) diff --git a/lib/fluent/counter/client.rb b/lib/fluent/counter/client.rb index a780baa52b..ecf574058a 100644 --- a/lib/fluent/counter/client.rb +++ b/lib/fluent/counter/client.rb @@ -192,6 +192,23 @@ def on_message(data) end class Future + class Result + attr_reader :data, :errors + + def initialize(result) + @errors = result['errors'] + @data = result['data'] + end + + def success? + @errors.nil? || @errors.empty? + end + + def error? + !success? + end + end + def initialize(loop, mutex) @set = false @result = nil @@ -200,12 +217,12 @@ def initialize(loop, mutex) end def set(v) - @result = v + @result = Result.new(v) @set = true end def errors - get['errors'] + get.errors end def errors? @@ -214,11 +231,11 @@ def errors? end def data - get['data'] + get.data end def get - # Block until `set` method is called and @result is set a value + # Block until `set` method is called and @result is set join if @result.nil? @result end diff --git a/test/counter/test_client.rb b/test/counter/test_client.rb index 010dac2c01..ebb41ea773 100644 --- a/test/counter/test_client.rb +++ b/test/counter/test_client.rb @@ -103,9 +103,9 @@ def extract_value_from_server(server, scope, name) assert_nil extract_value_from_server(@server, @scope, param[:name]) response = @client.init(param) - data = response['data'].first + data = response.data.first - assert_nil response['errors'] + assert_nil response.errors assert_equal param[:name], data['name'] assert_equal param[:reset_interval], data['reset_interval'] assert_equal param[:type], data['type'] @@ -149,9 +149,9 @@ def extract_value_from_server(server, scope, name) @client.init(:name => 'key1', :reset_interval => 10) response = @client.init(param) - errors = response['errors'].first + errors = response.errors.first - assert_empty response['data'] + assert_empty response.data assert_equal expected_error, errors end @@ -163,7 +163,7 @@ def extract_value_from_server(server, scope, name) res2 = @client.init({ name: 'key1', reset_interval: 10 }, options: { ignore: true }) end - assert_equal res1['data'], res2['data'] + assert_equal res1.data, res2.data end test 'return an error object and data object' do @@ -172,8 +172,8 @@ def extract_value_from_server(server, scope, name) @client.init(param) response = @client.init([param2, param]) - data = response['data'].first - error = response['errors'].first + data = response.data.first + error = response.errors.first assert_equal param2[:name], data['name'] assert_equal param2[:reset_interval], data['reset_interval'] @@ -204,9 +204,9 @@ def extract_value_from_server(server, scope, name) assert extract_value_from_server(@server, @scope, @name) response = @client.delete(@name) - v = response['data'].first + v = response.data.first - assert_nil response['errors'] + assert_nil response.errors assert_equal @init_obj[:name], v['name'] assert_equal @init_obj[:type], v['type'] assert_equal @init_obj[:reset_interval], v['reset_interval'] @@ -234,9 +234,9 @@ def extract_value_from_server(server, scope, name) test 'return an error object' do |(param, expected_error)| response = @client.delete(param) - errors = response['errors'].first + errors = response.errors.first - assert_empty response['data'] + assert_empty response.data assert_equal expected_error, errors end @@ -244,8 +244,8 @@ def extract_value_from_server(server, scope, name) unknown_name = 'key2' response = @client.delete(@name, unknown_name) - data = response['data'].first - error = response['errors'].first + data = response.data.first + error = response.errors.first assert_equal @name, data['name'] assert_equal @init_obj[:reset_interval], data['reset_interval'] @@ -332,8 +332,8 @@ def extract_value_from_server(server, scope, name) test 'return an error object' do |(param, expected_error)| response = @client.inc(param) - errors = response['errors'].first - assert_empty response['data'] + errors = response.errors.first + assert_empty response.data assert_equal expected_error, errors end @@ -344,8 +344,8 @@ def extract_value_from_server(server, scope, name) ] response = @client.inc(parmas) - data = response['data'].first - error = response['errors'].first + data = response.data.first + error = response.errors.first assert_equal @name, data['name'] assert_equal 10, data['current'] @@ -374,7 +374,7 @@ def extract_value_from_server(server, scope, name) test 'get a value' do v1 = extract_value_from_server(@server, @scope, @name) - v2 = @client.get(@name)['data'].first + v2 = @client.get(@name).data.first assert_equal v1['name'], v2['name'] assert_equal v1['current'], v2['current'] @@ -402,9 +402,9 @@ def extract_value_from_server(server, scope, name) test 'return an error object' do |(param, expected_error)| response = @client.get(param) - errors = response['errors'].first + errors = response.errors.first - assert_empty response['data'] + assert_empty response.data assert_equal expected_error, errors end @@ -412,8 +412,8 @@ def extract_value_from_server(server, scope, name) unknown_name = 'key2' response = @client.get(@name, unknown_name) - data = response['data'].first - error = response['errors'].first + data = response.data.first + error = response.errors.first assert_equal @name, data['name'] assert_equal @init_obj[:reset_interval], data['reset_interval'] @@ -451,7 +451,7 @@ def extract_value_from_server(server, scope, name) Timecop.travel(travel_sec) v2 = @client.reset(@name) - data = v2['data'].first + data = v2.data.first c = data['counter_data'] @@ -479,7 +479,7 @@ def extract_value_from_server(server, scope, name) Timecop.travel(travel_sec) v2 = @client.reset(@name) - data = v2['data'].first + data = v2.data.first c = data['counter_data'] @@ -516,9 +516,9 @@ def extract_value_from_server(server, scope, name) test 'return an error object' do |(param, expected_error)| response = @client.reset(param) - errors = response['errors'].first + errors = response.errors.first - assert_empty response['data'] + assert_empty response.data assert_equal expected_error, errors end @@ -529,8 +529,8 @@ def extract_value_from_server(server, scope, name) Timecop.travel(travel_sec) response = @client.reset(@name, unknown_name) - data = response['data'].first - error = response['errors'].first + data = response.data.first + error = response.errors.first counter = data['counter_data'] assert_true data['success'] From 59f84410357885ae215fb8e77617fea69c7f30da Mon Sep 17 00:00:00 2001 From: Masahiro Nakagawa Date: Thu, 5 Apr 2018 05:09:04 +0900 Subject: [PATCH 20/23] Fix shutdown condition for counter server --- lib/fluent/supervisor.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/fluent/supervisor.rb b/lib/fluent/supervisor.rb index fa773f3818..8fe36f901a 100644 --- a/lib/fluent/supervisor.rb +++ b/lib/fluent/supervisor.rb @@ -42,6 +42,8 @@ module Fluent module ServerModule def before_run @start_time = Time.now + @rpc_server = nil + @counter = nil if config[:rpc_endpoint] @rpc_endpoint = config[:rpc_endpoint] @@ -66,7 +68,7 @@ def before_run def after_run stop_rpc_server if @rpc_endpoint - stop_counter_server if @counter_endpoint + stop_counter_server if @counter Fluent::Supervisor.cleanup_resources end From 3ac49797d8cfd58b25a1dad7a1e0cc57bb79daac Mon Sep 17 00:00:00 2001 From: Masahiro Nakagawa Date: Fri, 6 Apr 2018 05:17:15 +0900 Subject: [PATCH 21/23] counter: Client API supports block style result handling Signed-off-by: Masahiro Nakagawa --- lib/fluent/counter/client.rb | 87 +++++++++++++++++++++++++++++++++--- 1 file changed, 81 insertions(+), 6 deletions(-) diff --git a/lib/fluent/counter/client.rb b/lib/fluent/counter/client.rb index ecf574058a..df36a78996 100644 --- a/lib/fluent/counter/client.rb +++ b/lib/fluent/counter/client.rb @@ -75,20 +75,50 @@ def establish(scope) # 2. init({ name: 'name',reset_interval: 20 }, options: {}) # 3. init([{ name: 'name1',reset_interval: 20 }, { name: 'name2',reset_interval: 20 }]) # 4. init([{ name: 'name1',reset_interval: 20 }, { name: 'name2',reset_interval: 20 }], options: {}) + # 5. init([{ name: 'name1',reset_interval: 20 }, { name: 'name2',reset_interval: 20 }], options: {async: true}) { |res| ... } def init(params, options: {}) exist_scope! params = [params] unless params.is_a?(Array) res = send_request('init', @scope, params, options) # if `async` is true, return a Future object (non blocking). - # if `async` is false or missing, block at this method and return a Hash object. - options[:async] ? res : res.get + # if `async` is false or missing, block at this method and return a Future::Result object. + if options[:async] + if block_given? + Thread.start do + yield res.get + end + else + res + end + else + if block_given? + yield res.get + else + res.get + end + end end def delete(*params, options: {}) exist_scope! res = send_request('delete', @scope, params, options) - options[:async] ? res : res.get + + if options[:async] + if block_given? + Thread.start do + yield res.get + end + else + res + end + else + if block_given? + yield res.get + else + res.get + end + end end # === Example @@ -102,19 +132,64 @@ def inc(params, options: {}) exist_scope! params = [params] unless params.is_a?(Array) res = send_request('inc', @scope, params, options) - options[:async] ? res : res.get + + if options[:async] + if block_given? + Thread.start do + yield res.get + end + else + res + end + else + if block_given? + yield res.get + else + res.get + end + end end def get(*params, options: {}) exist_scope! res = send_request('get', @scope, params, options) - options[:async] ? res : res.get + + if options[:async] + if block_given? + Thread.start do + yield res.get + end + else + res + end + else + if block_given? + yield res.get + else + res.get + end + end end def reset(*params, options: {}) exist_scope! res = send_request('reset', @scope, params, options) - options[:async] ? res : res.get + + if options[:async] + if block_given? + Thread.start do + yield res.get + end + else + res + end + else + if block_given? + yield res.get + else + res.get + end + end end private From 21beb517c59208cbeb6fb71776fab364b256dee1 Mon Sep 17 00:00:00 2001 From: Masahiro Nakagawa Date: Fri, 6 Apr 2018 05:51:28 +0900 Subject: [PATCH 22/23] Fix comment of Client#inc Signed-off-by: Masahiro Nakagawa --- lib/fluent/counter/client.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/fluent/counter/client.rb b/lib/fluent/counter/client.rb index df36a78996..dda4cc2b2c 100644 --- a/lib/fluent/counter/client.rb +++ b/lib/fluent/counter/client.rb @@ -124,10 +124,10 @@ def delete(*params, options: {}) # === Example # `inc` receives various arguments. # - # 1. init(name: 'name') - # 2. init({ name: 'name',value: 20 }, options: {}) - # 3. init([{ name: 'name1',value: 20 }, { name: 'name2',value: 20 }]) - # 4. init([{ name: 'name1',value: 20 }, { name: 'name2',value: 20 }], options: {}) + # 1. inc(name: 'name') + # 2. inc({ name: 'name',value: 20 }, options: {}) + # 3. inc([{ name: 'name1',value: 20 }, { name: 'name2',value: 20 }]) + # 4. inc([{ name: 'name1',value: 20 }, { name: 'name2',value: 20 }], options: {}) def inc(params, options: {}) exist_scope! params = [params] unless params.is_a?(Array) From 33f3682f9719e05feb4f7f0b2f8d39279124b733 Mon Sep 17 00:00:00 2001 From: Masahiro Nakagawa Date: Tue, 17 Apr 2018 17:05:12 +0900 Subject: [PATCH 23/23] Remove sync API from counter client Signed-off-by: Masahiro Nakagawa --- lib/fluent/counter/client.rb | 83 +++++++++------------------------ test/counter/test_client.rb | 90 ++++++++++++++++-------------------- 2 files changed, 61 insertions(+), 112 deletions(-) diff --git a/lib/fluent/counter/client.rb b/lib/fluent/counter/client.rb index dda4cc2b2c..fc2ea9989a 100644 --- a/lib/fluent/counter/client.rb +++ b/lib/fluent/counter/client.rb @@ -75,28 +75,19 @@ def establish(scope) # 2. init({ name: 'name',reset_interval: 20 }, options: {}) # 3. init([{ name: 'name1',reset_interval: 20 }, { name: 'name2',reset_interval: 20 }]) # 4. init([{ name: 'name1',reset_interval: 20 }, { name: 'name2',reset_interval: 20 }], options: {}) - # 5. init([{ name: 'name1',reset_interval: 20 }, { name: 'name2',reset_interval: 20 }], options: {async: true}) { |res| ... } + # 5. init([{ name: 'name1',reset_interval: 20 }, { name: 'name2',reset_interval: 20 }]) { |res| ... } def init(params, options: {}) exist_scope! params = [params] unless params.is_a?(Array) res = send_request('init', @scope, params, options) - # if `async` is true, return a Future object (non blocking). # if `async` is false or missing, block at this method and return a Future::Result object. - if options[:async] - if block_given? - Thread.start do - yield res.get - end - else - res - end - else - if block_given? + if block_given? + Thread.start do yield res.get - else - res.get end + else + res end end @@ -104,20 +95,12 @@ def delete(*params, options: {}) exist_scope! res = send_request('delete', @scope, params, options) - if options[:async] - if block_given? - Thread.start do - yield res.get - end - else - res - end - else - if block_given? + if block_given? + Thread.start do yield res.get - else - res.get end + else + res end end @@ -133,20 +116,12 @@ def inc(params, options: {}) params = [params] unless params.is_a?(Array) res = send_request('inc', @scope, params, options) - if options[:async] - if block_given? - Thread.start do - yield res.get - end - else - res - end - else - if block_given? + if block_given? + Thread.start do yield res.get - else - res.get end + else + res end end @@ -154,20 +129,12 @@ def get(*params, options: {}) exist_scope! res = send_request('get', @scope, params, options) - if options[:async] - if block_given? - Thread.start do - yield res.get - end - else - res - end - else - if block_given? + if block_given? + Thread.start do yield res.get - else - res.get end + else + res end end @@ -175,20 +142,12 @@ def reset(*params, options: {}) exist_scope! res = send_request('reset', @scope, params, options) - if options[:async] - if block_given? - Thread.start do - yield res.get - end - else - res - end - else - if block_given? + if block_given? + Thread.start do yield res.get - else - res.get end + else + res end end diff --git a/test/counter/test_client.rb b/test/counter/test_client.rb index ebb41ea773..bab55b5a1f 100644 --- a/test/counter/test_client.rb +++ b/test/counter/test_client.rb @@ -48,7 +48,6 @@ class CounterClientTest < ::Test::Unit::TestCase test 'call a set method to a corresponding object' do @future.should_receive(:set).once.with(Hash) - @client.send(:on_message, { 'id' => 1 }) end @@ -102,7 +101,7 @@ def extract_value_from_server(server, scope, name) test 'create a value' do |(param, initial_value)| assert_nil extract_value_from_server(@server, @scope, param[:name]) - response = @client.init(param) + response = @client.init(param).get data = response.data.first assert_nil response.errors @@ -123,7 +122,7 @@ def extract_value_from_server(server, scope, name) test 'raise an error when @scope is nil' do @client.instance_variable_set(:@scope, nil) assert_raise 'Call `establish` method to get a `scope` before calling this method' do - @client.init(name: 'key1', reset_interval: 10) + @client.init(name: 'key1', reset_interval: 10).get end end @@ -146,9 +145,8 @@ def extract_value_from_server(server, scope, name) ] ) test 'return an error object' do |(param, expected_error)| - @client.init(:name => 'key1', :reset_interval => 10) - response = @client.init(param) - + @client.init(:name => 'key1', :reset_interval => 10).get + response = @client.init(param).get errors = response.errors.first assert_empty response.data @@ -156,22 +154,20 @@ def extract_value_from_server(server, scope, name) end test 'return an existing value when passed key already exists and ignore option is true' do - res1 = @client.init(name: 'key1', reset_interval: 10) - + res1 = @client.init(name: 'key1', reset_interval: 10).get res2 = nil assert_nothing_raised do - res2 = @client.init({ name: 'key1', reset_interval: 10 }, options: { ignore: true }) + res2 = @client.init({ name: 'key1', reset_interval: 10 }, options: { ignore: true }).get end - assert_equal res1.data, res2.data end test 'return an error object and data object' do param = { name: 'key1', reset_interval: 10 } param2 = { name: 'key2', reset_interval: 10 } - @client.init(param) + @client.init(param).get - response = @client.init([param2, param]) + response = @client.init([param2, param]).get data = response.data.first error = response.errors.first @@ -182,9 +178,9 @@ def extract_value_from_server(server, scope, name) assert_equal "#{@scope}\t#{param[:name]} already exists in counter", error['message'] end - test 'return a future object when async option is true' do + test 'return a future object when async call' do param = { name: 'key', reset_interval: 10 } - r = @client.init(param, options: { async: true }) + r = @client.init(param) assert_true r.is_a?(Fluent::Counter::Future) assert_nil r.errors end @@ -197,13 +193,13 @@ def extract_value_from_server(server, scope, name) @key = Fluent::Counter::Store.gen_key(@scope, @name) @init_obj = { name: @name, reset_interval: 20, type: 'numeric' } - @client.init(@init_obj) + @client.init(@init_obj).get end test 'delete a value' do assert extract_value_from_server(@server, @scope, @name) - response = @client.delete(@name) + response = @client.delete(@name).get v = response.data.first assert_nil response.errors @@ -217,7 +213,7 @@ def extract_value_from_server(server, scope, name) test 'raise an error when @scope is nil' do @client.instance_variable_set(:@scope, nil) assert_raise 'Call `establish` method to get a `scope` before calling this method' do - @client.delete(@name) + @client.delete(@name).get end end @@ -232,8 +228,7 @@ def extract_value_from_server(server, scope, name) ] ) test 'return an error object' do |(param, expected_error)| - response = @client.delete(param) - + response = @client.delete(param).get errors = response.errors.first assert_empty response.data @@ -243,7 +238,7 @@ def extract_value_from_server(server, scope, name) test 'return an error object and data object' do unknown_name = 'key2' - response = @client.delete(@name, unknown_name) + response = @client.delete(@name, unknown_name).get data = response.data.first error = response.errors.first @@ -256,8 +251,8 @@ def extract_value_from_server(server, scope, name) assert_nil extract_value_from_server(@server, @scope, @name) end - test 'return a future object when async option is true' do - r = @client.delete(@name, options: { async: true }) + test 'return a future object when async call' do + r = @client.delete(@name) assert_true r.is_a?(Fluent::Counter::Future) assert_nil r.errors end @@ -270,7 +265,7 @@ def extract_value_from_server(server, scope, name) @key = Fluent::Counter::Store.gen_key(@scope, @name) @init_obj = { name: @name, reset_interval: 20, type: 'numeric' } - @client.init(@init_obj) + @client.init(@init_obj).get end test 'increment a value' do @@ -280,7 +275,7 @@ def extract_value_from_server(server, scope, name) Timecop.travel(1) inc_obj = { name: @name, value: 10 } - @client.inc(inc_obj) + @client.inc(inc_obj).get v = extract_value_from_server(@server, @scope, @name) assert_equal inc_obj[:value], v['total'] @@ -294,7 +289,7 @@ def extract_value_from_server(server, scope, name) assert_nil extract_value_from_server(@server, @scope, name) - @client.inc(param, options: { force: true }) + @client.inc(param, options: { force: true }).get v = extract_value_from_server(@server, @scope, name) assert v @@ -307,7 +302,7 @@ def extract_value_from_server(server, scope, name) test 'raise an error when @scope is nil' do @client.instance_variable_set(:@scope, nil) assert_raise 'Call `establish` method to get a `scope` before calling this method' do - @client.inc(name: 'name', value: 1) + @client.inc(name: 'name', value: 1).get end end @@ -330,8 +325,7 @@ def extract_value_from_server(server, scope, name) ] ) test 'return an error object' do |(param, expected_error)| - response = @client.inc(param) - + response = @client.inc(param).get errors = response.errors.first assert_empty response.data assert_equal expected_error, errors @@ -342,7 +336,7 @@ def extract_value_from_server(server, scope, name) { name: @name, value: 10 }, { name: 'unknown_key', value: 9 }, ] - response = @client.inc(parmas) + response = @client.inc(parmas).get data = response.data.first error = response.errors.first @@ -355,9 +349,9 @@ def extract_value_from_server(server, scope, name) assert_equal "`#{@scope}\tunknown_key` doesn't exist in counter", error['message'] end - test 'return a future object when async option is true' do + test 'return a future object when async call' do param = { name: 'key', value: 10 } - r = @client.inc(param, options: { async: true }) + r = @client.inc(param) assert_true r.is_a?(Fluent::Counter::Future) assert_nil r.errors end @@ -369,7 +363,7 @@ def extract_value_from_server(server, scope, name) @name = 'key' @init_obj = { name: @name, reset_interval: 20, type: 'numeric' } - @client.init(@init_obj) + @client.init(@init_obj).get end test 'get a value' do @@ -385,7 +379,7 @@ def extract_value_from_server(server, scope, name) test 'raise an error when @scope is nil' do @client.instance_variable_set(:@scope, nil) assert_raise 'Call `establish` method to get a `scope` before calling this method' do - @client.get(@name) + @client.get(@name).get end end @@ -400,10 +394,8 @@ def extract_value_from_server(server, scope, name) ] ) test 'return an error object' do |(param, expected_error)| - response = @client.get(param) - + response = @client.get(param).get errors = response.errors.first - assert_empty response.data assert_equal expected_error, errors end @@ -411,7 +403,7 @@ def extract_value_from_server(server, scope, name) test 'return an error object and data object' do unknown_name = 'key2' - response = @client.get(@name, unknown_name) + response = @client.get(@name, unknown_name).get data = response.data.first error = response.errors.first @@ -422,8 +414,8 @@ def extract_value_from_server(server, scope, name) assert_equal "`#{@scope}\t#{unknown_name}` doesn't exist in counter", error['message'] end - test 'return a future object when async option is true' do - r = @client.get(@name, options: { async: true }) + test 'return a future object when async call' do + r = @client.get(@name) assert_true r.is_a?(Fluent::Counter::Future) assert_nil r.errors end @@ -436,9 +428,9 @@ def extract_value_from_server(server, scope, name) @key = Fluent::Counter::Store.gen_key(@scope, @name) @init_obj = { name: @name, reset_interval: 5, type: 'numeric' } - @client.init(@init_obj) + @client.init(@init_obj).get @inc_obj = { name: @name, value: 10 } - @client.inc(@inc_obj) + @client.inc(@inc_obj).get end test 'reset a value after `reset_interval` passed' do @@ -450,7 +442,7 @@ def extract_value_from_server(server, scope, name) travel_sec = 6 # greater than reset_interval Timecop.travel(travel_sec) - v2 = @client.reset(@name) + v2 = @client.reset(@name).get data = v2.data.first c = data['counter_data'] @@ -478,7 +470,7 @@ def extract_value_from_server(server, scope, name) travel_sec = 4 # less than reset_interval Timecop.travel(travel_sec) - v2 = @client.reset(@name) + v2 = @client.reset(@name).get data = v2.data.first c = data['counter_data'] @@ -499,7 +491,7 @@ def extract_value_from_server(server, scope, name) test 'raise an error when @scope is nil' do @client.instance_variable_set(:@scope, nil) assert_raise 'Call `establish` method to get a `scope` before calling this method' do - @client.reset(@name) + @client.reset(@name).get end end @@ -514,10 +506,8 @@ def extract_value_from_server(server, scope, name) ] ) test 'return an error object' do |(param, expected_error)| - response = @client.reset(param) - + response = @client.reset(param).get errors = response.errors.first - assert_empty response.data assert_equal expected_error, errors end @@ -528,7 +518,7 @@ def extract_value_from_server(server, scope, name) travel_sec = 6 # greater than reset_interval Timecop.travel(travel_sec) - response = @client.reset(@name, unknown_name) + response = @client.reset(@name, unknown_name).get data = response.data.first error = response.errors.first counter = data['counter_data'] @@ -550,8 +540,8 @@ def extract_value_from_server(server, scope, name) assert_equal (@now + travel_sec), Fluent::EventTime.new(*v1['last_modified_at']) end - test 'return a future object when async option is true' do - r = @client.reset(@name, options: { async: true }) + test 'return a future object when async call' do + r = @client.reset(@name) assert_true r.is_a?(Fluent::Counter::Future) assert_nil r.errors end