[
https://issues.apache.org/jira/browse/HDFS-3981?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Konstantin Shvachko updated HDFS-3981:
--------------------------------------
Description: Incorrect condition in
{{FSNamesystem.getBlockLocatoins()}} can lead to updating times without write
lock. In most cases this condition will force
{{FSNamesystem.getBlockLocatoins()}} to hold write lock, even if times do not
need to be updated. (was: If now > inode.getAccessTime() +
getAccessTimePrecision() when attempt == 0, we will call dir.setTimes(src,
inode, -1, now, false) without writelock. So there will be races and an ealier
access time might overwrite a later access time. Looks like we need to change
the code to
{code}
if (doAccessTime && isAccessTimeSupported()) {
if (now > inode.getAccessTime() + getAccessTimePrecision()) {
// if we have to set access time but we only have the readlock, then
// restart this entire operation with the writeLock.
if (attempt == 0) {
continue;
}
dir.setTimes(src, inode, -1, now, false);
}
}
{code}
Also, seems we need to release readlock before trying to acquire writelock.
Otherwise, we might end up with still holding readlock after the function call.
Or we could simply remove the condition "if (attempt == 0)" for readUnlock(),
i.e. readUnlock() should be called even if attempt is 1.
The following code is from branch-2.0.1-alpha
{code:title=FSNamesystem.java|borderStyle=solid}
private LocatedBlocks getBlockLocationsUpdateTimes(String src,
long offset,
long length,
boolean doAccessTime,
boolean needBlockToken)
throws FileNotFoundException, UnresolvedLinkException, IOException {
for (int attempt = 0; attempt < 2; attempt++) {
if (attempt == 0) { // first attempt is with readlock
readLock();
} else { // second attempt is with write lock
writeLock(); // writelock is needed to set accesstime
}
try {
checkOperation(OperationCategory.READ);
// if the namenode is in safemode, then do not update access time
if (isInSafeMode()) {
doAccessTime = false;
}
long now = now();
INodeFile inode = dir.getFileINode(src);
if (inode == null) {
throw new FileNotFoundException("File does not exist: " + src);
}
assert !inode.isLink();
if (doAccessTime && isAccessTimeSupported()) {
if (now <= inode.getAccessTime() + getAccessTimePrecision()) {
// if we have to set access time but we only have the readlock, then
// restart this entire operation with the writeLock.
if (attempt == 0) {
continue;
}
}
dir.setTimes(src, inode, -1, now, false);
}
return blockManager.createLocatedBlocks(inode.getBlocks(),
inode.computeFileSize(false), inode.isUnderConstruction(),
offset, length, needBlockToken);
} finally {
if (attempt == 0) {
readUnlock();
} else {
writeUnlock();
}
}
}
return null; // can never reach here
}
{code})
Target Version/s: 0.23.4
Affects Version/s: (was: 2.0.1-alpha)
0.23.3
> access time is set without holding writelock in FSNamesystem
> ------------------------------------------------------------
>
> Key: HDFS-3981
> URL: https://issues.apache.org/jira/browse/HDFS-3981
> Project: Hadoop HDFS
> Issue Type: Bug
> Components: name-node
> Affects Versions: 0.23.3
> Reporter: Xiaobo Peng
> Assignee: Xiaobo Peng
> Priority: Minor
>
> Incorrect condition in {{FSNamesystem.getBlockLocatoins()}} can lead to
> updating times without write lock. In most cases this condition will force
> {{FSNamesystem.getBlockLocatoins()}} to hold write lock, even if times do not
> need to be updated.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
