[
https://issues.apache.org/jira/browse/HDFS-3608?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13417631#comment-13417631
]
Aaron T. Myers commented on HDFS-3608:
--------------------------------------
Patch looks pretty good to me, Colin, though I haven't tested it manually yet.
Can you comment on what manual testing of the patch you've done?
A few comments:
# I find the "used" member of the hdfsConn struct to be a little confusing with
regards to its unit. It's not really "representing how recently this connection
has been used," as the comment says, but is really "the number of times this
connection has been checked by a timer callback while there were no users of
the connection." I think it'd be better if it were just a value representing
the last time the connection was used, and if it were renamed "lastUsedTime".
If you want to keep the current semantics you have, you should rename this
member to something like "expiryChecksRemaining".
# Looks like there's a tab character in hdfsConnExpiry, on the RB_FOREACH_SAFE
line.
# In an error message in hdfsConnCheckKpath, you mention that a call to
"utimes" failed, when in fact it was a call to "stat"
# In hdfsConnCheckKpath, you always return "1" in the event of error, even if
you have a more meaningful errno number stored in "ret". Is that intentional?
You might want to add an @return annotation to hdfsConnCheckKpath regardless,
to explain what it's supposed to return.
# Could use function comments for hdfsConnFind, hdfsConnFree, printTime,
hdfsConnCompare, etc.
# I again see some places where we return negative numbers for errors, and
other places positive numbers.
# In hdfsConnRelease, I recommend you add the username associated with the
connection to the log message you're printing.
# There's a few places in fuseNewConnect where you just print "OOM". Might want
to put some indication of which allocation caused the OOM.
# I don't understand why you call fuseTimerAdd every time in fuseNewConnect. I
don't think it's a bug, since fuseTimerAdd exits early if the timer is already
active, but it seems like fuseTimerAdd should just be called once on startup?
This point seems supported by the fact that you never call fuseTimerRemove.
# Given that there's only ever one timer, it doesn't seem like there's a need
to put it in an RB tree, or for that matter having a generic timer interface at
all. Let's scrap fuse_timer.c, and just make a thread that wakes up
periodically and runs hdfsConnExpiry in fuse_connect.c.
> fuse_dfs: detect changes in UID ticket cache
> --------------------------------------------
>
> Key: HDFS-3608
> URL: https://issues.apache.org/jira/browse/HDFS-3608
> Project: Hadoop HDFS
> Issue Type: Bug
> Affects Versions: 2.1.0-alpha
> Reporter: Colin Patrick McCabe
> Assignee: Colin Patrick McCabe
> Priority: Minor
> Attachments: HDFS-3608.004.patch, HDFS-3608.006.patch
>
>
> Currently in fuse_dfs, if one kinits as some principal "foo" and then does
> some operation on fuse_dfs, then kdestroy and kinit as some principal "bar",
> subsequent operations done via fuse_dfs will still use cached credentials for
> "foo". The reason for this is that fuse_dfs caches Filesystem instances using
> the UID of the user running the command as the key into the cache. This is a
> very uncommon scenario, since it's pretty uncommon for a single user to want
> to use credentials for several different principals on the same box.
> However, we can use inotify to detect changes in the Kerberos ticket cache
> file and force the next operation to create a new FileSystem instance in that
> case. This will also require a reference counting mechanism in fuse_dfs so
> that we can free the FileSystem classes when they refer to previous Kerberos
> ticket caches.
> Another mechanism is to run a stat periodically on the ticket cache file.
> This is a good fallback mechanism if inotify does not work on the file (for
> example, because it's on an NFS mount.)
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
