Uploaded image for project: 'Hadoop Common'
  1. Hadoop Common
  2. HADOOP-9265

S3 blockstore filesystem breaks part of the Filesystem contract

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.1.1, 3.0.0-alpha1
    • Fix Version/s: 3.0.0-alpha1
    • Component/s: fs/s3
    • Labels:
      None

      Description

      The extended tests of HADOOP-9258 show that s3 is failing things which we always expected an FS to do

      1. getFileStatus("/") to return a FileStatus -currently it returns a FileNotFoundException.
      2. rename("somedir","somedir/childdir") to fail. currently it returns true after deleting all the data in somedir/
      1. HADOOP-9265.patch
        3 kB
        Tom White
      2. HADOOP-9265.patch
        3 kB
        Tom White

        Issue Links

          Activity

          Hide
          aw Allen Wittenauer added a comment -

          Removed from the release note field:
          fixed in HADOOP-9258

          Show
          aw Allen Wittenauer added a comment - Removed from the release note field: fixed in HADOOP-9258
          Hide
          hudson Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #1384 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1384/)
          HADOOP-9258 Add stricter tests to FileSystemContractTestBase (includes fixes for production code HADOOP-9261 & HADOOP-9265 and test enhancements HADOOP-9228, HADOOP-9227 & HADOOP-9259) (Revision 1460646)

          Result = FAILURE
          stevel : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1460646
          Files :

          • /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/S3FileSystem.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/s3native/NativeS3FileSystem.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileSystemContractBaseTest.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/s3/InMemoryFileSystemStore.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/s3native/NativeS3FileSystemContractBaseTest.java
          Show
          hudson Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1384 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1384/ ) HADOOP-9258 Add stricter tests to FileSystemContractTestBase (includes fixes for production code HADOOP-9261 & HADOOP-9265 and test enhancements HADOOP-9228 , HADOOP-9227 & HADOOP-9259 ) (Revision 1460646) Result = FAILURE stevel : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1460646 Files : /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/S3FileSystem.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/s3native/NativeS3FileSystem.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileSystemContractBaseTest.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/s3/InMemoryFileSystemStore.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/s3native/NativeS3FileSystemContractBaseTest.java
          Hide
          hudson Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #1356 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1356/)
          HADOOP-9258 Add stricter tests to FileSystemContractTestBase (includes fixes for production code HADOOP-9261 & HADOOP-9265 and test enhancements HADOOP-9228, HADOOP-9227 & HADOOP-9259) (Revision 1460646)

          Result = FAILURE
          stevel : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1460646
          Files :

          • /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/S3FileSystem.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/s3native/NativeS3FileSystem.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileSystemContractBaseTest.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/s3/InMemoryFileSystemStore.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/s3native/NativeS3FileSystemContractBaseTest.java
          Show
          hudson Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1356 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1356/ ) HADOOP-9258 Add stricter tests to FileSystemContractTestBase (includes fixes for production code HADOOP-9261 & HADOOP-9265 and test enhancements HADOOP-9228 , HADOOP-9227 & HADOOP-9259 ) (Revision 1460646) Result = FAILURE stevel : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1460646 Files : /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/S3FileSystem.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/s3native/NativeS3FileSystem.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileSystemContractBaseTest.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/s3/InMemoryFileSystemStore.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/s3native/NativeS3FileSystemContractBaseTest.java
          Hide
          hudson Hudson added a comment -

          Integrated in Hadoop-Yarn-trunk #167 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/167/)
          HADOOP-9258 Add stricter tests to FileSystemContractTestBase (includes fixes for production code HADOOP-9261 & HADOOP-9265 and test enhancements HADOOP-9228, HADOOP-9227 & HADOOP-9259) (Revision 1460646)

          Result = SUCCESS
          stevel : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1460646
          Files :

          • /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/S3FileSystem.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/s3native/NativeS3FileSystem.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileSystemContractBaseTest.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/s3/InMemoryFileSystemStore.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/s3native/NativeS3FileSystemContractBaseTest.java
          Show
          hudson Hudson added a comment - Integrated in Hadoop-Yarn-trunk #167 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/167/ ) HADOOP-9258 Add stricter tests to FileSystemContractTestBase (includes fixes for production code HADOOP-9261 & HADOOP-9265 and test enhancements HADOOP-9228 , HADOOP-9227 & HADOOP-9259 ) (Revision 1460646) Result = SUCCESS stevel : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1460646 Files : /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/S3FileSystem.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/s3native/NativeS3FileSystem.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileSystemContractBaseTest.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/s3/InMemoryFileSystemStore.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/s3native/NativeS3FileSystemContractBaseTest.java
          Hide
          hudson Hudson added a comment -

          Integrated in Hadoop-trunk-Commit #3520 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3520/)
          HADOOP-9258 Add stricter tests to FileSystemContractTestBase (includes fixes for production code HADOOP-9261 & HADOOP-9265 and test enhancements HADOOP-9228, HADOOP-9227 & HADOOP-9259) (Revision 1460646)

          Result = SUCCESS
          stevel : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1460646
          Files :

          • /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/S3FileSystem.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/s3native/NativeS3FileSystem.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileSystemContractBaseTest.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/s3/InMemoryFileSystemStore.java
          • /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/s3native/NativeS3FileSystemContractBaseTest.java
          Show
          hudson Hudson added a comment - Integrated in Hadoop-trunk-Commit #3520 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3520/ ) HADOOP-9258 Add stricter tests to FileSystemContractTestBase (includes fixes for production code HADOOP-9261 & HADOOP-9265 and test enhancements HADOOP-9228 , HADOOP-9227 & HADOOP-9259 ) (Revision 1460646) Result = SUCCESS stevel : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1460646 Files : /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/S3FileSystem.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/s3native/NativeS3FileSystem.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileSystemContractBaseTest.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/s3/InMemoryFileSystemStore.java /hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/s3native/NativeS3FileSystemContractBaseTest.java
          Hide
          mattf Matt Foley added a comment -

          Steve, have you reviewed and agree this can be committed? Chris gave +1.

          Show
          mattf Matt Foley added a comment - Steve, have you reviewed and agree this can be committed? Chris gave +1.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          ignore the last comment, it's referring to HADOOP-9261

          Show
          stevel@apache.org Steve Loughran added a comment - ignore the last comment, it's referring to HADOOP-9261
          Hide
          stevel@apache.org Steve Loughran added a comment -

          There's enough changes here that we do need rigorous review.

          The previous patch saw copy-onto-self and bailed out early -but these checks were made before the destination path was fully generated -because if you specify a destination directory, the source goes in under there. Which means you could set the dest dir to be the parent of the source file, so generating a rename(src, src) after the equality check had taken place -a rename that would return false.

          Now the value is checked once up front -for fast exit without talking to S3, and then just before the operation actually takes place.

          I also added lots of inline comments to make it clearer what is going on.

          Show
          stevel@apache.org Steve Loughran added a comment - There's enough changes here that we do need rigorous review. The previous patch saw copy-onto-self and bailed out early -but these checks were made before the destination path was fully generated -because if you specify a destination directory, the source goes in under there. Which means you could set the dest dir to be the parent of the source file, so generating a rename(src, src) after the equality check had taken place -a rename that would return false. Now the value is checked once up front -for fast exit without talking to S3, and then just before the operation actually takes place. I also added lots of inline comments to make it clearer what is going on.
          Hide
          cnauroth Chris Nauroth added a comment -

          +1 for the new patch

          I repeated the tests and they passed.

          Just to clarify, I was not objecting to the prior patch. I just wanted to mention the error handling issue for consideration, and Steve made some good points to justify the behavior.

          Show
          cnauroth Chris Nauroth added a comment - +1 for the new patch I repeated the tests and they passed. Just to clarify, I was not objecting to the prior patch. I just wanted to mention the error handling issue for consideration, and Steve made some good points to justify the behavior.
          Hide
          tomwhite Tom White added a comment -

          Thanks for the feedback. Here's an updated version which creates a root inode on S3 in case it doesn't exist.

          Show
          tomwhite Tom White added a comment - Thanks for the feedback. Here's an updated version which creates a root inode on S3 in case it doesn't exist.
          Hide
          cnauroth Chris Nauroth added a comment -

          +1

          Thanks, Steve. I expect a realistic usage scenario wouldn't stop at querying root, so the client would just see those S3 errors a little later in the sequence.

          Show
          cnauroth Chris Nauroth added a comment - +1 Thanks, Steve. I expect a realistic usage scenario wouldn't stop at querying root, so the client would just see those S3 errors a little later in the sequence.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          chris -good point. But it responds really fast -and round trip time is always visible on S3, especially remotely.

          I'm +1 to this patch -it's not going to break anyone's existing code as the operations would always fail to date. And as nobody has ever reported it yet, its possibly never been encountered before.

          Show
          stevel@apache.org Steve Loughran added a comment - chris -good point. But it responds really fast -and round trip time is always visible on S3, especially remotely. I'm +1 to this patch -it's not going to break anyone's existing code as the operations would always fail to date. And as nobody has ever reported it yet, its possibly never been encountered before.
          Hide
          cnauroth Chris Nauroth added a comment -

          I applied the patch locally and verified that it fixed the new tests from HADOOP-9258.

          -    InputStream in = get(pathToKey(path), true);
          +    String key = pathToKey(path);
          +    if (isRoot(key)) {
          +      return true;
          +    }
          +    InputStream in = get(key, true);
          

          With this change, a query for root always resolves locally without interacting with S3. This could mask some other kinds of S3 errors, such as AccessDenied, ExpiredToken, or NoSuchBucket. Is that a concern?

          Show
          cnauroth Chris Nauroth added a comment - I applied the patch locally and verified that it fixed the new tests from HADOOP-9258 . - InputStream in = get(pathToKey(path), true ); + String key = pathToKey(path); + if (isRoot(key)) { + return true ; + } + InputStream in = get(key, true ); With this change, a query for root always resolves locally without interacting with S3. This could mask some other kinds of S3 errors, such as AccessDenied, ExpiredToken, or NoSuchBucket. Is that a concern?
          Hide
          tomwhite Tom White added a comment -

          Here's a fix for both these problems. With the patch and HADOOP-9258 all the tests in Jets3tS3FileSystemContractTest and TestInMemoryS3FileSystemContract pass.

          Show
          tomwhite Tom White added a comment - Here's a fix for both these problems. With the patch and HADOOP-9258 all the tests in Jets3tS3FileSystemContractTest and TestInMemoryS3FileSystemContract pass.

            People

            • Assignee:
              stevel@apache.org Steve Loughran
              Reporter:
              stevel@apache.org Steve Loughran
            • Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development