update jets3t dependency.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12586387/HADOOP-9623.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no new tests are needed for this patch.
Also please list what manual steps were performed to verify this patch.

-1 javac. The applied patch generated 1172 javac compiler warnings (more than the trunk's current 1156 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-common-project/hadoop-common.

+1 contrib tests. The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/2603//testReport/
Javac warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/2603//artifact/trunk/patchprocess/diffJavacWarnings.txt
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/2603//console

This message is automatically generated.

This looks the same as or a branch 2.x version of HADOOP-8136. This is something we need to do -it's just I'm too overloaded to do it right now. I'd like to get it into the 2.1-beta2 though

This patch is ready for review. Recommend taking latest(0.9.0) at this point, as 2.X stabilizes.

Hi Steve Loughran, do you know of anyone else who could review?

Overall it looks fine. Do Jets3tNativeS3FileSystemContractTest and Jets3tS3FileSystemContractTest pass with this change? They are live tests run against the real S3 service and are not run by default. You'll need to put S3 credentials into hadoop-common-project/hadoop-common/src/test/resources/core-site.xml to run them.

Nit: catch blocks should be on the same line as the preceding brace.

Hi Tom -

Thanks for the feedback, I will try to scrounge up some time to do the tests and report back the results.

Cheers,
Tim

Canceling in favor of: https://issues.apache.org/jira/browse/HADOOP-9680

Ran into some dependency issues with httpcore with the existing patch. Fixing that and adding some exception handling and logging.

I did the following testing:
1. distcp files of 1G, 5G and 10G
a) hdfs -> s3
b) s3 -> hdfs
2. Teragen to create a 5G file in s3 directly

These were all done from a single node hadoop deployment.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12608135/HADOOP-9623.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no new tests are needed for this patch.
Also please list what manual steps were performed to verify this patch.

-1 javac. The applied patch generated 1532 javac compiler warnings (more than the trunk's current 1525 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-common-project/hadoop-common.

+1 contrib tests. The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3210//testReport/
Javac warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/3210//artifact/trunk/patchprocess/diffJavacWarnings.txt
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3210//console

This message is automatically generated.

Fixing for javac warnings related to using deprecated API from jets3t

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12608242/HADOOP-9623_2.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no new tests are needed for this patch.
Also please list what manual steps were performed to verify this patch.

+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-common-project/hadoop-common.

+1 contrib tests. The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3211//testReport/
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3211//console

This message is automatically generated.

Looks straightforward in general.

+    } catch (ServiceException e) {
+      return null;
     }
   }
 
@@ -194,6 +197,8 @@ private InputStream get(String key, long byteRangeStart) throws IOException {
         throw (IOException) e.getCause();
       }
       throw new S3Exception(e);
+    } catch (ServiceException e) {
+      return null;
     }
   }
 

Why are these exceptions ignored? Should they be logged? Or could we at least check the exception text before doing this?

The hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/s3/Jets3tFileSystemStore.java file, where you saw those is a piece of the code that's not recommended or used. It's for the S3 interface, not S3N, and there have been conversations about deprecating it (no jira filed yet afaik though). I didn't pay much attention to that code base. Just made sure that no API conflict happened and that it compiled.

Thinking about this more. I'll log the exceptions and do some smoke tests on the s3 connector too. Already done that for the S3N connector. Creating jira for deprecating the S3 connector in a future release.

If there are no alternate suggestions, I think it would be preferable to log these exceptions in Jets3tFileSystemStore.java. We can always change it later if it becomes too verbose.

Let's also make sure that we pay some attention to the (non-S3N) S3 code base as well. It may be removed in the future, but it hasn't been yet.

Incorporating Colin's comments. Did some smoke tests with the s3 interface as well.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12612324/HADOOP-9623_3.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no new tests are needed for this patch.
Also please list what manual steps were performed to verify this patch.

+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-common-project/hadoop-common.

+1 contrib tests. The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/3267//testReport/
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/3267//console

This message is automatically generated.

+1, thanks Amandeep

SUCCESS: Integrated in Hadoop-trunk-Commit #4697 (See https://builds.apache.org/job/Hadoop-trunk-Commit/4697/)
move HADOOP-9623 to 2.2.1 (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1539255)

• /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
  HADOOP-9623. Update jet3t dependency to 0.9.0 (Amandeep Khurana via Colin Patrick McCabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1539253)
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/s3/Jets3tFileSystemStore.java
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/s3/MigrationTool.java
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/s3native/Jets3tNativeFileSystemStore.java
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/s3native/NativeS3FileSystem.java
• /hadoop/common/trunk/hadoop-project/pom.xml

SUCCESS: Integrated in Hadoop-Yarn-trunk #384 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/384/)
move HADOOP-9623 to 2.2.1 (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1539255)

• /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
  HADOOP-9623. Update jet3t dependency to 0.9.0 (Amandeep Khurana via Colin Patrick McCabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1539253)
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/s3/Jets3tFileSystemStore.java
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/s3/MigrationTool.java
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/s3native/Jets3tNativeFileSystemStore.java
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/s3native/NativeS3FileSystem.java
• /hadoop/common/trunk/hadoop-project/pom.xml

FAILURE: Integrated in Hadoop-Mapreduce-trunk #1601 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1601/)
move HADOOP-9623 to 2.2.1 (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1539255)

• /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
  HADOOP-9623. Update jet3t dependency to 0.9.0 (Amandeep Khurana via Colin Patrick McCabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1539253)
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/s3/Jets3tFileSystemStore.java
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/s3/MigrationTool.java
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/s3native/Jets3tNativeFileSystemStore.java
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/s3native/NativeS3FileSystem.java
• /hadoop/common/trunk/hadoop-project/pom.xml

SUCCESS: Integrated in Hadoop-Hdfs-trunk #1575 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1575/)
move HADOOP-9623 to 2.2.1 (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1539255)

• /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
  HADOOP-9623. Update jet3t dependency to 0.9.0 (Amandeep Khurana via Colin Patrick McCabe) (cmccabe: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1539253)
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/s3/Jets3tFileSystemStore.java
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/s3/MigrationTool.java
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/s3native/Jets3tNativeFileSystemStore.java
• /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/s3native/NativeS3FileSystem.java
• /hadoop/common/trunk/hadoop-project/pom.xml

I'm just going to highlight that this patch actually appears to have caused some significant regressions in some cases that weren't tested by the default contract. Specifically seek() out of range and some other problems don't raise exceptions any more, just return a null ref that causes NPEs later. I've done fixes for this in the next HADOOP-9361, but the changes in s3n error handling are pretty significant https://github.com/steveloughran/hadoop-trunk/blob/7e674307256b3c853d7ef91e58b74df4607e3200/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/s3native/Jets3tNativeFileSystemStore.java#L423