debugrubyMinor
Need reviews for impacts of dirty bug fix
Viewed 0 times
fixbugreviewsneedforimpactsdirty
Problem
I was having a hard time with bug logstash#665. Hope they will fix it some day, but sparc/solaris support is probably not on top of their priority list. For further reference, original error is:
After digging a lot I found out that the problem here is the usage of a native library which is not correclty retrieved on Solaris. So when ruby fails, it is trying a fallback to java, which fails as well. Now I have retrieved the original watch.rb source file and isolated the problematic function:
More precisely, the bug was happening there:
```
stat = File::Stat.new(file)
@files[file] = {
:size => 0,
:inode => [stat.ino, stat.dev_major, stat.dev_
NotImplementedError: stat.st_dev unsupported or native support failed to load dev_major at org/jruby/RubyFileStat.java:190
_discover_file at /tmp/aaaa/logstash-1.4.1.dev/vendor/bundle/jruby/1.9/gems/filewatch-0.5.1/lib/filewatch/watch.rb:140After digging a lot I found out that the problem here is the usage of a native library which is not correclty retrieved on Solaris. So when ruby fails, it is trying a fallback to java, which fails as well. Now I have retrieved the original watch.rb source file and isolated the problematic function:
private
def _discover_file(path, initial=false)
globbed_dirs = Dir.glob(path)
@logger.debug("_discover_file_glob: #{path}: glob is: #{globbed_dirs}")
if globbed_dirs.empty? && File.file?(path)
globbed_dirs = [path]
@logger.debug("_discover_file_glob: #{path}: glob is: #{globbed_dirs} because glob did not work")
end
globbed_dirs.each do |file|
next if @files.member?(file)
next unless File.file?(file)
@logger.debug("_discover_file: #{path}: new: #{file} (exclude is #{@exclude.inspect})")
skip = false
@exclude.each do |pattern|
if File.fnmatch?(pattern, File.basename(file))
@logger.debug("_discover_file: #{file}: skipping because it " +
"matches exclude #{pattern}")
skip = true
break
end
end
next if skip
stat = File::Stat.new(file)
@files[file] = {
:size => 0,
:inode => [stat.ino, stat.dev_major, stat.dev_minor],
:create_sent => false,
}
if initial
@files[file][:initial] = true
end
end
end # def _discover_fileMore precisely, the bug was happening there:
```
stat = File::Stat.new(file)
@files[file] = {
:size => 0,
:inode => [stat.ino, stat.dev_major, stat.dev_
Solution
In Unix filesystems, an inode is a number that uniquely identifies a file within the filesystem. (More strictly speaking, an inode is a data structure that holds metadata about a file within the filesystem; each inode is assigned an inode number, and the number is all you care about unless you're a kernel developer.)
However, the inode number only uniquely identifies a file within its filesystem. If
By ignoring the device number, you might end up acting on events on a file with a coincidentally equal inode number on a different filesystem — though the chance is miniscule.
If you decide to keep this workaround, then I think you should make a corresponding change on line 71 as well.
However, the inode number only uniquely identifies a file within its filesystem. If
/tmp and /usr are separate filesystems, inode 3461 of the /tmp filesystem might be /tmp/.ICE-unix while inode 3461 on /usr might be /usr/share/dict/words. Therefore, to uniquely identify a file on the whole computer, a tuple of (inode number, device major number, device minor number) is needed.By ignoring the device number, you might end up acting on events on a file with a coincidentally equal inode number on a different filesystem — though the chance is miniscule.
If you decide to keep this workaround, then I think you should make a corresponding change on line 71 as well.
Context
StackExchange Code Review Q#46922, answer score: 4
Revisions (0)
No revisions yet.