Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Retry writes in IO sink during write error(s).

- Steno::Sink::IO#max_retries limits number of retries and
  defaults to -1 (no retry).
- Added/updated existing tests to suit this change.
- Added missing tests for Steno::Sink::IO.for_file(...) method.

Change-Id: I07cfb998908ab91736a961ba3ec5584ad6e1a6a9
  • Loading branch information...
commit 20032850654bbbc2654da11be8348860cc8d3c6e 1 parent e323dbe
@kowshik kowshik authored
View
9 lib/steno/config.rb
@@ -42,7 +42,9 @@ def to_config_hash(hash)
}
if hash[:file]
- opts[:sinks] << Steno::Sink::IO.for_file(hash[:file])
+ max_retries = hash[:max_retries]
+ opts[:sinks] << Steno::Sink::IO.for_file(hash[:file],
+ :max_retries => max_retries)
end
if hash[:syslog]
@@ -50,7 +52,10 @@ def to_config_hash(hash)
opts[:sinks] << Steno::Sink::Syslog.instance
end
- opts[:sinks] << Steno::Sink::IO.new(STDOUT) if opts[:sinks].empty?
+ if opts[:sinks].empty?
+ opts[:sinks] << Steno::Sink::IO.new(STDOUT)
+ end
+
opts
end
View
44 lib/steno/sink/io.rb
@@ -10,24 +10,36 @@ class << self
# Returns a new sink configured to append to the file at path.
#
# @param [String] path
- # @param [True, False] autoflush If true, encoded records will not be
- # buffered by Ruby.
- #
+ # @param [Hash] If the key :autoflush is set to true, encoded records
+ # will not be buffered by Ruby. The key :max_retries
+ # is forwarded to Steno::Sink::IO object during creation.
# @return [Steno::Sink::IO]
- def for_file(path, autoflush = true)
+ def for_file(path, opts = {})
+ autoflush = true
+ if opts.include?(:autoflush)
+ autoflush = opts[:autoflush]
+ end
+
io = File.open(path, "a+")
io.sync = autoflush
- new(io)
+ new(io, :max_retries => opts[:max_retries])
end
end
- # @param [IO] io The IO object that will be written to
- # @param [Steno::Codec::Base] codec
- def initialize(io, codec = nil)
- super(codec)
+ attr_reader :max_retries
+ # @param [IO] io The IO object that will be written to
+ # @param [Hash] opts Key :codec is used to specify a codec inheriting from
+ # Steno::Codec::Base.
+ # Key :max_retries takes an integer value which specifies
+ # the number of times the write operation can be retried
+ # when IOError is raised while writing a record.
+ def initialize(io, opts = {})
+ super(opts[:codec])
+
+ @max_retries = opts[:max_retries] || -1
@io_lock = Mutex.new
@io = io
end
@@ -35,7 +47,19 @@ def initialize(io, codec = nil)
def add_record(record)
bytes = @codec.encode_record(record)
- @io_lock.synchronize { @io.write(bytes) }
+ @io_lock.synchronize do
+ retries = 0
+ begin
+ @io.write(bytes)
+ rescue IOError => e
+ if retries < @max_retries
+ retries += 1
+ retry
+ else
+ raise e
+ end
+ end
+ end
nil
end
View
2  lib/steno/version.rb
@@ -1,3 +1,3 @@
module Steno
- VERSION = "0.0.20"
+ VERSION = "1.0.0"
end
View
26 spec/unit/config_spec.rb
@@ -10,7 +10,8 @@
@mock_sink_file = mock("sink")
@mock_sink_file.stub(:codec=)
- Steno::Sink::IO.should_receive(:for_file).with(@log_path)
+ Steno::Sink::IO.should_receive(:for_file).with(@log_path,
+ :max_retries => 5)
.and_return(@mock_sink_file)
@mock_sink_syslog = mock("sink")
@@ -36,7 +37,8 @@
:file => @log_path,
:level => "debug2",
:default_log_level => "warn",
- :syslog => "test"
+ :syslog => "test",
+ :max_retries => 5,
}
end
@@ -45,7 +47,8 @@
"file" => @log_path,
"level" => "debug2",
"default_log_level" => "warn",
- "syslog" => "test"
+ "syslog" => "test",
+ "max_retries" => 5,
}
end
end
@@ -90,11 +93,12 @@
end
it "should add a file sink if the 'file' key is specified" do
- write_config(@config_path, { "file" => @log_path })
+ write_config(@config_path, { "file" => @log_path, "max_retries" => 2 })
mock_sink = mock("sink")
mock_sink.stub(:codec=)
- Steno::Sink::IO.should_receive(:for_file).with(@log_path).and_return(mock_sink)
+ Steno::Sink::IO.should_receive(:for_file).
+ with(@log_path, :max_retries => 2).and_return(mock_sink)
config = Steno::Config.from_file(@config_path)
config.sinks.size.should == 1
config.sinks[0].should == mock_sink
@@ -113,6 +117,18 @@
config.sinks[0].should == mock_sink
end
+ it "should add an io sink to stdout if no sinks are explicitly specified in the config file" do
+ write_config(@config_path, {})
+ mock_sink = mock("sink")
+ mock_sink.stub(:codec=)
+
+ Steno::Sink::IO.should_receive(:new).with(STDOUT).and_return(mock_sink)
+
+ config = Steno::Config.from_file(@config_path)
+ config.sinks.size.should == 1
+ config.sinks[0].should == mock_sink
+ end
+
it "should merge supplied overrides with the file based config" do
write_config(@config_path, { "default_log_level" => "debug" })
View
65 spec/unit/sink/io_spec.rb
@@ -9,6 +9,41 @@
Steno::Record.new("source", level, "message")
end
+ describe ".for_file" do
+ it "should return a new sink configured to append to the file at path with autosync set to true by default" do
+ mock_handle = mock("file handle")
+
+ File.should_receive(:open).with("path", "a+").and_return(mock_handle)
+ mock_handle.should_receive(:sync=).with(true)
+
+ mock_sink = mock("sink")
+ Steno::Sink::IO.should_receive(:new).with(mock_handle,
+ :max_retries => 10).
+ and_return(mock_sink)
+
+ returned = Steno::Sink::IO.for_file("path",
+ :max_retries => 10)
+ returned.should == mock_sink
+ end
+
+ it "should return a new sink configured to append to the file at path with specified options" do
+ mock_handle = mock("file handle")
+
+ File.should_receive(:open).with("path", "a+").and_return(mock_handle)
+ mock_handle.should_receive(:sync=).with(false)
+
+ mock_sink = mock("sink")
+ Steno::Sink::IO.should_receive(:new).with(mock_handle,
+ :max_retries => 10).
+ and_return(mock_sink)
+
+ returned = Steno::Sink::IO.for_file("path",
+ :autoflush => false,
+ :max_retries => 10)
+ returned.should == mock_sink
+ end
+ end
+
describe "#add_record" do
it "should encode the record and write it to the underlying io object" do
codec = mock("codec")
@@ -17,7 +52,35 @@
io = mock("io")
io.should_receive(:write).with(record.message)
- Steno::Sink::IO.new(io, codec).add_record(record)
+ Steno::Sink::IO.new(io, :codec => codec).add_record(record)
+ end
+
+ it "should by default not retry on IOError" do
+ codec = mock("codec")
+ codec.should_receive(:encode_record).with(record).and_return(record.message)
+
+ io = mock("io")
+
+ io.should_receive(:write).with(record.message).ordered.and_raise(IOError)
+
+ expect {
+ Steno::Sink::IO.new(io, :codec => codec).add_record(record)
+ }.to raise_error(IOError)
+ end
+
+ it "should retry not more than specified number of times on IOError" do
+ codec = mock("codec")
+ codec.should_receive(:encode_record).with(record).and_return(record.message)
+
+ io = mock("io")
+
+ io.should_receive(:write).exactly(3).times.with(record.message).ordered.
+ and_raise(IOError)
+
+ expect {
+ Steno::Sink::IO.new(io, :codec => codec, :max_retries => 2).
+ add_record(record)
+ }.to raise_error(IOError)
end
end
Please sign in to comment.
Something went wrong with that request. Please try again.