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

Add a Blobstore interface to add to blobstore FileSystems

    Details

    • Type: Improvement
    • Status: Patch Available
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: 2.6.0
    • Fix Version/s: None
    • Component/s: fs, fs/s3, fs/swift
    • Labels:
      None

      Description

      We can make the fact that some FileSystem implementations are really blobstores, with different atomicity and consistency guarantees, by adding a Blobstore interface to add to them.

      This could also be a place to add a Copy(Path,Path) method, assuming that all blobstores implement at server-side copy operation as a substitute for rename.

      1. HADOOP-9565-001.patch
        16 kB
        Steve Loughran
      2. HADOOP-9565-002.patch
        17 kB
        Steve Loughran
      3. HADOOP-9565-003.patch
        26 kB
        Steve Loughran
      4. HADOOP-9565-004.patch
        51 kB
        Thomas Demoor
      5. HADOOP-9565-005.patch
        55 kB
        Thomas Demoor
      6. HADOOP-9565-006.patch
        65 kB
        Steve Loughran
      7. HADOOP-9565-008.patch
        66 kB
        Mingliang Liu
      8. HADOOP-9565-010.patch
        46 kB
        Steve Loughran
      9. HADOOP-9565-branch-2-007.patch
        71 kB
        Pieter Reuse

        Issue Links

          Activity

          Hide
          stevel@apache.org Steve Loughran added a comment -

          This is what I'm thinking

          1. Blobstore interface for FileSystem and AbstractFileSystem
          2. blobstores to provide an instance specific enum set of consistency guarantees
          3. a local/remote flag, so that those FSs that know when they are in-cluster (swift explicitly, s3 once it does an nslookup & sees the bucket is on a 10. address).
          4. an optional (how to check?) copy() method that triggers a remote copy see http://docs.aws.amazon.com/AmazonS3/latest/API/RESTObjectCOPY.html for an example -much lower cost than a local copy.
          5. a put(Path, InputStream, length) that can be used to do a direct PUT without having to buffer it through a local file. This would be more efficient and should offer a better way to do these uploads than hiding them in a close operation that may be having its exceptions swallowed.
          Show
          stevel@apache.org Steve Loughran added a comment - This is what I'm thinking Blobstore interface for FileSystem and AbstractFileSystem blobstores to provide an instance specific enum set of consistency guarantees a local/remote flag, so that those FSs that know when they are in-cluster (swift explicitly, s3 once it does an nslookup & sees the bucket is on a 10. address). an optional (how to check?) copy() method that triggers a remote copy see http://docs.aws.amazon.com/AmazonS3/latest/API/RESTObjectCOPY.html for an example -much lower cost than a local copy. a put(Path, InputStream, length) that can be used to do a direct PUT without having to buffer it through a local file. This would be more efficient and should offer a better way to do these uploads than hiding them in a close operation that may be having its exceptions swallowed.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Simplest just to start with a marker interface,

          1. consistency rules vary across S3 clusters, so an enum couldn't even be constant for an implementation -would need to know the endpoint. Safest to say "assume eventual consistency and non atomic operations"
          2. Put and copy may not be supported by all, should really go into their own interfaces.
          Show
          stevel@apache.org Steve Loughran added a comment - Simplest just to start with a marker interface, consistency rules vary across S3 clusters, so an enum couldn't even be constant for an implementation -would need to know the endpoint. Safest to say "assume eventual consistency and non atomic operations" Put and copy may not be supported by all, should really go into their own interfaces.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          also: call it ObjectStore as that is potentially more widely known/the formal name.

          Show
          stevel@apache.org Steve Loughran added a comment - also: call it ObjectStore as that is potentially more widely known/the formal name.
          Show
          ksumit Sumit Kumar added a comment - I see some interface like this in hadoop-azure codebase https://github.com/apache/hadoop-common/blob/trunk/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/StorageInterface.java
          Hide
          thodemoor Thomas Demoor added a comment -

          I support the enum set to indicate system properties such as consistency and operation guarantees and the other ideas postulated above but believe this will be a daunting task.

          HDFS is a consistent single-writer filesystem that uses temp files + rename for concurrency. A lot of object stores are not consistent and they are evidently multi-writer, as they are distributed systems. Therefore, as Steve pointed out in the pdf, rename and delete are difficult to implement atomically. Some remarks:

          Consistency:

          1. Eventual consistency is an ambiguous term (read-after-write, atomic reads, etc.), thus more detail will have to be provided. Fortunately, the fact that no concurrent writes are performed makes things easier. However, writing meaningful test seems very difficult. Will one impose a time limit to define "eventual"? Data loss is destined to happen: HADOOP-9577
          2. We have chosen strong consistency (even for multi-geo sites) over availability.
          3. I believe Azure is consistent under certain conditions (single geo site, ...) thus, as Steve pointed out, the enum set would also be end-point dependent for Azure.
          4. Documenting (cfr. pdf) where consistency and atomicity are required in the MapReduce (and other?) applications running on top of YARN is super interesting

          MapReduce-HDFS integration

          1. Other filesystems struggle due to the fact that hadoop.mapreduce code is written with HDFS in mind. For instance, as discussed in (the comments of) HADOOP-9577, a job writes to different temp dirs for each job attempt and then renames one to the output dir upon committing. An object store would rather let all speculative copies write to the same filename and let one of them "win". However, this requires implementing a custom OutputCommitter, which is not part of hadoop.fs and things quickly get quite messy.
          2. Another example is storing the intermediate output (shuffle phase) on local disk, to spare the HDFS NameNode, rather than in the distributed storage system (cfr. MapR Hadoop).
          3. Evidently, this tight integration of mapreduce and HDFS is completely understandable but maybe, similar to resource management and job tracking being pulled out to YARN, these semantics, which lie very close to the FileSystem, might be unified in the future.

          We fairly recently picked up interest in Hadoop and would like to join the "HCFS". If all goes well, we plan on contributing to HADOOP-9361 in the future.

          Show
          thodemoor Thomas Demoor added a comment - I support the enum set to indicate system properties such as consistency and operation guarantees and the other ideas postulated above but believe this will be a daunting task. HDFS is a consistent single-writer filesystem that uses temp files + rename for concurrency. A lot of object stores are not consistent and they are evidently multi-writer, as they are distributed systems. Therefore, as Steve pointed out in the pdf, rename and delete are difficult to implement atomically. Some remarks: Consistency: Eventual consistency is an ambiguous term (read-after-write, atomic reads, etc.), thus more detail will have to be provided. Fortunately, the fact that no concurrent writes are performed makes things easier. However, writing meaningful test seems very difficult. Will one impose a time limit to define "eventual"? Data loss is destined to happen: HADOOP-9577 We have chosen strong consistency (even for multi-geo sites) over availability. I believe Azure is consistent under certain conditions (single geo site, ...) thus, as Steve pointed out, the enum set would also be end-point dependent for Azure. Documenting (cfr. pdf) where consistency and atomicity are required in the MapReduce (and other?) applications running on top of YARN is super interesting MapReduce-HDFS integration Other filesystems struggle due to the fact that hadoop.mapreduce code is written with HDFS in mind. For instance, as discussed in (the comments of) HADOOP-9577 , a job writes to different temp dirs for each job attempt and then renames one to the output dir upon committing. An object store would rather let all speculative copies write to the same filename and let one of them "win". However, this requires implementing a custom OutputCommitter, which is not part of hadoop.fs and things quickly get quite messy. Another example is storing the intermediate output (shuffle phase) on local disk, to spare the HDFS NameNode, rather than in the distributed storage system (cfr. MapR Hadoop). Evidently, this tight integration of mapreduce and HDFS is completely understandable but maybe, similar to resource management and job tracking being pulled out to YARN, these semantics, which lie very close to the FileSystem, might be unified in the future. We fairly recently picked up interest in Hadoop and would like to join the "HCFS". If all goes well, we plan on contributing to HADOOP-9361 in the future.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Thomas: good points, I'll follow up in more detail. in HDFS-9361 I have tried to specify how HDFS behaves, including what is explicity atomic.
          -what is harder is knowing how applications expect HDFS to behave...

          Show
          stevel@apache.org Steve Loughran added a comment - Thomas: good points, I'll follow up in more detail. in HDFS-9361 I have tried to specify how HDFS behaves, including what is explicity atomic. -what is harder is knowing how applications expect HDFS to behave...
          Hide
          thodemoor Thomas Demoor added a comment -

          My bad, I saw the pdf attached to HADOOP-9371 and stopped thinking Hence, I missed that the apt.vm documentation file included in the patches is much more comprehensive. I'll give it a look (together with the markdown documentation in HADOOP-9361).

          Show
          thodemoor Thomas Demoor added a comment - My bad, I saw the pdf attached to HADOOP-9371 and stopped thinking Hence, I missed that the apt.vm documentation file included in the patches is much more comprehensive. I'll give it a look (together with the markdown documentation in HADOOP-9361 ).
          Hide
          stevel@apache.org Steve Loughran added a comment -

          This is the -001 patch, which defines a new class, {{abstract class ObjectStoreFileSystem extends FileSystem }}, which is a marker interface. It allows the FS Semantics to be queried, and requirements placed on an FS & raise an IOE if they aren't met. semantics include: consistency (create, delete, list, update), atomicity (create, delete, rename...) and whether flush() does anything.

          This patch is just the class and tests, without any update to the FS specification doc, and without reparenting the in-hadoop-codebase object stores, which for swift:// and //s3? come with nearly no guarantees.

          1. Swift offers create consistency
          2. s3 create consistency varies a bit by region though.
          3. Azure does offer a consistent mode, so you can swap hdfs out for it.
          4. Netflix S3mper and AWS equivalent does improve s3 consistency, especially metadata consistency

          I don't expect apps to adjust their behaviour based on the guarantees, but think some code could/should reject operations if the dest isn't suitable for their actions (e.g. speculative operations committed by rename() require atomic rename operations)

          Show
          stevel@apache.org Steve Loughran added a comment - This is the -001 patch, which defines a new class, {{abstract class ObjectStoreFileSystem extends FileSystem }}, which is a marker interface. It allows the FS Semantics to be queried, and requirements placed on an FS & raise an IOE if they aren't met. semantics include: consistency (create, delete, list, update), atomicity (create, delete, rename...) and whether flush() does anything. This patch is just the class and tests, without any update to the FS specification doc, and without reparenting the in-hadoop-codebase object stores, which for swift:// and //s3? come with nearly no guarantees. Swift offers create consistency s3 create consistency varies a bit by region though. Azure does offer a consistent mode, so you can swap hdfs out for it. Netflix S3mper and AWS equivalent does improve s3 consistency, especially metadata consistency I don't expect apps to adjust their behaviour based on the guarantees, but think some code could/should reject operations if the dest isn't suitable for their actions (e.g. speculative operations committed by rename() require atomic rename operations)
          Hide
          hadoopqa Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 2 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          -1 release audit. The applied patch generated 1 release audit warnings.

          -1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-common:

          org.apache.hadoop.metrics2.impl.TestMetricsSystemImpl

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

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/5139//testReport/
          Release audit warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/5139//artifact/patchprocess/patchReleaseAuditProblems.txt
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5139//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12684414/HADOOP-9565-001.patch against trunk revision c732ed7. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. -1 release audit . The applied patch generated 1 release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-common-project/hadoop-common: org.apache.hadoop.metrics2.impl.TestMetricsSystemImpl +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/5139//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/5139//artifact/patchprocess/patchReleaseAuditProblems.txt Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5139//console This message is automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          patch -002; fixes missing header from test file

          Show
          stevel@apache.org Steve Loughran added a comment - patch -002; fixes missing header from test file
          Hide
          hadoopqa Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12685312/HADOOP-9565-002.patch
          against trunk revision 0653918.

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

          +1 tests included. The patch appears to include 2 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) 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/5161//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5161//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12685312/HADOOP-9565-002.patch against trunk revision 0653918. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) 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/5161//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5161//console This message is automatically generated.
          Hide
          hadoopqa Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 2 new or modified test files.

          -1 javac. The patch appears to cause the build to fail.

          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5182//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12685312/HADOOP-9565-002.patch against trunk revision e227fb8. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. -1 javac . The patch appears to cause the build to fail. Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5182//console This message is automatically generated.
          Hide
          hadoopqa Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 2 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-common:

          org.apache.hadoop.ha.TestZKFailoverControllerStress

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

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

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12685312/HADOOP-9565-002.patch against trunk revision e227fb8. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-common-project/hadoop-common: org.apache.hadoop.ha.TestZKFailoverControllerStress +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/5183//testReport/ Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5183//console This message is automatically generated.
          Hide
          hadoopqa Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 2 new or modified test files.

          -1 javac. The patch appears to cause the build to fail.

          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5184//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12685312/HADOOP-9565-002.patch against trunk revision e227fb8. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. -1 javac . The patch appears to cause the build to fail. Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5184//console This message is automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          HADOOP-11525 proposes another flag, whether copy has side-effects in the dest fs.

          Show
          stevel@apache.org Steve Loughran added a comment - HADOOP-11525 proposes another flag, whether copy has side-effects in the dest fs.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Patch -003

          1. renamed class to simply ObjectStore
          2. upped semantics mask to 64 bits long.
          3. pulled all semantic bitmasks to a StorageSemantics interface
          4. added the semantics of DISTINCT_FILES_AND_DIRECTORIES, WRITE_IN_PROGRESS_VISIBLE and WRITE_ON_FLUSH.
          5. marked up s3, s3n and swift. S3 really has a consistency model that is instance-specific; need to make it something that you can declare in your fs config options if it matters that much and you know what is offered.
          6. implemented use case of HADOOP-11525 ; handling of WRITE_IN_PROGRESS_VISIBLE on a copy, so avoiding a PUT/GET/PUT/DELETE sequence.

          It'd be good to have a couple more uses of this method sorted out, so that we can see how well it works...someone needs to write an output committer that does the best it can given the semantics offered by the destination

          Show
          stevel@apache.org Steve Loughran added a comment - Patch -003 renamed class to simply ObjectStore upped semantics mask to 64 bits long. pulled all semantic bitmasks to a StorageSemantics interface added the semantics of DISTINCT_FILES_AND_DIRECTORIES , WRITE_IN_PROGRESS_VISIBLE and WRITE_ON_FLUSH . marked up s3, s3n and swift. S3 really has a consistency model that is instance-specific; need to make it something that you can declare in your fs config options if it matters that much and you know what is offered. implemented use case of HADOOP-11525 ; handling of WRITE_IN_PROGRESS_VISIBLE on a copy, so avoiding a PUT/GET/PUT/DELETE sequence. It'd be good to have a couple more uses of this method sorted out, so that we can see how well it works...someone needs to write an output committer that does the best it can given the semantics offered by the destination
          Hide
          hadoopqa Hadoop QA added a comment -

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

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

          +1 tests included. The patch appears to include 2 new or modified test files.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 javadoc. There were no new javadoc warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          -1 release audit. The applied patch generated 1 release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws hadoop-tools/hadoop-openstack.

          Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/5550//testReport/
          Release audit warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/5550//artifact/patchprocess/patchReleaseAuditProblems.txt
          Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5550//console

          This message is automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12695818/HADOOP-9565-003.patch against trunk revision ffc75d6. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) warnings. -1 release audit . The applied patch generated 1 release audit warnings. +1 core tests . The patch passed unit tests in hadoop-common-project/hadoop-common hadoop-tools/hadoop-aws hadoop-tools/hadoop-openstack. Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/5550//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/5550//artifact/patchprocess/patchReleaseAuditProblems.txt Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/5550//console This message is automatically generated.
          Hide
          cnauroth Chris Nauroth added a comment -

          This is nice work, Steve. Thanks for putting this together. Here are a few comments:

          1. Have you considered attaching semantics directly to FileSystem and FileContext instead of introducing the ObjectStore abstract base class? This could give us a modest simplification of calling code, because it wouldn't have to downcast, either directly or implicitly by taking the extra hop through the static ObjectStore#hasStoreSemantics.
          2. Shall we change StoreSemantics to an enum and then phrase the API in terms of EnumSet<StoreSemantics> for improved type safety? There is existing precedent for use of EnumSet in the FileSystem API, so I expect callers will find it familiar.
          3. I saw JavaDoc typos on ATOMIC_DIR_DELETE and DISTINCT_FILES_AND_DIRECTORIES.
          Show
          cnauroth Chris Nauroth added a comment - This is nice work, Steve. Thanks for putting this together. Here are a few comments: Have you considered attaching semantics directly to FileSystem and FileContext instead of introducing the ObjectStore abstract base class? This could give us a modest simplification of calling code, because it wouldn't have to downcast, either directly or implicitly by taking the extra hop through the static ObjectStore#hasStoreSemantics . Shall we change StoreSemantics to an enum and then phrase the API in terms of EnumSet<StoreSemantics> for improved type safety? There is existing precedent for use of EnumSet in the FileSystem API, so I expect callers will find it familiar. I saw JavaDoc typos on ATOMIC_DIR_DELETE and DISTINCT_FILES_AND_DIRECTORIES .
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Semantics directly off FileContext and FileSystem?

          1. Having a clear separation between object store and FS tells the world that if something doesn't say extends ObjectStore then its an FS with all the normal expectations of consistency, atomicity, durability, etc. Having that extra subclass can exist to warn that something may be wrong.
          2. making getSemantics() abstract forces everyone to look at what their semantics really are and declare them, rather than take a possibly incorrect default. (we couldn't make it abstract and would have to default to POSIX)

          That said:

          1. those object stores that can replace HDFS are effectively filesystems. The ObjectStore extension would then only be needed if/when we added more features (e.g PUT?)
          2. having it everywhere makes it easier to chain filesystems together; some wrapper FS client (like a performance counter) could relay the probe without caring about FS type; callers would know it is there too.
          3. we could add something alongside querying capabilities. Today we have filesystems that don't support append (checksum FS), seek on streams (FTP), truncate, extended attributes, encryption flags, etc. There's no cue that they are missing other than exceptions when you try to use them.

          I do fear that trying to add semantics and feature flags to the FS API itself is going to prove more controversial. We could start with ObjectStore and then decide whether to pull up at a later date.

          enumset vs bitmask?

          Bitmask.

          It's easier to manipulate during chaining. Something like Netflix S3mper injects consistency atop s3, so could do

          long getSemantics() {
            return inner.getSemantics() | STORE_CONSISTENCY_COMPLETE;
          }
          

          or —and this is the hard one in enumset —, something that removed a feature

          long getSemantics() {
            return inner.getSemantics()  & ! CONSISTENT_CREATE ;
          }
          

          we could also use the operation in reporting error messages, such as highlighting which requirements weren't met in the exception text:

          long s = store.getSemantics();
          if ( (s & STORE_POSIX_WRITE_SEMANTICS) != STORE_POSIX_WRITE_SEMANTICS) {
            throw new IOException("Missing semantics:" + ( s & STORE_POSIX_WRITE_SEMANTICS) + " see https://wiki.apache.org/hadoop/ObjectStore");
          } 
          

          Where it really excels though, is the fact that a numeric value can be defined in a hadoop configuration XML. As a hex value.

          Thus someone could say

           <property>
            <name>fs.s3a.semantics</name>
            <value>0x0f</value>
          </property>
          

          I think we will need precisely that for S3 clients, because some S3-API endpoints (e.g what Amplidata are doing) do offer stricter semantics, and even amazon itself varies between "nothing", 0x0 on US-East, to create, 0x01 , everywhere else.

          The only way we could let people configure it in the XML file is to use integers, ideally with the values (including common aggregated values) listed somewhere. The javadocs will do this, —automatically for the decimal values, manually for the hex ones if we add that (I've postponed it until the patch is ready & the values are fixed)

          Therefore while I agree with anyone who thinks it is a low-level C/C++ view of the world, in the hands of the competent, it is more powerful than the Java work that tries to wrap it all in set theory.

          Show
          stevel@apache.org Steve Loughran added a comment - Semantics directly off FileContext and FileSystem ? Having a clear separation between object store and FS tells the world that if something doesn't say extends ObjectStore then its an FS with all the normal expectations of consistency, atomicity, durability, etc. Having that extra subclass can exist to warn that something may be wrong. making getSemantics() abstract forces everyone to look at what their semantics really are and declare them, rather than take a possibly incorrect default. (we couldn't make it abstract and would have to default to POSIX) That said: those object stores that can replace HDFS are effectively filesystems. The ObjectStore extension would then only be needed if/when we added more features (e.g PUT?) having it everywhere makes it easier to chain filesystems together; some wrapper FS client (like a performance counter) could relay the probe without caring about FS type; callers would know it is there too. we could add something alongside querying capabilities. Today we have filesystems that don't support append (checksum FS), seek on streams (FTP), truncate, extended attributes, encryption flags, etc. There's no cue that they are missing other than exceptions when you try to use them. I do fear that trying to add semantics and feature flags to the FS API itself is going to prove more controversial. We could start with ObjectStore and then decide whether to pull up at a later date. enumset vs bitmask? Bitmask. It's easier to manipulate during chaining. Something like Netflix S3mper injects consistency atop s3, so could do long getSemantics() { return inner .getSemantics() | STORE_CONSISTENCY_COMPLETE; } or —and this is the hard one in enumset —, something that removed a feature long getSemantics() { return inner .getSemantics() & ! CONSISTENT_CREATE ; } we could also use the operation in reporting error messages, such as highlighting which requirements weren't met in the exception text: long s = store.getSemantics(); if ( (s & STORE_POSIX_WRITE_SEMANTICS) != STORE_POSIX_WRITE_SEMANTICS) { throw new IOException( "Missing semantics:" + ( s & STORE_POSIX_WRITE_SEMANTICS) + " see https: //wiki.apache.org/hadoop/ObjectStore" ); } Where it really excels though, is the fact that a numeric value can be defined in a hadoop configuration XML. As a hex value. Thus someone could say <property> <name>fs.s3a.semantics</name> <value>0x0f</value> </property> I think we will need precisely that for S3 clients, because some S3-API endpoints (e.g what Amplidata are doing) do offer stricter semantics, and even amazon itself varies between "nothing", 0x0 on US-East, to create, 0x01 , everywhere else. The only way we could let people configure it in the XML file is to use integers, ideally with the values (including common aggregated values) listed somewhere. The javadocs will do this, —automatically for the decimal values, manually for the hex ones if we add that (I've postponed it until the patch is ready & the values are fixed) Therefore while I agree with anyone who thinks it is a low-level C/C++ view of the world, in the hands of the competent, it is more powerful than the Java work that tries to wrap it all in set theory.
          Hide
          cnauroth Chris Nauroth added a comment -

          Thanks for the detailed response, Steve.

          I have found myself wanting the Java set API (not just EnumSet) to make it easier to chain operations.

          I have no objection to using the extra ObjectStore layer now and considering promotion up to FileSystem later. However, this does get a bit more involved on the FileContext side. I suppose it means we'd need:

          1. ObjectStoreAbstractFileSystem for implementors to extend.
          2. ObjectStoreFileContext used as a return value from FileContext#getFileContext and queried for semantics by callers.
          3. ObjectStoreDelegateToFileSystem so that implementors still have the convenience of DelegateToFileSystem as a simple passthrough after they have a working FileSystem subclass.
          Show
          cnauroth Chris Nauroth added a comment - Thanks for the detailed response, Steve. I have found myself wanting the Java set API (not just EnumSet ) to make it easier to chain operations. I have no objection to using the extra ObjectStore layer now and considering promotion up to FileSystem later. However, this does get a bit more involved on the FileContext side. I suppose it means we'd need: ObjectStoreAbstractFileSystem for implementors to extend. ObjectStoreFileContext used as a return value from FileContext#getFileContext and queried for semantics by callers. ObjectStoreDelegateToFileSystem so that implementors still have the convenience of DelegateToFileSystem as a simple passthrough after they have a working FileSystem subclass.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          yes, it gets more complex in FileSystem. That's where having uniform operations would be nice.

          But what if we were to add some object-store-centric operations, where PUT(path, inputstream src, long len) is the obvious choice -rather than pretending it was a streaming write? I suppose that could go into AbstractFileSystem and on a filesys be implemented as a streaming write.

          Show
          stevel@apache.org Steve Loughran added a comment - yes, it gets more complex in FileSystem. That's where having uniform operations would be nice. But what if we were to add some object-store-centric operations, where PUT(path, inputstream src, long len) is the obvious choice -rather than pretending it was a streaming write? I suppose that could go into AbstractFileSystem and on a filesys be implemented as a streaming write.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          One thing to consider is: what are the key bits of code which care that the destination is an object store.

          I think distCp may be one: it's often used as the way to get data in/out a transient HDFS instance.

          HADOOP-11487 shows how eventual consistency can catch it out

          Show
          stevel@apache.org Steve Loughran added a comment - One thing to consider is: what are the key bits of code which care that the destination is an object store. I think distCp may be one: it's often used as the way to get data in/out a transient HDFS instance. HADOOP-11487 shows how eventual consistency can catch it out
          Hide
          thodemoor Thomas Demoor added a comment -

          I think I identified the rename path in HADOOP-11487, see my comment. I'd have to take a closer look to check if we can provide a workaround. But I'll take a stab at the FileOutputCommitter first, as that sees heavier use.

          +1 for the idea of "fs.s3a.semantics". There are consistent S3 object stores out there. some of the other semantics are hopeless

          Show
          thodemoor Thomas Demoor added a comment - I think I identified the rename path in HADOOP-11487 , see my comment. I'd have to take a closer look to check if we can provide a workaround. But I'll take a stab at the FileOutputCommitter first, as that sees heavier use. +1 for the idea of "fs.s3a.semantics". There are consistent S3 object stores out there. some of the other semantics are hopeless
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Thomas, if you could look at how to do a good committer to object stores, and so verify whether the information here is adequate, that'd be great.

          At the very least we should be looking for this interface on the speculative committer and logging @ warn.

          Show
          stevel@apache.org Steve Loughran added a comment - Thomas, if you could look at how to do a good committer to object stores, and so verify whether the information here is adequate, that'd be great. At the very least we should be looking for this interface on the speculative committer and logging @ warn.
          Hide
          eddyxu Lei (Eddy) Xu added a comment -

          Hi, Steve Loughran and Thomas Demoor. Do you guys have any update on this one?

          Show
          eddyxu Lei (Eddy) Xu added a comment - Hi, Steve Loughran and Thomas Demoor . Do you guys have any update on this one?
          Hide
          stevel@apache.org Steve Loughran added a comment -

          ..no, though I was talking to Sanjay Radia last week on the topic of doing this as part of FileContext instead: that'd have to expose the information & relay it to implementations, moving code that wanted to know these things into using the newer API

          Show
          stevel@apache.org Steve Loughran added a comment - ..no, though I was talking to Sanjay Radia last week on the topic of doing this as part of FileContext instead: that'd have to expose the information & relay it to implementations, moving code that wanted to know these things into using the newer API
          Hide
          thodemoor Thomas Demoor added a comment -

          Just got back from holiday, I'll finish up my patch and post it this week (once I process the dreaded holiday backlog).

          Show
          thodemoor Thomas Demoor added a comment - Just got back from holiday, I'll finish up my patch and post it this week (once I process the dreaded holiday backlog).
          Hide
          thodemoor Thomas Demoor added a comment -

          Any ideas on how we can test the changes in other packages (CommandWithDestination, FileOutputCommiiter) vs an AWS bucket with maven? We're currently moving offices so I can't access to my test cluster so no extensive tests for now. Guess I'll have to wait it out.

          Moving to FileContext is probably fine but I'm slightly concerned about external projects/ still using the old API. How many are there?

          FYI THe FileOutputCommitter has seen some patches recently (MAPREDUCE-4815, MAPREDUCE-6275).

          Show
          thodemoor Thomas Demoor added a comment - Any ideas on how we can test the changes in other packages (CommandWithDestination, FileOutputCommiiter) vs an AWS bucket with maven? We're currently moving offices so I can't access to my test cluster so no extensive tests for now. Guess I'll have to wait it out. Moving to FileContext is probably fine but I'm slightly concerned about external projects/ still using the old API. How many are there? FYI THe FileOutputCommitter has seen some patches recently ( MAPREDUCE-4815 , MAPREDUCE-6275 ).
          Hide
          thodemoor Thomas Demoor added a comment -

          FYI, currently testing my implementation.

          Show
          thodemoor Thomas Demoor added a comment - FYI, currently testing my implementation.
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          -1 patch 0m 0s The patch command could not apply the patch during dryrun.



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12695818/HADOOP-9565-003.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / 3fa2efc
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/6585/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 patch 0m 0s The patch command could not apply the patch during dryrun. Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12695818/HADOOP-9565-003.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 3fa2efc Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/6585/console This message was automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          I know that lots of client apps are using the old FileSystem API, but here we are adding a new feature with new methods, targeted at apps that want to differentiate real posix-ish filesystems from object stores. This is new code.

          I've been thinking about the problem about non-atomic and slow operations (esp rename and delete) recently.

          Would it make sense to add asynchronous operations for these, in both object store and filecontext? Something like

          Future renameAsync(Path src, Path dst, boolean recurse)
          Future rmAsync(Path src, Path dst, boolean recurse)
          Future copyAsync(Path src, Path dst, boolean recurse)
          

          All filesystems could use a thread pool for operations; rename() and rm() would be fast, while copy would be the slow one. On an object store, rename and RM would be slow; copy could be one remotely if it supported a COPY operation, and that rename could be implemented as COPY+delete. Furthermore, as pointed out to me by Nick Dimiduk, for AWS S3 you can do an async rm by setting the TTL of a path to 1s and letting the object store do all the heavy lifting.

          The Future would return when the request had been submitted to the FS (throwing any IOE if needed); we'd have no guarantees about visibility to callers.

          Making these operations async would make it clear that they were potentially slow and not immediately visible, and allow apps to offload work so their sequence of actions would be fast (e.g. responding to user requests), with that slowness handled consistency elsewhere, and allowing for object stores to implement their algorithms appropriately. It wouldn't be enough on its own for a blobstore-aware output committer (due to the need for visibility to all callers), but could be a start.

          Show
          stevel@apache.org Steve Loughran added a comment - I know that lots of client apps are using the old FileSystem API, but here we are adding a new feature with new methods, targeted at apps that want to differentiate real posix-ish filesystems from object stores. This is new code. I've been thinking about the problem about non-atomic and slow operations (esp rename and delete) recently. Would it make sense to add asynchronous operations for these, in both object store and filecontext? Something like Future renameAsync(Path src, Path dst, boolean recurse) Future rmAsync(Path src, Path dst, boolean recurse) Future copyAsync(Path src, Path dst, boolean recurse) All filesystems could use a thread pool for operations; rename() and rm() would be fast, while copy would be the slow one. On an object store, rename and RM would be slow; copy could be one remotely if it supported a COPY operation, and that rename could be implemented as COPY+delete. Furthermore, as pointed out to me by Nick Dimiduk , for AWS S3 you can do an async rm by setting the TTL of a path to 1s and letting the object store do all the heavy lifting. The Future would return when the request had been submitted to the FS (throwing any IOE if needed); we'd have no guarantees about visibility to callers. Making these operations async would make it clear that they were potentially slow and not immediately visible, and allow apps to offload work so their sequence of actions would be fast (e.g. responding to user requests), with that slowness handled consistency elsewhere, and allowing for object stores to implement their algorithms appropriately. It wouldn't be enough on its own for a blobstore-aware output committer (due to the need for visibility to all callers), but could be a start.
          Hide
          airbots Chen He added a comment -

          Thank you for the contribution Steve Loughran. I have a question about the copying process. Why we have to add ".COPYING" for swift storage which is using filename to decide the location of file blocks?

          Another potential problem is the rename process. It may cause YARN timeout (10 mins) if we use distcp to copy a large file.

          Show
          airbots Chen He added a comment - Thank you for the contribution Steve Loughran . I have a question about the copying process. Why we have to add ". COPYING " for swift storage which is using filename to decide the location of file blocks? Another potential problem is the rename process. It may cause YARN timeout (10 mins) if we use distcp to copy a large file.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          I don't know about .COPYING. that's not done in the swiftfs itself, it's distcp/CLI
          rename: same thing.

          distcp assumes that directory rename is atomic and fast, so does a copy with the suffix COPYING so that the (slow) upload can take place bit by bit. When the upload is finished it does a quick rename() , only in swift and s3 that rename is in fact a second copy.

          the fix here is for distcp & fs cli to recognise object stores and act differently, or adding an explicit 'don't rename" option for all object stores.

          Show
          stevel@apache.org Steve Loughran added a comment - I don't know about .COPYING . that's not done in the swiftfs itself, it's distcp/CLI rename: same thing. distcp assumes that directory rename is atomic and fast, so does a copy with the suffix COPYING so that the (slow) upload can take place bit by bit. When the upload is finished it does a quick rename() , only in swift and s3 that rename is in fact a second copy. the fix here is for distcp & fs cli to recognise object stores and act differently, or adding an explicit 'don't rename" option for all object stores.
          Hide
          airbots Chen He added a comment -

          Thank you for the explanation, Steve Loughran. You are right, the .COPYING is added by CLI (distcp refers to this also) and hardcoded there. IMHO, it may be more flexible if we can choose to not add .COPYING by setting a parameter like "OBJECTSTORE_NO_RENAME_IN_COPY".

          Show
          airbots Chen He added a comment - Thank you for the explanation, Steve Loughran . You are right, the . COPYING is added by CLI (distcp refers to this also) and hardcoded there. IMHO, it may be more flexible if we can choose to not add . COPYING by setting a parameter like "OBJECTSTORE_NO_RENAME_IN_COPY".
          Hide
          airbots Chen He added a comment -

          The ".COPYING" mechanism actually has problem. I create the bug HDFS-8673.

          Show
          airbots Chen He added a comment - The ". COPYING " mechanism actually has problem. I create the bug HDFS-8673 .
          Hide
          eddyxu Lei (Eddy) Xu added a comment -

          Chen He, you might want to check the discussion in HADOOP-11525 about how to avoid COPYING file. I feel that using this blobstore interface would a better solution, because it is a more fundamental and comprehensive fix for the problem.

          Show
          eddyxu Lei (Eddy) Xu added a comment - Chen He , you might want to check the discussion in HADOOP-11525 about how to avoid COPYING file. I feel that using this blobstore interface would a better solution, because it is a more fundamental and comprehensive fix for the problem.
          Hide
          Thomas Demoor Thomas Demoor added a comment -

          004 introduces an objectStore-aware FileOutputCommitter: it writes directly to the outputPath if in-progress/failed writes are not visible on the object store.

          Jitendra Nath Pandey, Anu Engineer, Chris Nauroth and Jakob Homan: this is the approach I think HDFS-7240 can easily use this to avoid the "rename issue".

          The implementation optios were constrained by the fact that FileOutputCommitter has a stable public interface:

          • implementing the changes by subclassing was not possible
          • the public / protected methods could not be changed. Often they are also static (for use in MR1), further limiting options.

          Some smaller stylistic changes (checkstyle, etc.) to the code in 003.patch

          Show
          Thomas Demoor Thomas Demoor added a comment - 004 introduces an objectStore-aware FileOutputCommitter: it writes directly to the outputPath if in-progress/failed writes are not visible on the object store. Jitendra Nath Pandey , Anu Engineer , Chris Nauroth and Jakob Homan : this is the approach I think HDFS-7240 can easily use this to avoid the "rename issue". The implementation optios were constrained by the fact that FileOutputCommitter has a stable public interface: implementing the changes by subclassing was not possible the public / protected methods could not be changed. Often they are also static (for use in MR1), further limiting options. Some smaller stylistic changes (checkstyle, etc.) to the code in 003.patch
          Hide
          eddyxu Lei (Eddy) Xu added a comment -

          Hi, Thomas Demoor. Thanks a lot for picking up this one.

          I have a general question here. This 004 patch looks as a similar approach to HADOOP-11525, in which it exposes storage semantics / characteris to the caller. I got the impression that we were talking about some designs that hide the file/object store implementation behind an interface (something like write() vs atomicWrite(), or similar). Maybe I misunderstand this, but do we have some consensus now about what the implementation should be? Steve Loughran and Thomas Demoor, could you give us more color to the design?

          Right now, is the main caller change in FsShell and MapReduce. Do we have a rough estimation about how many other code / projects can be benefits from this? HBase ? Should they modify the code by detecting the semantics?

          Last, addition to the consistency / atomic semantics, should we have some performance-wise flags, e.g., SLOW_RENAME, SLOW_LIST_STATUS and etc. For instance, if the keys are not range partitioned in an object store (potentially in HDFS-7240), listStatus might be slow.

          One minor improvement:

          // lowest common denominator: AWS EAST provides no guarantees
          122	  public static final long SEMANTICS_DEFAULT = 0x0fL;
          

          Could you calculate it by explicitly using StoreSemantics.Foo?

          Thanks much. Looking forward to hear from you.

          Show
          eddyxu Lei (Eddy) Xu added a comment - Hi, Thomas Demoor . Thanks a lot for picking up this one. I have a general question here. This 004 patch looks as a similar approach to HADOOP-11525 , in which it exposes storage semantics / characteris to the caller. I got the impression that we were talking about some designs that hide the file/object store implementation behind an interface (something like write() vs atomicWrite() , or similar). Maybe I misunderstand this, but do we have some consensus now about what the implementation should be? Steve Loughran and Thomas Demoor , could you give us more color to the design? Right now, is the main caller change in FsShell and MapReduce . Do we have a rough estimation about how many other code / projects can be benefits from this? HBase ? Should they modify the code by detecting the semantics? Last, addition to the consistency / atomic semantics, should we have some performance-wise flags, e.g., SLOW_RENAME , SLOW_LIST_STATUS and etc. For instance, if the keys are not range partitioned in an object store (potentially in HDFS-7240 ), listStatus might be slow. One minor improvement: // lowest common denominator: AWS EAST provides no guarantees 122 public static final long SEMANTICS_DEFAULT = 0x0fL; Could you calculate it by explicitly using StoreSemantics.Foo ? Thanks much. Looking forward to hear from you.
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          0 pre-patch 19m 27s Pre-patch trunk compilation is healthy.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 tests included 0m 0s The patch appears to include 2 new or modified test files.
          +1 javac 7m 38s There were no new javac warning messages.
          -1 javadoc 9m 39s The applied patch generated 21 additional warning messages.
          +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings.
          -1 checkstyle 1m 47s The applied patch generated 2 new checkstyle issues (total was 15, now 17).
          +1 whitespace 0m 5s The patch has no lines that end in whitespace.
          +1 install 1m 37s mvn install still works.
          +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
          +1 findbugs 4m 48s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
          +1 common tests 22m 16s Tests passed in hadoop-common.
          +1 mapreduce tests 1m 43s Tests passed in hadoop-mapreduce-client-core.
          +1 tools/hadoop tests 0m 14s Tests passed in hadoop-aws.
          +1 tools/hadoop tests 0m 14s Tests passed in hadoop-openstack.
              70m 54s  



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12743331/HADOOP-9565-004.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / 2eae130
          javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/7136/artifact/patchprocess/diffJavadocWarnings.txt
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/7136/artifact/patchprocess/diffcheckstylehadoop-common.txt
          hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7136/artifact/patchprocess/testrun_hadoop-common.txt
          hadoop-mapreduce-client-core test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7136/artifact/patchprocess/testrun_hadoop-mapreduce-client-core.txt
          hadoop-aws test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7136/artifact/patchprocess/testrun_hadoop-aws.txt
          hadoop-openstack test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7136/artifact/patchprocess/testrun_hadoop-openstack.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7136/testReport/
          Java 1.7.0_55
          uname Linux asf900.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7136/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 19m 27s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 2 new or modified test files. +1 javac 7m 38s There were no new javac warning messages. -1 javadoc 9m 39s The applied patch generated 21 additional warning messages. +1 release audit 0m 23s The applied patch does not increase the total number of release audit warnings. -1 checkstyle 1m 47s The applied patch generated 2 new checkstyle issues (total was 15, now 17). +1 whitespace 0m 5s The patch has no lines that end in whitespace. +1 install 1m 37s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 findbugs 4m 48s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 common tests 22m 16s Tests passed in hadoop-common. +1 mapreduce tests 1m 43s Tests passed in hadoop-mapreduce-client-core. +1 tools/hadoop tests 0m 14s Tests passed in hadoop-aws. +1 tools/hadoop tests 0m 14s Tests passed in hadoop-openstack.     70m 54s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12743331/HADOOP-9565-004.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 2eae130 javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/7136/artifact/patchprocess/diffJavadocWarnings.txt checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/7136/artifact/patchprocess/diffcheckstylehadoop-common.txt hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7136/artifact/patchprocess/testrun_hadoop-common.txt hadoop-mapreduce-client-core test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7136/artifact/patchprocess/testrun_hadoop-mapreduce-client-core.txt hadoop-aws test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7136/artifact/patchprocess/testrun_hadoop-aws.txt hadoop-openstack test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7136/artifact/patchprocess/testrun_hadoop-openstack.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7136/testReport/ Java 1.7.0_55 uname Linux asf900.gq1.ygridcore.net 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7136/console This message was automatically generated.
          Hide
          Thomas Demoor Thomas Demoor added a comment -

          Thanks for your review Lei (Eddy) Xu. Indeed, the patch contains code similar to that of HADOOP-11525 as Steve Loughran previously merged that into 003.patch. My additions to his work (diff between 003.patch and 004.patch) are mainly to the FileOutputCommitter.

          FileOutputCommitter

          Most parts of the larger ecosystem use FileOutputCommitter: MapReduce, Spark, Tez, ... HBase is a notable counterexample: I don' know if they use rename a lot, I do know append is an operation typically not supported on object stores (thus I think HBase on an objectstore is not a very good usecase). I think you have a better view of the ecosystem than I do: do you know of hadoop filesystem "users" that do not use FileOutputCommitter but do "commit by renaming" themselves? I am willing to try to optimize them for objectstores, if the usecase makes sense. I will have a look at making distcp object-store aware as well (so expect 005.patch ).

          Currently, what we do is basically if(canWeUseAtomicWrite()), which then triggers one of the 2 (relatively) separate code paths. I don't think there's a simple one-liner one can change from write() to atomicWrite(): as HDFS is a single-writer POSIX-style filesystem, write() is accompanied by a whole scheme of other operations that together make up the (higher-level) atomic "commit operation". For instance for the FileOutputCommitter: object stores want a clean atomicWrite() but for HDFS it uses a scheme of temporary directories for each attempt and at the end it commits the "successful" attempt by renaming and deletes all other attempts. I do not see how that can be replaced by a simple interface write()/atomicWrite() while keeping backwards-compatibility (FileOutputCommitter is hardcoded in lots of applications), but suggestions would be very welcome.

          Slow / Async operations

          I think the flags for slow operations are a good idea. I think Steve Loughran's comment above on adding async operations takes that idea one step further. Some renames / deletes can be async, other can't but evidently only the client can know if async is possible for their codepath. I think these are good ideas to offer more options to users in the future, but it will require some PR to get them picked up. The code that is currently in the patch aims at things we can improve in a manner that is invisible to end-users. I think therefore these are best tracked in separate issues.

          Steve Loughran and Nick Dimiduk: afaik TTL is a feature of AWS Cloudfront (cache): i.e. TTL in the cache, get from S3 after that. This does not affect the S3 object. Furthermore, S3 has an "Object Expiration" feature, but its policy is defined per bucket so I'm not sure it's directly applicable here.

          Show
          Thomas Demoor Thomas Demoor added a comment - Thanks for your review Lei (Eddy) Xu . Indeed, the patch contains code similar to that of HADOOP-11525 as Steve Loughran previously merged that into 003.patch. My additions to his work (diff between 003.patch and 004.patch) are mainly to the FileOutputCommitter. FileOutputCommitter Most parts of the larger ecosystem use FileOutputCommitter: MapReduce, Spark, Tez, ... HBase is a notable counterexample: I don' know if they use rename a lot, I do know append is an operation typically not supported on object stores (thus I think HBase on an objectstore is not a very good usecase). I think you have a better view of the ecosystem than I do: do you know of hadoop filesystem "users" that do not use FileOutputCommitter but do "commit by renaming" themselves? I am willing to try to optimize them for objectstores, if the usecase makes sense. I will have a look at making distcp object-store aware as well (so expect 005.patch ). Currently, what we do is basically if(canWeUseAtomicWrite()) , which then triggers one of the 2 (relatively) separate code paths. I don't think there's a simple one-liner one can change from write() to atomicWrite() : as HDFS is a single-writer POSIX-style filesystem, write() is accompanied by a whole scheme of other operations that together make up the (higher-level) atomic "commit operation". For instance for the FileOutputCommitter: object stores want a clean atomicWrite() but for HDFS it uses a scheme of temporary directories for each attempt and at the end it commits the "successful" attempt by renaming and deletes all other attempts. I do not see how that can be replaced by a simple interface write() / atomicWrite() while keeping backwards-compatibility (FileOutputCommitter is hardcoded in lots of applications), but suggestions would be very welcome. Slow / Async operations I think the flags for slow operations are a good idea. I think Steve Loughran 's comment above on adding async operations takes that idea one step further. Some renames / deletes can be async, other can't but evidently only the client can know if async is possible for their codepath. I think these are good ideas to offer more options to users in the future, but it will require some PR to get them picked up. The code that is currently in the patch aims at things we can improve in a manner that is invisible to end-users. I think therefore these are best tracked in separate issues. Steve Loughran and Nick Dimiduk : afaik TTL is a feature of AWS Cloudfront (cache): i.e. TTL in the cache, get from S3 after that. This does not affect the S3 object. Furthermore, S3 has an "Object Expiration" feature, but its policy is defined per bucket so I'm not sure it's directly applicable here.
          Hide
          ndimiduk Nick Dimiduk added a comment -

          Thomas Demoor from the S3 docs, looks like it has supported asynchronous object expiration through TTL for some time.

          Show
          ndimiduk Nick Dimiduk added a comment - Thomas Demoor from the S3 docs , looks like it has supported asynchronous object expiration through TTL for some time.
          Hide
          Thomas Demoor Thomas Demoor added a comment -

          Hi Nick,

          Steve Loughran also brought this up in HADOOP-12292. Please see my response there explaining why I don't think this is a valid option.

          Show
          Thomas Demoor Thomas Demoor added a comment - Hi Nick, Steve Loughran also brought this up in HADOOP-12292 . Please see my response there explaining why I don't think this is a valid option.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          -1 patch 0m 3s HADOOP-9565 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.



          Subsystem Report/Notes
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12743331/HADOOP-9565-004.patch
          JIRA Issue HADOOP-9565
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8620/console
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. -1 patch 0m 3s HADOOP-9565 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. Subsystem Report/Notes JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12743331/HADOOP-9565-004.patch JIRA Issue HADOOP-9565 Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8620/console Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          Thomas Demoor Thomas Demoor added a comment -

          Adjusted distcp: removed tempfile usage for object stores that support direct concurrent writes. Evidently, this yields a 2x speedup .

          AFAIK we have most things covered now?

          Show
          Thomas Demoor Thomas Demoor added a comment - Adjusted distcp: removed tempfile usage for object stores that support direct concurrent writes. Evidently, this yields a 2x speedup . AFAIK we have most things covered now?
          Hide
          Thomas Demoor Thomas Demoor added a comment -

          Also fixed the javadoc issues (thanks Emre Sevinç).

          Show
          Thomas Demoor Thomas Demoor added a comment - Also fixed the javadoc issues (thanks Emre Sevinç ).
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 11s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          0 mvndep 0m 14s Maven dependency ordering for branch
          +1 mvninstall 7m 3s trunk passed
          +1 compile 6m 29s trunk passed with JDK v1.8.0_66
          +1 compile 7m 9s trunk passed with JDK v1.7.0_91
          +1 checkstyle 1m 10s trunk passed
          +1 mvnsite 2m 41s trunk passed
          +1 mvneclipse 1m 8s trunk passed
          +1 findbugs 4m 14s trunk passed
          +1 javadoc 2m 0s trunk passed with JDK v1.8.0_66
          +1 javadoc 2m 19s trunk passed with JDK v1.7.0_91
          0 mvndep 0m 15s Maven dependency ordering for patch
          +1 mvninstall 1m 57s the patch passed
          +1 compile 6m 26s the patch passed with JDK v1.8.0_66
          +1 javac 6m 26s the patch passed
          +1 compile 7m 9s the patch passed with JDK v1.7.0_91
          +1 javac 7m 9s the patch passed
          -1 checkstyle 1m 9s root: patch generated 5 new + 204 unchanged - 11 fixed = 209 total (was 215)
          +1 mvnsite 2m 41s the patch passed
          +1 mvneclipse 1m 8s the patch passed
          -1 whitespace 0m 0s The patch has 26 line(s) that end in whitespace. Use git apply --whitespace=fix.
          +1 findbugs 5m 21s the patch passed
          -1 javadoc 3m 0s hadoop-common-project_hadoop-common-jdk1.8.0_66 with JDK v1.8.0_66 generated 15 new + 1 unchanged - 0 fixed = 16 total (was 1)
          +1 javadoc 1m 58s the patch passed with JDK v1.8.0_66
          -1 javadoc 5m 38s hadoop-common-project_hadoop-common-jdk1.7.0_91 with JDK v1.7.0_91 generated 15 new + 13 unchanged - 0 fixed = 28 total (was 13)
          +1 javadoc 2m 16s the patch passed with JDK v1.7.0_91
          -1 unit 7m 42s hadoop-common in the patch failed with JDK v1.8.0_66.
          +1 unit 2m 3s hadoop-mapreduce-client-core in the patch passed with JDK v1.8.0_66.
          +1 unit 7m 19s hadoop-distcp in the patch passed with JDK v1.8.0_66.
          +1 unit 0m 12s hadoop-openstack in the patch passed with JDK v1.8.0_66.
          +1 unit 0m 13s hadoop-aws in the patch passed with JDK v1.8.0_66.
          +1 unit 8m 15s hadoop-common in the patch passed with JDK v1.7.0_91.
          +1 unit 2m 23s hadoop-mapreduce-client-core in the patch passed with JDK v1.7.0_91.
          +1 unit 7m 16s hadoop-distcp in the patch passed with JDK v1.7.0_91.
          +1 unit 0m 13s hadoop-openstack in the patch passed with JDK v1.7.0_91.
          +1 unit 0m 15s hadoop-aws in the patch passed with JDK v1.7.0_91.
          +1 asflicense 0m 24s Patch does not generate ASF License warnings.
          103m 9s



          Reason Tests
          JDK v1.8.0_66 Failed junit tests hadoop.security.ssl.TestReloadingX509TrustManager



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12787808/HADOOP-9565-005.patch
          JIRA Issue HADOOP-9565
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux b65a10505902 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / ec12ce8
          Default Java 1.7.0_91
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/8621/artifact/patchprocess/diff-checkstyle-root.txt
          whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/8621/artifact/patchprocess/whitespace-eol.txt
          javadoc hadoop-common-project_hadoop-common-jdk1.8.0_66: https://builds.apache.org/job/PreCommit-HADOOP-Build/8621/artifact/patchprocess/diff-javadoc-javadoc-hadoop-common-project_hadoop-common-jdk1.8.0_66.txt
          javadoc hadoop-common-project_hadoop-common-jdk1.7.0_91: https://builds.apache.org/job/PreCommit-HADOOP-Build/8621/artifact/patchprocess/diff-javadoc-javadoc-hadoop-common-project_hadoop-common-jdk1.7.0_91.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8621/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_66.txt
          unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/8621/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_66.txt
          JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8621/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-tools/hadoop-distcp hadoop-tools/hadoop-openstack hadoop-tools/hadoop-aws U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8621/console
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 11s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 14s Maven dependency ordering for branch +1 mvninstall 7m 3s trunk passed +1 compile 6m 29s trunk passed with JDK v1.8.0_66 +1 compile 7m 9s trunk passed with JDK v1.7.0_91 +1 checkstyle 1m 10s trunk passed +1 mvnsite 2m 41s trunk passed +1 mvneclipse 1m 8s trunk passed +1 findbugs 4m 14s trunk passed +1 javadoc 2m 0s trunk passed with JDK v1.8.0_66 +1 javadoc 2m 19s trunk passed with JDK v1.7.0_91 0 mvndep 0m 15s Maven dependency ordering for patch +1 mvninstall 1m 57s the patch passed +1 compile 6m 26s the patch passed with JDK v1.8.0_66 +1 javac 6m 26s the patch passed +1 compile 7m 9s the patch passed with JDK v1.7.0_91 +1 javac 7m 9s the patch passed -1 checkstyle 1m 9s root: patch generated 5 new + 204 unchanged - 11 fixed = 209 total (was 215) +1 mvnsite 2m 41s the patch passed +1 mvneclipse 1m 8s the patch passed -1 whitespace 0m 0s The patch has 26 line(s) that end in whitespace. Use git apply --whitespace=fix. +1 findbugs 5m 21s the patch passed -1 javadoc 3m 0s hadoop-common-project_hadoop-common-jdk1.8.0_66 with JDK v1.8.0_66 generated 15 new + 1 unchanged - 0 fixed = 16 total (was 1) +1 javadoc 1m 58s the patch passed with JDK v1.8.0_66 -1 javadoc 5m 38s hadoop-common-project_hadoop-common-jdk1.7.0_91 with JDK v1.7.0_91 generated 15 new + 13 unchanged - 0 fixed = 28 total (was 13) +1 javadoc 2m 16s the patch passed with JDK v1.7.0_91 -1 unit 7m 42s hadoop-common in the patch failed with JDK v1.8.0_66. +1 unit 2m 3s hadoop-mapreduce-client-core in the patch passed with JDK v1.8.0_66. +1 unit 7m 19s hadoop-distcp in the patch passed with JDK v1.8.0_66. +1 unit 0m 12s hadoop-openstack in the patch passed with JDK v1.8.0_66. +1 unit 0m 13s hadoop-aws in the patch passed with JDK v1.8.0_66. +1 unit 8m 15s hadoop-common in the patch passed with JDK v1.7.0_91. +1 unit 2m 23s hadoop-mapreduce-client-core in the patch passed with JDK v1.7.0_91. +1 unit 7m 16s hadoop-distcp in the patch passed with JDK v1.7.0_91. +1 unit 0m 13s hadoop-openstack in the patch passed with JDK v1.7.0_91. +1 unit 0m 15s hadoop-aws in the patch passed with JDK v1.7.0_91. +1 asflicense 0m 24s Patch does not generate ASF License warnings. 103m 9s Reason Tests JDK v1.8.0_66 Failed junit tests hadoop.security.ssl.TestReloadingX509TrustManager Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12787808/HADOOP-9565-005.patch JIRA Issue HADOOP-9565 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux b65a10505902 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / ec12ce8 Default Java 1.7.0_91 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/8621/artifact/patchprocess/diff-checkstyle-root.txt whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/8621/artifact/patchprocess/whitespace-eol.txt javadoc hadoop-common-project_hadoop-common-jdk1.8.0_66: https://builds.apache.org/job/PreCommit-HADOOP-Build/8621/artifact/patchprocess/diff-javadoc-javadoc-hadoop-common-project_hadoop-common-jdk1.8.0_66.txt javadoc hadoop-common-project_hadoop-common-jdk1.7.0_91: https://builds.apache.org/job/PreCommit-HADOOP-Build/8621/artifact/patchprocess/diff-javadoc-javadoc-hadoop-common-project_hadoop-common-jdk1.7.0_91.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8621/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_66.txt unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/8621/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_66.txt JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8621/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-tools/hadoop-distcp hadoop-tools/hadoop-openstack hadoop-tools/hadoop-aws U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8621/console Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          looking at this —sorry for not doing it earlier. As the initial stuff was my code, I'm not qualified to vote it in though.

          1. I like how you've picked this up for s3a and swift
          2. I think we're going to have to cover azure. Which sort-of has atomic renames (it has renames, they're just not always O(1). I don't know if we can/should include complexity of operation into the bitmap. If so, cost of close(), open(), flush().
          3. and I think we should add a couple of bits for "supports append" and "flush-saves data"; if false it means that nothing gets written until the end.

          As a follow on from this, I'd like to have something in FileContext too. Even with a separate patch, we ought to be able to share structures, the constants. The problem we have there is FileContext is its own thing that relays to the (hidden) AbstractFilesystem subclass. I think we'd need to have a subclass of FileContext which indicates this, or we make publishing FS semantics something common to all filesystems/file contexts. Thoughts?

          Show
          stevel@apache.org Steve Loughran added a comment - looking at this —sorry for not doing it earlier. As the initial stuff was my code, I'm not qualified to vote it in though. I like how you've picked this up for s3a and swift I think we're going to have to cover azure. Which sort-of has atomic renames (it has renames, they're just not always O(1). I don't know if we can/should include complexity of operation into the bitmap. If so, cost of close(), open(), flush(). and I think we should add a couple of bits for "supports append" and "flush-saves data"; if false it means that nothing gets written until the end. As a follow on from this, I'd like to have something in FileContext too. Even with a separate patch, we ought to be able to share structures, the constants. The problem we have there is FileContext is its own thing that relays to the (hidden) AbstractFilesystem subclass. I think we'd need to have a subclass of FileContext which indicates this, or we make publishing FS semantics something common to all filesystems/file contexts. Thoughts?
          Hide
          stevel@apache.org Steve Loughran added a comment -

          I've been thinking about this, and wondering if we could have better extensibility by providing a lookup operation where you asked for the specific method and got back an enum of values:

          
          getOperationSemantics("create") = <SUPPORTED, O_1, CONSISTENT, ATOMIC, SYNCHRONOUS, PERSISTENT>   // HDDA change is visible, operation with check for existence is atomic
          getOperationSemantics("create") = <SUPPORTED, O_1>   // s3
          
          getOperationSemantics("append") = <SUPPORTED, PERSISTENT>   // HDFS: supported but no consistency guarantees
          getOperationSemantics("append") = <>   // s3 doesn't support append
          
          getOperationSemantics("delete") = <SUPPORTED, O_1, CONSISTENT, ATOMIC, SYNCHRONOUS, PERSISTENT>   // HDFS: supported but no consistency guarantees
          getOperationSemantics("delete") = <SUPPORTED, O_N, PERSISTENT>   // maybe async
          
          getOperationSemantics("rename") = <SUPPORTED, O_1, CONSISTENT, ATOMIC, SYNCHRONOUS, PERSISTENT> // hdfs
          getOperationSemantics("rename") = <SUPPORTED, CLIENT_SIDE, O_N, SYNCHRONOUS, PERSISTENT> // s3
          
          getOperationSemantics("OutputStream.close") = <SUPPORTED, O_N, ATOMIC, SYNCHRONOUS, PERSISTENT>  // s3
          getOperationSemantics("OutputStream.close") = <SUPPORTED, O_1, ATOMIC, CONSISTENT, SYNCHRONOUS, PERSISTENT>  // HDFS
          getOperationSemantics("OutputStream.write") = <SUPPORTED, O_N, PERSISTENT>  // HDFS
          getOperationSemantics("OutputStream.write") = <SUPPORTED, O_1>  // s3
          
          getOperationSemantics("OutputStream.flush") = <SUPPORTED, O_N, SYNCHRONOUS, PERSISTENT>  // HDFS
          getOperationSemantics("OutputStream.flush") = <SUPPORTED, NO_OP, O_1>  // s3 won't fail on the cal, but it doesn't do anything
          
          
          

          I know it's potentially much more complex, especially for clients, but it does expose all the information apps may possibly need.

          Example: dfsclient & can look for rename being 0_1 and !CLIENT_SIDE; if not, it bypasses rename and writes direct.

          another example, some code trying to use create(overwrite=false) for locking could check and fail if "create" wasn't atomic/persistent (i.e. check & create atomic, result visible to all)

          Show
          stevel@apache.org Steve Loughran added a comment - I've been thinking about this, and wondering if we could have better extensibility by providing a lookup operation where you asked for the specific method and got back an enum of values: getOperationSemantics( "create" ) = <SUPPORTED, O_1, CONSISTENT, ATOMIC, SYNCHRONOUS, PERSISTENT> // HDDA change is visible, operation with check for existence is atomic getOperationSemantics( "create" ) = <SUPPORTED, O_1> // s3 getOperationSemantics( "append" ) = <SUPPORTED, PERSISTENT> // HDFS: supported but no consistency guarantees getOperationSemantics( "append" ) = <> // s3 doesn't support append getOperationSemantics( "delete" ) = <SUPPORTED, O_1, CONSISTENT, ATOMIC, SYNCHRONOUS, PERSISTENT> // HDFS: supported but no consistency guarantees getOperationSemantics( "delete" ) = <SUPPORTED, O_N, PERSISTENT> // maybe async getOperationSemantics( "rename" ) = <SUPPORTED, O_1, CONSISTENT, ATOMIC, SYNCHRONOUS, PERSISTENT> // hdfs getOperationSemantics( "rename" ) = <SUPPORTED, CLIENT_SIDE, O_N, SYNCHRONOUS, PERSISTENT> // s3 getOperationSemantics( "OutputStream.close" ) = <SUPPORTED, O_N, ATOMIC, SYNCHRONOUS, PERSISTENT> // s3 getOperationSemantics( "OutputStream.close" ) = <SUPPORTED, O_1, ATOMIC, CONSISTENT, SYNCHRONOUS, PERSISTENT> // HDFS getOperationSemantics( "OutputStream.write" ) = <SUPPORTED, O_N, PERSISTENT> // HDFS getOperationSemantics( "OutputStream.write" ) = <SUPPORTED, O_1> // s3 getOperationSemantics( "OutputStream.flush" ) = <SUPPORTED, O_N, SYNCHRONOUS, PERSISTENT> // HDFS getOperationSemantics( "OutputStream.flush" ) = <SUPPORTED, NO_OP, O_1> // s3 won't fail on the cal, but it doesn't do anything I know it's potentially much more complex, especially for clients, but it does expose all the information apps may possibly need. Example: dfsclient & can look for rename being 0_1 and !CLIENT_SIDE; if not, it bypasses rename and writes direct. another example, some code trying to use create(overwrite=false) for locking could check and fail if "create" wasn't atomic/persistent (i.e. check & create atomic, result visible to all)
          Hide
          aartokhy Aaron Tokhy added a comment -

          FileOutputCommitter recently introduced a notion of 'algorithm version' (through mapreduce.fileoutputcommitter.algorithm.version). Perhaps the change to FileOutputCommitter would involve adding another algorithm version (3) for performing a direct commit, separate from this change? FileOutputCommitter could ideally be split into 3 subclasses today, 1 per algorithm version.

          Another boolean configuration could be added to mapred-default.xml, called mapreduce.fileoutputcommitter.infer.algorithm.version, which would select the appropriate FileOutputCommitter algorithm based on the FileSystem for the Path in mapreduce.output.fileoutputformat.outputdir. MRAppMaster could then select the appropriate algorithm depending on the semantics available in the target filesystem.

          It might also make some sense to break up FileOutputCommitter algorithms into separate classes to accommodate for these changes, since the algorithm used may not work well on some FileSystem implementations (especially due to the attempts of performing atomic renames, on FileSystem implementations where directory renames are not atomic).

          Show
          aartokhy Aaron Tokhy added a comment - FileOutputCommitter recently introduced a notion of 'algorithm version' (through mapreduce.fileoutputcommitter.algorithm.version). Perhaps the change to FileOutputCommitter would involve adding another algorithm version (3) for performing a direct commit, separate from this change? FileOutputCommitter could ideally be split into 3 subclasses today, 1 per algorithm version. Another boolean configuration could be added to mapred-default.xml, called mapreduce.fileoutputcommitter.infer.algorithm.version, which would select the appropriate FileOutputCommitter algorithm based on the FileSystem for the Path in mapreduce.output.fileoutputformat.outputdir. MRAppMaster could then select the appropriate algorithm depending on the semantics available in the target filesystem. It might also make some sense to break up FileOutputCommitter algorithms into separate classes to accommodate for these changes, since the algorithm used may not work well on some FileSystem implementations (especially due to the attempts of performing atomic renames, on FileSystem implementations where directory renames are not atomic).
          Hide
          PieterReuse Pieter Reuse added a comment -

          I've checked with Thomas Demoor and I will work on a new patch addressing the remarks of Steve Loughran and Aaron Tokhy.

          Show
          PieterReuse Pieter Reuse added a comment - I've checked with Thomas Demoor and I will work on a new patch addressing the remarks of Steve Loughran and Aaron Tokhy .
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Looking at this, very nice to see that you have done a committer.

          regarding algorithm versions, maybe we should do something else: have a specific object store committer

          • strengths: isolated. Can also be run against local filesystems experimentally.
          • weakness: takes a long time to get picked up downstream, unless you can explicitly request the committer (as spark does; I don't know about other things)

          There's one more thing we can look at here for committing, which is that on an object store write you may be able to abort the write before the close(), so avoid any data write (and with multipart xfer, kill that specific upload). That could help with speculative writes to the same output file: they could perhaps both target the same destination.

          Show
          stevel@apache.org Steve Loughran added a comment - Looking at this, very nice to see that you have done a committer. regarding algorithm versions, maybe we should do something else: have a specific object store committer strengths: isolated. Can also be run against local filesystems experimentally. weakness: takes a long time to get picked up downstream, unless you can explicitly request the committer (as spark does; I don't know about other things) There's one more thing we can look at here for committing, which is that on an object store write you may be able to abort the write before the close(), so avoid any data write (and with multipart xfer, kill that specific upload). That could help with speculative writes to the same output file: they could perhaps both target the same destination.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          -1 patch 0m 5s HADOOP-9565 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.



          Subsystem Report/Notes
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12787808/HADOOP-9565-005.patch
          JIRA Issue HADOOP-9565
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10084/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. -1 patch 0m 5s HADOOP-9565 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. Subsystem Report/Notes JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12787808/HADOOP-9565-005.patch JIRA Issue HADOOP-9565 Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10084/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          patch 006. Move off bitmask into string named features; various standard ones defined.

          I've also written a stub of a Put command; I'm wondering if we can do that (and a Copy()), what could we do with an explicit committer for object stores

          Show
          stevel@apache.org Steve Loughran added a comment - patch 006. Move off bitmask into string named features; various standard ones defined. I've also written a stub of a Put command; I'm wondering if we can do that (and a Copy()), what could we do with an explicit committer for object stores
          Hide
          cnauroth Chris Nauroth added a comment -

          I haven't looked at this patch in a while, but has there been any consideration of ViewFs? With ViewFs, we'd have a single FileSystem instance visible to the caller, but it would route to different FileSystem implementations dependent upon the specified Path. It might be an object store for some paths, but HDFS for others. Does this API somehow need to be sensitive to Path?

          Show
          cnauroth Chris Nauroth added a comment - I haven't looked at this patch in a while, but has there been any consideration of ViewFs ? With ViewFs, we'd have a single FileSystem instance visible to the caller, but it would route to different FileSystem implementations dependent upon the specified Path . It might be an object store for some paths, but HDFS for others. Does this API somehow need to be sensitive to Path ?
          Hide
          PieterReuse Pieter Reuse added a comment -

          Thanks for patch 006, Steve. Having Strings instead of bitmasks is definitely an improvement. But maybe an enum is worth considering here.

          I don't think the Put command adds functionality. An ObjectStore-object is still a FileSystem-object, having the full FileSystem interface available. Or am I missing something here?

          Thank you for mentioning ViewFs here, Chris. It's important that this patch doesn't break the great ease of use ViewFs brings. But considering ViewFs is client-side only and this patch only brings performance enhancements, I don't think it is worth the extra miles checking the config of ViewFs whether the Path belongs to an ObjectStore which is part of a ViewFs instance.

          Show
          PieterReuse Pieter Reuse added a comment - Thanks for patch 006, Steve. Having Strings instead of bitmasks is definitely an improvement. But maybe an enum is worth considering here. I don't think the Put command adds functionality. An ObjectStore-object is still a FileSystem-object, having the full FileSystem interface available. Or am I missing something here? Thank you for mentioning ViewFs here, Chris. It's important that this patch doesn't break the great ease of use ViewFs brings. But considering ViewFs is client-side only and this patch only brings performance enhancements, I don't think it is worth the extra miles checking the config of ViewFs whether the Path belongs to an ObjectStore which is part of a ViewFs instance.
          Hide
          cnauroth Chris Nauroth added a comment -

          I'm afraid I'm unclear about the goals here, specifically related to the definition of Put and Copy operations.

            public void Put(Path path, OutputStream source, long length, long options)
          

          This method signature surprised me. Was the OutputStream argument meant to be an InputStream for the implementation read?

          Like Pieter, it's unclear to me that this is significant as part of the ObjectStore interface. It looks similar to the FileUtil#copy utility methods, not a primitive operation on FileSystem or ObjectStore. Early comments indicate that this could have been a way for S3A to implement a direct streaming write instead of spooling to local disk first. Now that S3A has S3AFastOutputStream, does that meet the need? Or are you looking to provide clients with a guaranteed non-spooling API regardless of configuration?

          Is the options the significant part that I'm missing? Is the idea that the caller passes along options to describe desired semantics, and this gives the ObjectStore implementation the opportunity to throw UnsatisfiedSemanticsException?

          Maybe a usage example showing how you expect callers to use Put would clarify it.

          Regarding Copy, the description indicates that method could substitute for rename. Looking at S3A as a concrete example, the rename already uses S3 server-side copy. How do you expect an S3A implementation of Copy would differ from the current S3AFileSystem#rename? Do you expect it can relax the notoriously complex semantics of rename, and therefore trigger fewer S3 calls to check metadata?

          Perhaps ViewFs could be handled incrementally later based on need. The difficulty is that I think proper ViewFs support will impact the public API definition, and then downstream applications need to react by migrating to ViewFs-compatible method signatures. A prior example is FileSystem#getDelegationToken vs. FileSystem#addDelegationTokens. Even after a long time, there are lingering bugs in downstream applications that have not migrated to addDelegationTokens, making them unusable with ViewFs in secured deployments.

          Show
          cnauroth Chris Nauroth added a comment - I'm afraid I'm unclear about the goals here, specifically related to the definition of Put and Copy operations. public void Put(Path path, OutputStream source, long length, long options) This method signature surprised me. Was the OutputStream argument meant to be an InputStream for the implementation read? Like Pieter, it's unclear to me that this is significant as part of the ObjectStore interface. It looks similar to the FileUtil#copy utility methods, not a primitive operation on FileSystem or ObjectStore . Early comments indicate that this could have been a way for S3A to implement a direct streaming write instead of spooling to local disk first. Now that S3A has S3AFastOutputStream , does that meet the need? Or are you looking to provide clients with a guaranteed non-spooling API regardless of configuration? Is the options the significant part that I'm missing? Is the idea that the caller passes along options to describe desired semantics, and this gives the ObjectStore implementation the opportunity to throw UnsatisfiedSemanticsException ? Maybe a usage example showing how you expect callers to use Put would clarify it. Regarding Copy, the description indicates that method could substitute for rename. Looking at S3A as a concrete example, the rename already uses S3 server-side copy. How do you expect an S3A implementation of Copy would differ from the current S3AFileSystem#rename ? Do you expect it can relax the notoriously complex semantics of rename, and therefore trigger fewer S3 calls to check metadata? Perhaps ViewFs could be handled incrementally later based on need. The difficulty is that I think proper ViewFs support will impact the public API definition, and then downstream applications need to react by migrating to ViewFs-compatible method signatures. A prior example is FileSystem#getDelegationToken vs. FileSystem#addDelegationTokens . Even after a long time, there are lingering bugs in downstream applications that have not migrated to addDelegationTokens , making them unusable with ViewFs in secured deployments.
          Hide
          cnauroth Chris Nauroth added a comment -

          FYI, Steve is out for a few weeks, so please do not expect immediate responses from him. (Have fun, Steve.)

          Show
          cnauroth Chris Nauroth added a comment - FYI, Steve is out for a few weeks, so please do not expect immediate responses from him. (Have fun, Steve.)
          Hide
          PieterReuse Pieter Reuse added a comment -

          Uploaded patch version 7:

          • Renamed UnsatisfiedSemanticsException to UnsatisfiedFeatureException
          • Renamed ObjectStoreFeatures to ObjectStoreFeature and changed to an enum-based approach
          • added style improvements to FileOutputCommitter, since we're changing a lot of lines throughout the class. Makes this patch prone to become non-applicable when other patches change this class.

          Chris, thanks for pointing out Steve will be enjoying a well earned break the coming weeks. We will have patience w.r.t. his response.

          Show
          PieterReuse Pieter Reuse added a comment - Uploaded patch version 7: Renamed UnsatisfiedSemanticsException to UnsatisfiedFeatureException Renamed ObjectStoreFeatures to ObjectStoreFeature and changed to an enum-based approach added style improvements to FileOutputCommitter, since we're changing a lot of lines throughout the class. Makes this patch prone to become non-applicable when other patches change this class. Chris, thanks for pointing out Steve will be enjoying a well earned break the coming weeks. We will have patience w.r.t. his response.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          -1 patch 0m 5s HADOOP-9565 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.



          Subsystem Report/Notes
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12820968/HADOOP-9565-007.patch
          JIRA Issue HADOOP-9565
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10118/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. -1 patch 0m 5s HADOOP-9565 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. Subsystem Report/Notes JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12820968/HADOOP-9565-007.patch JIRA Issue HADOOP-9565 Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10118/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          PieterReuse Pieter Reuse added a comment -

          renamed file to HADOOP-9565-branch-2-007.patch so Hadoop QA can apply it.

          Show
          PieterReuse Pieter Reuse added a comment - renamed file to HADOOP-9565 -branch-2-007.patch so Hadoop QA can apply it.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          -1 docker 5m 28s Docker failed to build yetus/hadoop:b59b8b7.



          Subsystem Report/Notes
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12821320/HADOOP-9565-branch-2-007.patch
          JIRA Issue HADOOP-9565
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10139/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. -1 docker 5m 28s Docker failed to build yetus/hadoop:b59b8b7. Subsystem Report/Notes JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12821320/HADOOP-9565-branch-2-007.patch JIRA Issue HADOOP-9565 Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10139/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          Thomas Demoor Thomas Demoor added a comment - - edited

          Steve the "avoid data write" thing you mention is exactly why these direct outputcommitters (and what I did for the FileOutputCommitter) work on object stores. Multiple writers can write to the same object concurrently. At any point, the last-started successfully-completed write is what is visible.

          Regular put:

          • Content length (=N) communicated at start of request.
          • Once N bytes hit S3 the object becomes visible
          • If hadoop task aborts before writing N bytes the upload will timeout and the object version is garbage collected by S3.

          MulitpartUpload:

          • Requires explicit API call to complete (or abort)
          • Only when complete API call is used the object becomes visible
          • If hadoop task fails the upload will remain to be active (s3a has the purge functionality to automatically clean these up after a certain period) but the object is NOT visible

          The interesting thing to think about are network partitions.

          Show
          Thomas Demoor Thomas Demoor added a comment - - edited Steve the "avoid data write" thing you mention is exactly why these direct outputcommitters (and what I did for the FileOutputCommitter) work on object stores. Multiple writers can write to the same object concurrently. At any point, the last-started successfully-completed write is what is visible. Regular put: Content length (=N) communicated at start of request. Once N bytes hit S3 the object becomes visible If hadoop task aborts before writing N bytes the upload will timeout and the object version is garbage collected by S3. MulitpartUpload: Requires explicit API call to complete (or abort) Only when complete API call is used the object becomes visible If hadoop task fails the upload will remain to be active (s3a has the purge functionality to automatically clean these up after a certain period) but the object is NOT visible The interesting thing to think about are network partitions.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          ...OK, so the multipart is incremental, but and that final commit triggers the artifacts coming into existence.

          one thing we don't have in committers is an operation to explicitly abort the final commit, so that if a speculative task knows that it mustn't commit, it backs off. I think EMR had something like that in the distant past; I don't know about today.

          w.r.t. network partitions, loss of access to S3 from inside an EC2 farm is going to be pretty rare and traumatic; I don't know about other deployments. I would discard the use case "commit to long haul dest" as an unusual —though distcp kind of disproves that, doesn't it

          Show
          stevel@apache.org Steve Loughran added a comment - ...OK, so the multipart is incremental, but and that final commit triggers the artifacts coming into existence. one thing we don't have in committers is an operation to explicitly abort the final commit, so that if a speculative task knows that it mustn't commit, it backs off. I think EMR had something like that in the distant past; I don't know about today. w.r.t. network partitions, loss of access to S3 from inside an EC2 farm is going to be pretty rare and traumatic; I don't know about other deployments. I would discard the use case "commit to long haul dest" as an unusual —though distcp kind of disproves that, doesn't it
          Hide
          airbots Chen He added a comment - - edited

          From our experiences, the main renaming overhead comes from "FileOutputCommitter.commitTask()". Because it moves the files from temp dir to dest dir. Some frameworks may not care whether the final task files are under "dst/_temporary/0/_temporary/" or "dst/". Why don't we add a parameter such as "mapreduce.skip.task.commit" parameter (default is false), so that once a task is done, the output just stay in "dst/_temporary/0/_temporary/". Then, the next job or application just need to take the "dst/" as input dir, they do not care about whether is is deep or not. It avoids the atomicwrite issue, provide compatibility, and avoid rename overhead. If there is no objection, I am happy to create a JIRA to tracking that.

          Show
          airbots Chen He added a comment - - edited From our experiences, the main renaming overhead comes from "FileOutputCommitter.commitTask()". Because it moves the files from temp dir to dest dir. Some frameworks may not care whether the final task files are under "dst/_temporary/0/_temporary/" or "dst/". Why don't we add a parameter such as "mapreduce.skip.task.commit" parameter (default is false), so that once a task is done, the output just stay in "dst/_temporary/0/_temporary/". Then, the next job or application just need to take the "dst/" as input dir, they do not care about whether is is deep or not. It avoids the atomicwrite issue, provide compatibility, and avoid rename overhead. If there is no objection, I am happy to create a JIRA to tracking that.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          the problem with this is that the rename is the commit operation to say "we've successfully committed"; if you grab the temp stuff, that's the same as using any Direct committer: speculation and failures

          Show
          stevel@apache.org Steve Loughran added a comment - the problem with this is that the rename is the commit operation to say "we've successfully committed"; if you grab the temp stuff, that's the same as using any Direct committer: speculation and failures
          Hide
          airbots Chen He added a comment - - edited

          Hi Steve Loughran, thank you for spending time on my question. The new version of FileOutputCommitter has algorithm 2 which does not have serial rename of all tasks in commitJob. Just find the parameter. It should resolve our problem.

          Show
          airbots Chen He added a comment - - edited Hi Steve Loughran , thank you for spending time on my question. The new version of FileOutputCommitter has algorithm 2 which does not have serial rename of all tasks in commitJob. Just find the parameter. It should resolve our problem.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          algorithm 2 is what I've been using for hive & spark testing...it works, but I haven't done enough failure testing to explore its failure modes, especially when speculation=true

          Show
          stevel@apache.org Steve Loughran added a comment - algorithm 2 is what I've been using for hive & spark testing...it works, but I haven't done enough failure testing to explore its failure modes, especially when speculation=true
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          -1 patch 0m 11s HADOOP-9565 does not apply to branch-2. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.



          Subsystem Report/Notes
          JIRA Issue HADOOP-9565
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12821320/HADOOP-9565-branch-2-007.patch
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11432/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. -1 patch 0m 11s HADOOP-9565 does not apply to branch-2. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. Subsystem Report/Notes JIRA Issue HADOOP-9565 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12821320/HADOOP-9565-branch-2-007.patch Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11432/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          liuml07 Mingliang Liu added a comment -

          This seems useful. Do we still work on this? Thanks,

          Show
          liuml07 Mingliang Liu added a comment - This seems useful. Do we still work on this? Thanks,
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          -1 patch 0m 10s HADOOP-9565 does not apply to branch-2. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.



          Subsystem Report/Notes
          JIRA Issue HADOOP-9565
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12821320/HADOOP-9565-branch-2-007.patch
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12236/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. -1 patch 0m 10s HADOOP-9565 does not apply to branch-2. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. Subsystem Report/Notes JIRA Issue HADOOP-9565 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12821320/HADOOP-9565-branch-2-007.patch Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12236/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          It's languished unloved for a long time.

          I'd thought of a marker interface to say "this is not a real FS", but as you know, the behaviour of a blobstore can very between implementations, and indeed actual deployments.

          in HDFS-11644 there's work going on to add a way to probe a stream for having specific capabilities, this would be an equivalent.

          Let me revisit this on friday and see if I can bring it up in line with my current thinking.

          Show
          stevel@apache.org Steve Loughran added a comment - It's languished unloved for a long time. I'd thought of a marker interface to say "this is not a real FS", but as you know, the behaviour of a blobstore can very between implementations, and indeed actual deployments. in HDFS-11644 there's work going on to add a way to probe a stream for having specific capabilities, this would be an equivalent. Let me revisit this on friday and see if I can bring it up in line with my current thinking.
          Hide
          liuml07 Mingliang Liu added a comment -

          Rebased. Should revisit the features each file system supports.

          Show
          liuml07 Mingliang Liu added a comment - Rebased. Should revisit the features each file system supports.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          0 mvndep 1m 30s Maven dependency ordering for branch
          +1 mvninstall 13m 33s trunk passed
          +1 compile 15m 53s trunk passed
          +1 checkstyle 1m 54s trunk passed
          +1 mvnsite 4m 13s trunk passed
          +1 mvneclipse 2m 59s trunk passed
          -1 findbugs 1m 27s hadoop-common-project/hadoop-common in trunk has 19 extant Findbugs warnings.
          -1 findbugs 0m 36s hadoop-tools/hadoop-azure in trunk has 1 extant Findbugs warnings.
          +1 javadoc 3m 29s trunk passed
          0 mvndep 0m 16s Maven dependency ordering for patch
          +1 mvninstall 2m 57s the patch passed
          +1 compile 13m 7s the patch passed
          +1 javac 13m 7s the patch passed
          -0 checkstyle 1m 57s root: The patch generated 17 new + 149 unchanged - 12 fixed = 166 total (was 161)
          +1 mvnsite 4m 33s the patch passed
          +1 mvneclipse 3m 17s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          -1 findbugs 1m 39s hadoop-common-project/hadoop-common generated 1 new + 19 unchanged - 0 fixed = 20 total (was 19)
          -1 javadoc 0m 53s hadoop-common-project_hadoop-common generated 14 new + 0 unchanged - 0 fixed = 14 total (was 0)
          +1 unit 8m 7s hadoop-common in the patch passed.
          +1 unit 2m 47s hadoop-mapreduce-client-core in the patch passed.
          +1 unit 12m 41s hadoop-distcp in the patch passed.
          +1 unit 0m 27s hadoop-openstack in the patch passed.
          +1 unit 0m 33s hadoop-aws in the patch passed.
          +1 unit 1m 32s hadoop-azure in the patch passed.
          +1 unit 0m 26s hadoop-aliyun in the patch passed.
          +1 unit 3m 41s hadoop-azure-datalake in the patch passed.
          +1 asflicense 0m 40s The patch does not generate ASF License warnings.
          143m 30s



          Reason Tests
          FindBugs module:hadoop-common-project/hadoop-common
            The method name org.apache.hadoop.fs.objectstores.ObjectStore.Put(Path, OutputStream, long, long) doesn't start with a lower case letter At ObjectStore.java:long, long) doesn't start with a lower case letter At ObjectStore.java:[line 180]



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:14b5c93
          JIRA Issue HADOOP-9565
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12866428/HADOOP-9565-008.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux e02929b1cae3 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 81092b1
          Default Java 1.8.0_131
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12238/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-common-warnings.html
          findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12238/artifact/patchprocess/branch-findbugs-hadoop-tools_hadoop-azure-warnings.html
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/12238/artifact/patchprocess/diff-checkstyle-root.txt
          findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12238/artifact/patchprocess/new-findbugs-hadoop-common-project_hadoop-common.html
          javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/12238/artifact/patchprocess/diff-javadoc-javadoc-hadoop-common-project_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12238/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-tools/hadoop-distcp hadoop-tools/hadoop-openstack hadoop-tools/hadoop-aws hadoop-tools/hadoop-azure hadoop-tools/hadoop-aliyun hadoop-tools/hadoop-azure-datalake U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12238/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 1m 30s Maven dependency ordering for branch +1 mvninstall 13m 33s trunk passed +1 compile 15m 53s trunk passed +1 checkstyle 1m 54s trunk passed +1 mvnsite 4m 13s trunk passed +1 mvneclipse 2m 59s trunk passed -1 findbugs 1m 27s hadoop-common-project/hadoop-common in trunk has 19 extant Findbugs warnings. -1 findbugs 0m 36s hadoop-tools/hadoop-azure in trunk has 1 extant Findbugs warnings. +1 javadoc 3m 29s trunk passed 0 mvndep 0m 16s Maven dependency ordering for patch +1 mvninstall 2m 57s the patch passed +1 compile 13m 7s the patch passed +1 javac 13m 7s the patch passed -0 checkstyle 1m 57s root: The patch generated 17 new + 149 unchanged - 12 fixed = 166 total (was 161) +1 mvnsite 4m 33s the patch passed +1 mvneclipse 3m 17s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 1m 39s hadoop-common-project/hadoop-common generated 1 new + 19 unchanged - 0 fixed = 20 total (was 19) -1 javadoc 0m 53s hadoop-common-project_hadoop-common generated 14 new + 0 unchanged - 0 fixed = 14 total (was 0) +1 unit 8m 7s hadoop-common in the patch passed. +1 unit 2m 47s hadoop-mapreduce-client-core in the patch passed. +1 unit 12m 41s hadoop-distcp in the patch passed. +1 unit 0m 27s hadoop-openstack in the patch passed. +1 unit 0m 33s hadoop-aws in the patch passed. +1 unit 1m 32s hadoop-azure in the patch passed. +1 unit 0m 26s hadoop-aliyun in the patch passed. +1 unit 3m 41s hadoop-azure-datalake in the patch passed. +1 asflicense 0m 40s The patch does not generate ASF License warnings. 143m 30s Reason Tests FindBugs module:hadoop-common-project/hadoop-common   The method name org.apache.hadoop.fs.objectstores.ObjectStore.Put(Path, OutputStream, long, long) doesn't start with a lower case letter At ObjectStore.java:long, long) doesn't start with a lower case letter At ObjectStore.java: [line 180] Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue HADOOP-9565 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12866428/HADOOP-9565-008.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux e02929b1cae3 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 81092b1 Default Java 1.8.0_131 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12238/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-common-warnings.html findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12238/artifact/patchprocess/branch-findbugs-hadoop-tools_hadoop-azure-warnings.html checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/12238/artifact/patchprocess/diff-checkstyle-root.txt findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12238/artifact/patchprocess/new-findbugs-hadoop-common-project_hadoop-common.html javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/12238/artifact/patchprocess/diff-javadoc-javadoc-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12238/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-tools/hadoop-distcp hadoop-tools/hadoop-openstack hadoop-tools/hadoop-aws hadoop-tools/hadoop-azure hadoop-tools/hadoop-aliyun hadoop-tools/hadoop-azure-datalake U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12238/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          Thomas Demoor Thomas Demoor added a comment -

          This ticket has been worked on by multiple people.

          Steve made a foundation to "detect" objectstores and expose their semantics.
          We added a "directoutputcommitter", I think this is now made redundant by HADOOP-13786 so you should definitely take that out. iirc we also made some distcp hacks which might need to go out.

          Show
          Thomas Demoor Thomas Demoor added a comment - This ticket has been worked on by multiple people. Steve made a foundation to "detect" objectstores and expose their semantics. We added a "directoutputcommitter", I think this is now made redundant by HADOOP-13786 so you should definitely take that out. iirc we also made some distcp hacks which might need to go out.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Linking to HDFS-11644, which relates to something similar for exposing capabilities of specific streams. As well as covering syncable-ness (v. important for HBase), we could share some other facts about the streams.

          I had a talk with Mingliang about this yesterday, explained why I like strings over enums

          1. not brittle to versions: code build against, say, Hadoop 3.1.3 could still probe Hadoop 3.1.1 for a feature only added in Hadoop 3.1.2
          2. lets specific filesystems declare specific capabilities without having to make changes in the base class.

          Looking at the recent patches, though: over complex. I'm now considering

          1. just use a Configuration(false) object as a way of declaring behaviour
          2. something equivalent to contract-test-options.xml for each FS, using a naming scheme like "fs.feature.*; different stores could add their own, e.g. "fs.s3a.feature.consistent"".
          3. pull out anything for validating feature sets into a helper class
          4. make the probe hasFeature(path, feature) so that filesystems which relay FS calls (e.g. viewfs, webhdfs) can propagate the probe.
          Show
          stevel@apache.org Steve Loughran added a comment - Linking to HDFS-11644 , which relates to something similar for exposing capabilities of specific streams. As well as covering syncable-ness (v. important for HBase), we could share some other facts about the streams. I had a talk with Mingliang about this yesterday, explained why I like strings over enums not brittle to versions: code build against, say, Hadoop 3.1.3 could still probe Hadoop 3.1.1 for a feature only added in Hadoop 3.1.2 lets specific filesystems declare specific capabilities without having to make changes in the base class. Looking at the recent patches, though: over complex. I'm now considering just use a Configuration(false) object as a way of declaring behaviour something equivalent to contract-test-options.xml for each FS, using a naming scheme like "fs.feature.*; different stores could add their own, e.g. "fs.s3a.feature.consistent"". pull out anything for validating feature sets into a helper class make the probe hasFeature(path, feature) so that filesystems which relay FS calls (e.g. viewfs, webhdfs) can propagate the probe.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          yes, the committer stuff is obsoleted for now by HADOO-13786. That can make use of a probe for a feature though, especially to see if there's a consistent FS (actually, FileOutputCommitter could do that check internally, and warn on any FS (s3 unless an it says otherwise, swift).

          Show
          stevel@apache.org Steve Loughran added a comment - yes, the committer stuff is obsoleted for now by HADOO-13786. That can make use of a probe for a feature though, especially to see if there's a consistent FS (actually, FileOutputCommitter could do that check internally, and warn on any FS (s3 unless an it says otherwise, swift).
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Patch 010: resetting everything and starting again with nothing but an API to query a filesystem for features. This is a WiP again, up for feedback.

          • boolean FileSystem.hasFeature(Path, String) lets you see if the bit of an FS under a path has a feature
          • Feature names in FileSystemFeatures are pretty much a copy and paste of org.apache.hadoop.fs.contract.ContractOptions.
          • The base class mechanism for setting up feature resolution, FeatureResolver is driven off XML configuration files; with a setup mechanism which allows for subclasses to dynamically redefine the features, such as when setting up a local FS.

          All the work of defining features is already in the contract tests; I've just pulled their declarations into the production hadoop-common, hadoop-hdfs-client JARs, not the test JARs, and modified the two migrated FS's contracts to load in the new XML file as well as the old one.

          One big difference is in resolution of a feature: when a feature is looked for it first looks for the schema-specific option fs.hdfs.contract.supports-atomic-rename before falling back to look for fs.contract.supports-atomic-rename, which is where a default can be defined.

          This allows for per-FS values to be set in core-default.xml,

          # default: listings work
          fs.contract.supports-consistent-listing  = false
          # but not S3A
          fs.s3a.contract.supports-consistent-listing  = false
          

          And overridden in core-site.xml, as it'd be in a deployment against WDC's servers

          fs.s3a.contract.supports-consistent-listing  = true
          

          And it permits S3A per-bucket override for deployments where only some buckets are consistent, such as "stevel-s3guard"

          fs.s3a.bucket.stevel-s3guard.contract.supports-consistent-listing  = true
          

          Although the feature resolver is driven purely by the loaded hash of config options, because the lookup is via the FS instance, it has the option of being dynamic: per-path, on-demand evaluation rather than on construction probes for OS type, ....

          Also, the underlying feature resolver has a Java 8 optional API, so it can return yes/no/don't know, the idea being "no" can be normative.

          Show
          stevel@apache.org Steve Loughran added a comment - Patch 010: resetting everything and starting again with nothing but an API to query a filesystem for features. This is a WiP again, up for feedback. boolean FileSystem.hasFeature(Path, String) lets you see if the bit of an FS under a path has a feature Feature names in FileSystemFeatures are pretty much a copy and paste of org.apache.hadoop.fs.contract.ContractOptions . The base class mechanism for setting up feature resolution, FeatureResolver is driven off XML configuration files; with a setup mechanism which allows for subclasses to dynamically redefine the features, such as when setting up a local FS. All the work of defining features is already in the contract tests; I've just pulled their declarations into the production hadoop-common, hadoop-hdfs-client JARs, not the test JARs, and modified the two migrated FS's contracts to load in the new XML file as well as the old one. One big difference is in resolution of a feature: when a feature is looked for it first looks for the schema-specific option fs.hdfs.contract.supports-atomic-rename before falling back to look for fs.contract.supports-atomic-rename , which is where a default can be defined. This allows for per-FS values to be set in core-default.xml, # default : listings work fs.contract.supports-consistent-listing = false # but not S3A fs.s3a.contract.supports-consistent-listing = false And overridden in core-site.xml, as it'd be in a deployment against WDC's servers fs.s3a.contract.supports-consistent-listing = true And it permits S3A per-bucket override for deployments where only some buckets are consistent, such as "stevel-s3guard" fs.s3a.bucket.stevel-s3guard.contract.supports-consistent-listing = true Although the feature resolver is driven purely by the loaded hash of config options, because the lookup is via the FS instance, it has the option of being dynamic: per-path, on-demand evaluation rather than on construction probes for OS type, .... Also, the underlying feature resolver has a Java 8 optional API, so it can return yes/no/don't know, the idea being "no" can be normative.
          Hide
          stevel@apache.org Steve Loughran added a comment - - edited

          Note how this is a query-only API, allowing for dynamic probes of FS features, rather than static "does this FS implement a specific interface", and because its done in the base FSClass, no need to add a new interface to cast an FS to to look for it. We just need to make the defaults valid for the filesystems, which we can do by having core-default/xml define the default FS behaviour and with every FS we manage defining their own.

          What about external filesystems? Well, the default resolver looks for a resource called contract/fs-$SCHEMA-features.xml. If the people implementing filesystems copy their contract test XML file into that location in the production JAR, it will be picked up automatically. This allows the authors to update their JARs and have the capabilities be visible on Hadoop implementations with the new API, but still load/run against the old one.

          see also: HDFS-11644 and StreamCapabilities

          Show
          stevel@apache.org Steve Loughran added a comment - - edited Note how this is a query-only API, allowing for dynamic probes of FS features, rather than static "does this FS implement a specific interface", and because its done in the base FSClass, no need to add a new interface to cast an FS to to look for it. We just need to make the defaults valid for the filesystems, which we can do by having core-default/xml define the default FS behaviour and with every FS we manage defining their own. What about external filesystems? Well, the default resolver looks for a resource called contract/fs-$SCHEMA-features.xml . If the people implementing filesystems copy their contract test XML file into that location in the production JAR, it will be picked up automatically. This allows the authors to update their JARs and have the capabilities be visible on Hadoop implementations with the new API, but still load/run against the old one. see also: HDFS-11644 and StreamCapabilities
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 7 new or modified test files.
          0 mvndep 0m 12s Maven dependency ordering for branch
          +1 mvninstall 13m 11s trunk passed
          +1 compile 16m 8s trunk passed
          +1 checkstyle 1m 41s trunk passed
          +1 mvnsite 2m 25s trunk passed
          +1 mvneclipse 0m 50s trunk passed
          -1 findbugs 1m 19s hadoop-common-project/hadoop-common in trunk has 19 extant Findbugs warnings.
          -1 findbugs 1m 22s hadoop-hdfs-project/hadoop-hdfs-client in trunk has 2 extant Findbugs warnings.
          -1 findbugs 1m 38s hadoop-hdfs-project/hadoop-hdfs in trunk has 10 extant Findbugs warnings.
          +1 javadoc 1m 52s trunk passed
          0 mvndep 0m 14s Maven dependency ordering for patch
          +1 mvninstall 1m 58s the patch passed
          +1 compile 13m 11s the patch passed
          -1 javac 13m 11s root generated 1 new + 787 unchanged - 0 fixed = 788 total (was 787)
          -0 checkstyle 1m 46s root: The patch generated 9 new + 106 unchanged - 16 fixed = 115 total (was 122)
          +1 mvnsite 2m 35s the patch passed
          +1 mvneclipse 1m 21s the patch passed
          -1 whitespace 0m 0s The patch has 4 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
          +1 xml 0m 6s The patch has no ill-formed XML file.
          +1 findbugs 5m 2s the patch passed
          -1 javadoc 0m 57s hadoop-common-project_hadoop-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
          -1 unit 6m 55s hadoop-common in the patch failed.
          +1 unit 1m 29s hadoop-hdfs-client in the patch passed.
          -1 unit 63m 39s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 44s The patch does not generate ASF License warnings.
          167m 55s



          Reason Tests
          Failed junit tests hadoop.fs.TestHarFileSystem
            hadoop.conf.TestCommonConfigurationFields
            hadoop.fs.TestFilterFileSystem
            hadoop.hdfs.web.TestWebHdfsTimeouts
            hadoop.hdfs.TestDFSStripedOutputStreamWithFailure010
            hadoop.fs.viewfs.TestViewFileSystemHdfs
            hadoop.hdfs.TestDFSRSDefault10x4StripedOutputStreamWithFailure
            hadoop.hdfs.TestDFSStripedOutputStreamWithFailure080



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:14b5c93
          JIRA Issue HADOOP-9565
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12867125/HADOOP-9565-010.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux 8e8e196badea 4.4.0-43-generic #63-Ubuntu SMP Wed Oct 12 13:48:03 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / c60164f
          Default Java 1.8.0_121
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12282/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-common-warnings.html
          findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12282/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-client-warnings.html
          findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12282/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html
          javac https://builds.apache.org/job/PreCommit-HADOOP-Build/12282/artifact/patchprocess/diff-compile-javac-root.txt
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/12282/artifact/patchprocess/diff-checkstyle-root.txt
          whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/12282/artifact/patchprocess/whitespace-eol.txt
          javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/12282/artifact/patchprocess/diff-javadoc-javadoc-hadoop-common-project_hadoop-common.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12282/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12282/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12282/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12282/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 7 new or modified test files. 0 mvndep 0m 12s Maven dependency ordering for branch +1 mvninstall 13m 11s trunk passed +1 compile 16m 8s trunk passed +1 checkstyle 1m 41s trunk passed +1 mvnsite 2m 25s trunk passed +1 mvneclipse 0m 50s trunk passed -1 findbugs 1m 19s hadoop-common-project/hadoop-common in trunk has 19 extant Findbugs warnings. -1 findbugs 1m 22s hadoop-hdfs-project/hadoop-hdfs-client in trunk has 2 extant Findbugs warnings. -1 findbugs 1m 38s hadoop-hdfs-project/hadoop-hdfs in trunk has 10 extant Findbugs warnings. +1 javadoc 1m 52s trunk passed 0 mvndep 0m 14s Maven dependency ordering for patch +1 mvninstall 1m 58s the patch passed +1 compile 13m 11s the patch passed -1 javac 13m 11s root generated 1 new + 787 unchanged - 0 fixed = 788 total (was 787) -0 checkstyle 1m 46s root: The patch generated 9 new + 106 unchanged - 16 fixed = 115 total (was 122) +1 mvnsite 2m 35s the patch passed +1 mvneclipse 1m 21s the patch passed -1 whitespace 0m 0s The patch has 4 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply +1 xml 0m 6s The patch has no ill-formed XML file. +1 findbugs 5m 2s the patch passed -1 javadoc 0m 57s hadoop-common-project_hadoop-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) -1 unit 6m 55s hadoop-common in the patch failed. +1 unit 1m 29s hadoop-hdfs-client in the patch passed. -1 unit 63m 39s hadoop-hdfs in the patch failed. +1 asflicense 0m 44s The patch does not generate ASF License warnings. 167m 55s Reason Tests Failed junit tests hadoop.fs.TestHarFileSystem   hadoop.conf.TestCommonConfigurationFields   hadoop.fs.TestFilterFileSystem   hadoop.hdfs.web.TestWebHdfsTimeouts   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure010   hadoop.fs.viewfs.TestViewFileSystemHdfs   hadoop.hdfs.TestDFSRSDefault10x4StripedOutputStreamWithFailure   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure080 Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue HADOOP-9565 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12867125/HADOOP-9565-010.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 8e8e196badea 4.4.0-43-generic #63-Ubuntu SMP Wed Oct 12 13:48:03 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / c60164f Default Java 1.8.0_121 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12282/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-common-warnings.html findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12282/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-client-warnings.html findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12282/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html javac https://builds.apache.org/job/PreCommit-HADOOP-Build/12282/artifact/patchprocess/diff-compile-javac-root.txt checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/12282/artifact/patchprocess/diff-checkstyle-root.txt whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/12282/artifact/patchprocess/whitespace-eol.txt javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/12282/artifact/patchprocess/diff-javadoc-javadoc-hadoop-common-project_hadoop-common.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12282/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12282/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12282/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12282/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          stopping work on this briefly as the idea for a config-driven mech now has me confused. Here's why

          1. The defaults for all filesystems need to be set somewhere
          2. The defaults for a specific schema need to be set. They may override this explicit set, or just redeclare everything.
          3. At least for the blobstores, but potentially for file:// too, we need to offer the ability to override the FS defaults. Presumably: core-site
          4. And for s3 we need per-bucket config, again. presumably, core-site.

          customisation for schemas/bucket instances need to be on top of any schema specific, s3a.xml, file.xml; but at the same time, anything in core-default.xml has to come under the schema. And, as the s3a per-bucket config isn't yet common across other instances, that's going to have to be set up by the FS instance.

          Overall then, we can't have a simple chain of new Configuration(true).addResource("s3a.xml") as that doesnt let core-site override s3a.xml values

          And I can't load the s3a values and then core-default + core-site if there's anything in core-default. plus there's per-bucket customisation.

          I think the XML driven one is becoming too convoluted. Probably better to go to switch statements, allowing FS implementations to implement their own policies on top of this if they want. After all, if they were going to have any path-specific state that was how things were going to end up anyway.

          Also, copy & paste of the FS contract test pulls in too many low level properties (the full set of rename semantics) which shouldn't really be exposed. Better to have an initial set of options which are minimal, rely on the new StreamCapabilities for streams.

          • one for every optional method supports-append, supports-concat; default = false
          • core requirements of a filesystem which object stores may not support: has-consistent-create, has-consistent-list, has-consistent-update, has-atomic-rename...
          Show
          stevel@apache.org Steve Loughran added a comment - stopping work on this briefly as the idea for a config-driven mech now has me confused. Here's why The defaults for all filesystems need to be set somewhere The defaults for a specific schema need to be set. They may override this explicit set, or just redeclare everything. At least for the blobstores, but potentially for file:// too, we need to offer the ability to override the FS defaults. Presumably: core-site And for s3 we need per-bucket config, again. presumably, core-site. customisation for schemas/bucket instances need to be on top of any schema specific, s3a.xml, file.xml; but at the same time, anything in core-default.xml has to come under the schema. And, as the s3a per-bucket config isn't yet common across other instances, that's going to have to be set up by the FS instance. Overall then, we can't have a simple chain of new Configuration(true).addResource("s3a.xml") as that doesnt let core-site override s3a.xml values And I can't load the s3a values and then core-default + core-site if there's anything in core-default. plus there's per-bucket customisation. I think the XML driven one is becoming too convoluted. Probably better to go to switch statements, allowing FS implementations to implement their own policies on top of this if they want. After all, if they were going to have any path-specific state that was how things were going to end up anyway. Also, copy & paste of the FS contract test pulls in too many low level properties (the full set of rename semantics) which shouldn't really be exposed. Better to have an initial set of options which are minimal, rely on the new StreamCapabilities for streams. one for every optional method supports-append , supports-concat ; default = false core requirements of a filesystem which object stores may not support : has-consistent-create , has-consistent-list , has-consistent-update , has-atomic-rename ...
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 16s Docker mode activated.
                Prechecks
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 7 new or modified test files.
                trunk Compile Tests
          0 mvndep 0m 42s Maven dependency ordering for branch
          +1 mvninstall 13m 52s trunk passed
          +1 compile 13m 45s trunk passed
          +1 checkstyle 1m 57s trunk passed
          +1 mvnsite 3m 9s trunk passed
          -1 findbugs 1m 34s hadoop-hdfs-project/hadoop-hdfs-client in trunk has 2 extant Findbugs warnings.
          -1 findbugs 1m 47s hadoop-hdfs-project/hadoop-hdfs in trunk has 9 extant Findbugs warnings.
          +1 javadoc 2m 8s trunk passed
                Patch Compile Tests
          0 mvndep 0m 17s Maven dependency ordering for patch
          +1 mvninstall 2m 6s the patch passed
          +1 compile 10m 27s the patch passed
          -1 javac 10m 27s root generated 1 new + 1418 unchanged - 0 fixed = 1419 total (was 1418)
          -0 checkstyle 2m 3s root: The patch generated 9 new + 106 unchanged - 16 fixed = 115 total (was 122)
          +1 mvnsite 3m 13s the patch passed
          -1 whitespace 0m 0s The patch has 4 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
          +1 xml 0m 6s The patch has no ill-formed XML file.
          +1 findbugs 5m 20s the patch passed
          -1 javadoc 0m 55s hadoop-common-project_hadoop-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
                Other Tests
          -1 unit 8m 6s hadoop-common in the patch failed.
          +1 unit 1m 25s hadoop-hdfs-client in the patch passed.
          -1 unit 65m 5s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 36s The patch does not generate ASF License warnings.
          164m 2s



          Reason Tests
          Failed junit tests hadoop.fs.TestFilterFileSystem
            hadoop.security.TestKDiag
            hadoop.fs.TestHarFileSystem
            hadoop.conf.TestCommonConfigurationFields
            hadoop.hdfs.TestDFSStripedOutputStreamWithFailure150



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:14b5c93
          JIRA Issue HADOOP-9565
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12867125/HADOOP-9565-010.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux 4b0e7cc14898 3.13.0-117-generic #164-Ubuntu SMP Fri Apr 7 11:05:26 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 024c3ec
          Default Java 1.8.0_131
          findbugs v3.1.0-RC1
          findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12967/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-client-warnings.html
          findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12967/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html
          javac https://builds.apache.org/job/PreCommit-HADOOP-Build/12967/artifact/patchprocess/diff-compile-javac-root.txt
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/12967/artifact/patchprocess/diff-checkstyle-root.txt
          whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/12967/artifact/patchprocess/whitespace-eol.txt
          javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/12967/artifact/patchprocess/diff-javadoc-javadoc-hadoop-common-project_hadoop-common.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12967/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12967/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12967/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12967/console
          Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 16s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 7 new or modified test files.       trunk Compile Tests 0 mvndep 0m 42s Maven dependency ordering for branch +1 mvninstall 13m 52s trunk passed +1 compile 13m 45s trunk passed +1 checkstyle 1m 57s trunk passed +1 mvnsite 3m 9s trunk passed -1 findbugs 1m 34s hadoop-hdfs-project/hadoop-hdfs-client in trunk has 2 extant Findbugs warnings. -1 findbugs 1m 47s hadoop-hdfs-project/hadoop-hdfs in trunk has 9 extant Findbugs warnings. +1 javadoc 2m 8s trunk passed       Patch Compile Tests 0 mvndep 0m 17s Maven dependency ordering for patch +1 mvninstall 2m 6s the patch passed +1 compile 10m 27s the patch passed -1 javac 10m 27s root generated 1 new + 1418 unchanged - 0 fixed = 1419 total (was 1418) -0 checkstyle 2m 3s root: The patch generated 9 new + 106 unchanged - 16 fixed = 115 total (was 122) +1 mvnsite 3m 13s the patch passed -1 whitespace 0m 0s The patch has 4 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply +1 xml 0m 6s The patch has no ill-formed XML file. +1 findbugs 5m 20s the patch passed -1 javadoc 0m 55s hadoop-common-project_hadoop-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)       Other Tests -1 unit 8m 6s hadoop-common in the patch failed. +1 unit 1m 25s hadoop-hdfs-client in the patch passed. -1 unit 65m 5s hadoop-hdfs in the patch failed. +1 asflicense 0m 36s The patch does not generate ASF License warnings. 164m 2s Reason Tests Failed junit tests hadoop.fs.TestFilterFileSystem   hadoop.security.TestKDiag   hadoop.fs.TestHarFileSystem   hadoop.conf.TestCommonConfigurationFields   hadoop.hdfs.TestDFSStripedOutputStreamWithFailure150 Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue HADOOP-9565 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12867125/HADOOP-9565-010.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 4b0e7cc14898 3.13.0-117-generic #164-Ubuntu SMP Fri Apr 7 11:05:26 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 024c3ec Default Java 1.8.0_131 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12967/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-client-warnings.html findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12967/artifact/patchprocess/branch-findbugs-hadoop-hdfs-project_hadoop-hdfs-warnings.html javac https://builds.apache.org/job/PreCommit-HADOOP-Build/12967/artifact/patchprocess/diff-compile-javac-root.txt checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/12967/artifact/patchprocess/diff-checkstyle-root.txt whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/12967/artifact/patchprocess/whitespace-eol.txt javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/12967/artifact/patchprocess/diff-javadoc-javadoc-hadoop-common-project_hadoop-common.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12967/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/12967/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12967/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12967/console Powered by Apache Yetus 0.6.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.

            People

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

              Dates

              • Created:
                Updated:

                Development