Hadoop QA
added a comment - 04/Sep/12 07:00 +1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12543618/hdfs-2793.txt
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 3 new or modified test files.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 eclipse:eclipse. The patch built with eclipse:eclipse.
+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs.
+1 contrib tests. The patch passed contrib unit tests.
Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3140//testReport/
Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3140//console
This message is automatically generated.

Aaron T. Myers
added a comment - 04/Sep/12 21:27 Patch looks pretty good to me. Just a few little comments:
I don't understand why this "if(...)" is necessary:
if (isFromClient) {
checkSuperuserPrivilege();
}
It's certainly the case that arbitrary clients should be super users, but when would a non-super user node be calling this method? I think it should be safe to always check for super user privileges.
Should the rollEdits RPC be marked @Idempotent ? I can't think of a case when calling it twice would be problematic.

Todd Lipcon
added a comment - 04/Sep/12 22:04 It's certainly the case that arbitrary clients should be super users, but when would a non-super user node be calling this method? I think it should be safe to always check for super user privileges.
Is the 2NN's principal always considered a superuser? I think so, but just want to confirm.
Should the rollEdits RPC be marked @Idempotent? I can't think of a case when calling it twice would be problematic.
Makes sense.

Hadoop QA
added a comment - 05/Sep/12 02:29 +1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12543780/hdfs-2793.txt
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 1 new or modified test files.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 eclipse:eclipse. The patch built with eclipse:eclipse.
+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs.
+1 contrib tests. The patch passed contrib unit tests.
Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3146//testReport/
Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3146//console
This message is automatically generated.

Hadoop QA
added a comment - 05/Sep/12 03:09 +1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12543787/hdfs-2793.txt
against trunk revision .
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 1 new or modified test files.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 eclipse:eclipse. The patch built with eclipse:eclipse.
+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs.
+1 contrib tests. The patch passed contrib unit tests.
Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/3147//testReport/
Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/3147//console
This message is automatically generated.