Skip to content

#189 Zero byte read from NFS - Fix method file_read in watched_file.rb #190

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

#189 Zero byte read from NFS - Fix method file_read in watched_file.rb #190

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions lib/filewatch/watched_file.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,27 @@ def file_seek(amount, whence = IO::SEEK_SET)
end

def file_read(amount)
set_accessed_at
@file.sysread(amount)
#debug "*** file_read #{path}"
cc = 3 # max attempts
dt = 1.0/128 # delay beetwen attempts
loop do
set_accessed_at
buf = @file.sysread(amount)
# return if no zero byte inside
return buf unless zc = buf.index("\0")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The zc variable is bound, but unused.

Additionally, for readability we prefer that variables are descriptive and unabiguous, and while I can tell what buf is, variables like cc and dt would be better off with more descriptive names like attempts_remaining and backoff_delay.

# update amount to read
amount = buf.bytesize
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

amount here has multiple meanings (amount requested vs amount retrieved), and the mutation of the input parameter combined with the retry pattern means that on each subsequent attempt we will read potentially fewer bytes than initially requested. This is rather opaque and can present surprises later on that would be hard to debug, so we try to avoid overloading variables like this.

# warn "*** ZERO-#{cc} byte (#{zc} of #{amount}) in #{path}"
cc -= 1
# was last attempt - returning buffer with zero byte
return buf if cc.zero?
# sllep before next read
sleep(dt)
# increase sleep time
dt *= 4
# get back in file
@file.sysseek(-amount, IO::SEEK_CUR)
end
end

def file_open?
Expand Down