[PATCH 0/6] misc fixes and features

[PATCH 0/6] misc fixes and features

[Eric Wong]

I mainly wanted to implement :connect_timeout, but ended up with
a bunch of other small improvements along the way.  Will release
soon since it's been a while.

Eric Wong (6):
  test_fresh: do not delete non-existent domain
  admin: map unset reject_bad_md5 field to nil
  socket/pure_ruby: connect with "exception:false" on Ruby 2.3+
  implement :connect_timeout option
  add .gitattributes for Ruby method detection
  README: stop mentioning cgit

 .gitattributes                   |  4 ++++
 README                           |  2 +-
 lib/mogilefs/admin.rb            |  2 ++
 lib/mogilefs/backend.rb          |  3 ++-
 lib/mogilefs/client.rb           |  4 +++-
 lib/mogilefs/mogilefs.rb         |  4 ++++
 lib/mogilefs/socket/pure_ruby.rb | 20 ++++++++++++++------
 test/fresh.rb                    |  2 +-
 test/test_fresh.rb               |  2 +-
 9 files changed, 32 insertions(+), 11 deletions(-)
 create mode 100644 .gitattributes

-- 
EW

[PATCH 1/6] test_fresh: do not delete non-existent domain

[Eric Wong]

This test bug was exposed due to using an old
MogileFS-Server without
commit d0ee2a27253ade4b50ef5b91471f4f520a67bf8c
("Work with DBD::SQLite's latest lock errors")
---
 test/test_fresh.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/test_fresh.rb b/test/test_fresh.rb
index 9499db5..ff7bae6 100644
--- a/test/test_fresh.rb
+++ b/test/test_fresh.rb
@@ -88,7 +88,7 @@ def test_new_file_info_checksum
     end
     test_new_file_info(:md5)
   ensure
-    @admin.delete_class @domain, "check"
+    @admin.delete_class(@domain, "check") if @domain
   end
 
   def test_create_open_close_opts
-- 
EW

[PATCH 2/6] admin: map unset reject_bad_md5 field to nil

[Eric Wong]

The 'reject_bad_md5' field may be of an unknown value
before the monitor is ready, so prepare for it and map
it to 'nil' instead of an empty string.  This hopefully
makes the Ruby API more Ruby-ish.
---
 lib/mogilefs/admin.rb | 2 ++
 test/fresh.rb         | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/mogilefs/admin.rb b/lib/mogilefs/admin.rb
index 289dea6..96a7898 100644
--- a/lib/mogilefs/admin.rb
+++ b/lib/mogilefs/admin.rb
@@ -82,6 +82,8 @@ def get_devices(devid = nil)
         row["reject_bad_md5"] = true
       when "0"
         row["reject_bad_md5"] = false
+      when ""
+        row["reject_bad_md5"] = nil
       end
     end
   end
diff --git a/test/fresh.rb b/test/fresh.rb
index 8d452c7..4c53ac2 100644
--- a/test/fresh.rb
+++ b/test/fresh.rb
@@ -82,7 +82,7 @@ def add_host_device_domain
     # MogileFS::Server 2.60+ shows reject_bad_md5 monitor status
     dev = @admin.get_devices[0]
     if dev.include?("reject_bad_md5")
-      assert [true, false].include?(dev["reject_bad_md5"])
+      assert [true, false, nil].include?(dev["reject_bad_md5"]), dev.inspect
     end
 
     out = err = nil
-- 
EW

[PATCH 3/6] socket/pure_ruby: connect with "exception:false" on Ruby 2.3+

[Eric Wong]

Exceptions are expensive for common errors, so avoid raising
them under Ruby 2.3+ which allows connect_nonblock to be
called with "exception: false" kwarg.
---
 lib/mogilefs/socket/pure_ruby.rb | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/lib/mogilefs/socket/pure_ruby.rb b/lib/mogilefs/socket/pure_ruby.rb
index d57a7c0..59e43e1 100644
--- a/lib/mogilefs/socket/pure_ruby.rb
+++ b/lib/mogilefs/socket/pure_ruby.rb
@@ -5,13 +5,21 @@
 class MogileFS::Socket < Socket
   include MogileFS::SocketCommon
 
-  def self.start(host, port)
-    sock = new(Socket::AF_INET, Socket::SOCK_STREAM, 0)
-    begin
-      sock.connect_nonblock(sockaddr_in(port, host))
-    rescue Errno::EINPROGRESS
+  if RUBY_VERSION.to_f >= 2.3
+    def self.start(host, port)
+      sock = new(Socket::AF_INET, Socket::SOCK_STREAM, 0)
+      sock.connect_nonblock(sockaddr_in(port, host), :exception => false)
+      sock.post_init(host, port)
+    end
+  else
+    def self.start(host, port)
+      sock = new(Socket::AF_INET, Socket::SOCK_STREAM, 0)
+      begin
+        sock.connect_nonblock(sockaddr_in(port, host))
+      rescue Errno::EINPROGRESS
+      end
+      sock.post_init(host, port)
     end
-    sock.post_init(host, port)
   end
 
   def self.tcp(host, port, timeout = 5)
-- 
EW

[PATCH 4/6] implement :connect_timeout option

[Eric Wong]

This can be useful for specifying a different timeout for
establishing a connection.  Some requests could be expensive and
want a higher :timeout measured in seconds, while the time to
establish a TCP connection on a healthy LAN could be less than
a millisecond.

This defaults to 3s to match the existing :timeout, but only
affects the amount of time the client will wait for establishing
a TCP connection to a tracker.
---
 lib/mogilefs/backend.rb  | 3 ++-
 lib/mogilefs/client.rb   | 4 +++-
 lib/mogilefs/mogilefs.rb | 4 ++++
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/lib/mogilefs/backend.rb b/lib/mogilefs/backend.rb
index 632d6f1..4cf2526 100644
--- a/lib/mogilefs/backend.rb
+++ b/lib/mogilefs/backend.rb
@@ -79,6 +79,7 @@ def initialize(args)
 
     @mutex = Mutex.new
     @timeout = args[:timeout] || 3
+    @connect_timeout = args[:connect_timeout] || 3
     @socket = nil
     @lasterr = nil
     @lasterrstr = nil
@@ -347,7 +348,7 @@ def socket
 
       begin
         addr, port = host.split(':'.freeze)
-        @socket = MogileFS::Socket.tcp(addr, port, @timeout)
+        @socket = MogileFS::Socket.tcp(addr, port, @connect_timeout)
         @active_host = host
       rescue SystemCallError, MogileFS::Timeout => err
         @dead[host] = [ MogileFS.now, err ]
diff --git a/lib/mogilefs/client.rb b/lib/mogilefs/client.rb
index 7a808f1..1e30874 100644
--- a/lib/mogilefs/client.rb
+++ b/lib/mogilefs/client.rb
@@ -27,6 +27,7 @@ def initialize(args)
     @readonly = args[:readonly] ? true : false
     @timeout = args[:timeout]
     @fail_timeout = args[:fail_timeout]
+    @connect_timeout = args[:connect_timeout]
 
     reload
   end
@@ -37,7 +38,8 @@ def initialize(args)
   def reload
     @backend = MogileFS::Backend.new(:hosts => @hosts,
                                      :timeout => @timeout,
-                                     :fail_timeout => @fail_timeout)
+                                     :fail_timeout => @fail_timeout,
+                                     :connect_timeout => @connect_timeout)
   end
 
   ##
diff --git a/lib/mogilefs/mogilefs.rb b/lib/mogilefs/mogilefs.rb
index f454a0f..d17f0b8 100644
--- a/lib/mogilefs/mogilefs.rb
+++ b/lib/mogilefs/mogilefs.rb
@@ -64,6 +64,10 @@ class MogileFS::MogileFS < MogileFS::Client
   #   Timeout for tracker backend responses.
   #   Defaults to 3 seconds.
   #
+  # [:connect_timeout => Integer]
+  #
+  #   Timeout for connecting to a tracker
+  #   Defaults to 3 seconds
   def initialize(args = {})
     @domain = args[:domain]
 
-- 
EW

[PATCH 5/6] add .gitattributes for Ruby method detection

[Eric Wong]

The "diff" function detection for C does not map well to
Ruby files, take advantage of gitattributes(5) to improve
method name detection in generated patches as well as
making "git diff -W" output more useful.
---
 .gitattributes | 4 ++++
 1 file changed, 4 insertions(+)
 create mode 100644 .gitattributes

diff --git a/.gitattributes b/.gitattributes
new file mode 100644
index 0000000..cfedc1f
--- /dev/null
+++ b/.gitattributes
@@ -0,0 +1,4 @@
+*.gemspec diff=ruby
+*.rb diff=ruby
+Rakefile diff=ruby
+bin/* diff=ruby
-- 
EW

[PATCH 6/6] README: stop mentioning cgit

[Eric Wong]

It is unlikely I will be using cgit in the future since
it has CSS and lacks mailing list integration.
---
 README | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/README b/README
index cc83816..fa37814 100644
--- a/README
+++ b/README
@@ -14,7 +14,7 @@ list-cc :: mailto:mogile@googlegroups.com
 list-archive :: http://bogomips.org/mogilefs-client-public
 email :: mailto:e@80x24.org
 repo :: git://bogomips.org/mogilefs-client.git
-cgit :: http://bogomips.org/mogilefs-client.git
+	http://bogomips.org/mogilefs-client.git
 gitweb :: http://repo.or.cz/w/ruby-mogilefs-client.git
 download :: http://bogomips.org/mogilefs-client/files/
 
-- 
EW

[7/6 PATCH] connect_timeout: match :timeout if unset

[Eric Wong]

And add some weak tests while we're at it.
Actually simulating a connection timeout on localhost
will be difficult.
---
 lib/mogilefs/backend.rb |  2 +-
 test/test_backend.rb    | 17 ++++++++++++++++-
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/lib/mogilefs/backend.rb b/lib/mogilefs/backend.rb
index 4cf2526..7f91508 100644
--- a/lib/mogilefs/backend.rb
+++ b/lib/mogilefs/backend.rb
@@ -79,7 +79,7 @@ def initialize(args)
 
     @mutex = Mutex.new
     @timeout = args[:timeout] || 3
-    @connect_timeout = args[:connect_timeout] || 3
+    @connect_timeout = args[:connect_timeout] || @timeout
     @socket = nil
     @lasterr = nil
     @lasterrstr = nil
diff --git a/test/test_backend.rb b/test/test_backend.rb
index 2078e9b..4d41354 100644
--- a/test/test_backend.rb
+++ b/test/test_backend.rb
@@ -25,12 +25,14 @@ def test_initialize
 
     assert_equal ['localhost:1'], @backend.hosts
     assert_equal 3, @backend.timeout
+    assert_equal 3, @backend.instance_variable_get(:@connect_timeout)
     assert_equal nil, @backend.lasterr
     assert_equal nil, @backend.lasterrstr
     assert_equal({}, @backend.dead)
 
     @backend = MogileFS::Backend.new :hosts => ['localhost:6001'], :timeout => 1
     assert_equal 1, @backend.timeout
+    assert_equal 1, @backend.instance_variable_get(:@connect_timeout)
   end
 
   def test_do_request
@@ -223,5 +225,18 @@ def test_fail_timeout
     c = MogileFS::MogileFS.new(o)
     assert_equal 0.666, c.backend.instance_variable_get(:@fail_timeout)
   end
-end
 
+  def test_connect_timeout
+    o = {
+      :domain => "none",
+      :hosts => %w(0:666 0:6 0:66),
+      :connect_timeout => 1
+    }
+    c = MogileFS::MogileFS.new(o)
+    assert_equal 1, c.backend.instance_variable_get(:@connect_timeout)
+    o[:timeout] = 5
+    c = MogileFS::MogileFS.new(o)
+    assert_equal 1, c.backend.instance_variable_get(:@connect_timeout)
+    assert_equal 5, c.backend.instance_variable_get(:@timeout)
+  end
+end
-- 
EW