ayushtkn commented on a change in pull request #3705:
URL: https://github.com/apache/hadoop/pull/3705#discussion_r762392267
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java
##########
@@ -1100,20 +1100,29 @@ public void rename2(String src, String dst,
Options.Rename... options)
}
@Override // ClientProtocol
- public boolean truncate(String src, long newLength, String clientName)
- throws IOException {
+ public boolean truncate(String src, long newLength, String clientName)
throws IOException {
Review comment:
nit: only formatting change and unrelated, Please avoid
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java
##########
@@ -1100,20 +1100,29 @@ public void rename2(String src, String dst,
Options.Rename... options)
}
@Override // ClientProtocol
- public boolean truncate(String src, long newLength, String clientName)
- throws IOException {
+ public boolean truncate(String src, long newLength, String clientName)
throws IOException {
checkNNStartup();
- stateChangeLog
- .debug("*DIR* NameNode.truncate: " + src + " to " + newLength);
+ if(stateChangeLog.isDebugEnabled()) {
+ stateChangeLog.debug("*DIR* NameNode.truncate: " + src + " to " +
+ newLength);
+ }
+ CacheEntryWithPayload cacheEntry =
RetryCache.waitForCompletion(retryCache, null);
+ if (cacheEntry != null && cacheEntry.isSuccess()) {
+ return (boolean)cacheEntry.getPayload();
+ }
+
String clientMachine = getClientMachine();
+ boolean ret = false;
try {
- return namesystem.truncate(
+ ret = namesystem.truncate(
src, newLength, clientName, clientMachine, now());
} finally {
+ RetryCache.setState(cacheEntry, true, ret);
Review comment:
Finally block will be executed in case of exception as well, we can not
hard-code `true` here.
Can check other codes like for `append` to get some reference & idea.
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java
##########
@@ -1100,20 +1100,29 @@ public void rename2(String src, String dst,
Options.Rename... options)
}
@Override // ClientProtocol
- public boolean truncate(String src, long newLength, String clientName)
- throws IOException {
+ public boolean truncate(String src, long newLength, String clientName)
throws IOException {
checkNNStartup();
- stateChangeLog
- .debug("*DIR* NameNode.truncate: " + src + " to " + newLength);
+ if(stateChangeLog.isDebugEnabled()) {
+ stateChangeLog.debug("*DIR* NameNode.truncate: " + src + " to " +
+ newLength);
+ }
+ CacheEntryWithPayload cacheEntry =
RetryCache.waitForCompletion(retryCache, null);
Review comment:
I think we should have
``
namesystem.checkOperation(OperationCategory.WRITE);
``
above the this like other calls and remove this check before lock in
FsNamesystem.
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NameNodeRpcServer.java
##########
@@ -1100,20 +1100,29 @@ public void rename2(String src, String dst,
Options.Rename... options)
}
@Override // ClientProtocol
- public boolean truncate(String src, long newLength, String clientName)
- throws IOException {
+ public boolean truncate(String src, long newLength, String clientName)
throws IOException {
checkNNStartup();
- stateChangeLog
- .debug("*DIR* NameNode.truncate: " + src + " to " + newLength);
+ if(stateChangeLog.isDebugEnabled()) {
+ stateChangeLog.debug("*DIR* NameNode.truncate: " + src + " to " +
+ newLength);
+ }
+ CacheEntryWithPayload cacheEntry =
RetryCache.waitForCompletion(retryCache, null);
+ if (cacheEntry != null && cacheEntry.isSuccess()) {
+ return (boolean)cacheEntry.getPayload();
+ }
+
String clientMachine = getClientMachine();
+ boolean ret = false;
try {
- return namesystem.truncate(
+ ret = namesystem.truncate(
src, newLength, clientName, clientMachine, now());
} finally {
+ RetryCache.setState(cacheEntry, true, ret);
metrics.incrFilesTruncated();
}
+ return ret;
}
-
+
Review comment:
nit:
unrelated change, revert!!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
