Details
-
Type:
Improvement
-
Status: Resolved
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: 2.8.0
-
Fix Version/s: 2.8.0, 3.0.0-alpha1
-
Component/s: hdfs-client
-
Labels:None
-
Target Version/s:
Description
We should add a pluggable interface for reading replicas in the DFSClient. This could be used to implement short-circuit reads on systems without file descriptors, or for other optimizations.
Issue Links
- is duplicated by
-
HDFS-7680
Support dataset-specific choice of short circuit implementation
-
- Open
-
Activity
- All
- Comments
- Work Log
- History
- Activity
- Transitions
Do you have any specific use cases in mind? Will creating a new BlockReader satisfy your use case?
One example is a storage appliance which wanted to create a custom short-circuit read implementation. Creating a new BlockReader does not satisfy this use-case because this block reader would have to have hardware-specific code which does not belong in upstream. For example, it would have dependencies on specific JNI and other libraries for interfacing with the hardware.
This patch is mostly proposing an interface, it looks good to me.
Only a few minor comments:
- Could you remove the change in ClientContext.java?
- Would ReplicaAccessor, ReplicaAccessorBuilder to be interface?
+1 after these being addressed.
 -1 overall |
| Vote | Subsystem | Runtime | Comment |
|---|---|---|---|
| 0 | pre-patch | 19m 42s | 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 1 new or modified test files. |
| -1 | javac | 7m 54s | The applied patch generated 1 additional warning messages. |
| +1 | javadoc | 9m 55s | There were no new javadoc warning messages. |
| +1 | release audit | 0m 25s | The applied patch does not increase the total number of release audit warnings. |
| -1 | checkstyle | 2m 29s | The applied patch generated 22 new checkstyle issues (total was 40, now 62). |
| +1 | whitespace | 0m 0s | The patch has no lines that end in whitespace. |
| +1 | install | 1m 41s | mvn install still works. |
| +1 | eclipse:eclipse | 0m 35s | The patch built with eclipse:eclipse. |
| +1 | findbugs | 4m 31s | The patch does not introduce any new Findbugs (version 3.0.0) warnings. |
| +1 | native | 3m 17s | Pre-build of native portion |
| -1 | hdfs tests | 203m 24s | Tests failed in hadoop-hdfs. |
| +1 | hdfs tests | 0m 28s | Tests passed in hadoop-hdfs-client. |
| 254m 26s |
| Reason | Tests |
|---|---|
| Failed unit tests | hadoop.hdfs.server.namenode.TestNameNodeMetricsLogger |
| hadoop.hdfs.server.namenode.TestFileTruncate | |
| Timed out tests | org.apache.hadoop.cli.TestHDFSCLI |
This message was automatically generated.
- Suppress javac warning about typecast when loading ReplicaAccesorBuilder class
- Remove unused import line in ReplicaAccessorBuilder and ReplicaAccessor
- Fix some lines which were longer than 80 characters
- Remove change to ClientContext.java
Would ReplicaAccessor, ReplicaAccessorBuilder to be interface?
Good question. They are deliberately abstract classes rather than interfaces, so that we can add more methods later if needed (as long as we provide a default implementation in the base class)
I've had catch (Throwable t) beaten out of me. Should these be Exception instead?
I haven't thought about it that much, but it seems like we could get a LinkageError (for example) when trying to instantiate an instance of a new class. This wouldn't be caught just by catching Exception. Loading a new class seems like a time when you might actually want to catch and log "the weird stuff".
| -1 overall |
| Vote | Subsystem | Runtime | Comment |
|---|---|---|---|
| 0 | pre-patch | 19m 21s | 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 1 new or modified test files. |
| +1 | javac | 7m 44s | There were no new javac warning messages. |
| +1 | javadoc | 9m 46s | There were no new javadoc warning messages. |
| +1 | release audit | 0m 22s | The applied patch does not increase the total number of release audit warnings. |
| -1 | checkstyle | 2m 33s | The applied patch generated 6 new checkstyle issues (total was 40, now 46). |
| +1 | whitespace | 0m 1s | The patch has no lines that end in whitespace. |
| +1 | install | 1m 35s | mvn install still works. |
| +1 | eclipse:eclipse | 0m 31s | The patch built with eclipse:eclipse. |
| +1 | findbugs | 4m 29s | The patch does not introduce any new Findbugs (version 3.0.0) warnings. |
| +1 | native | 3m 8s | Pre-build of native portion |
| +1 | hdfs tests | 163m 24s | Tests passed in hadoop-hdfs. |
| +1 | hdfs tests | 0m 29s | Tests passed in hadoop-hdfs-client. |
| 213m 30s |
| Subsystem | Report/Notes |
|---|---|
| Patch URL | http://issues.apache.org/jira/secure/attachment/12751509/HDFS-8924.002.patch |
| Optional Tests | javadoc javac unit findbugs checkstyle |
| git revision | trunk / 22de7c1 |
| checkstyle | https://builds.apache.org/job/PreCommit-HDFS-Build/12075/artifact/patchprocess/diffcheckstylehadoop-hdfs-client.txt |
| hadoop-hdfs test log | https://builds.apache.org/job/PreCommit-HDFS-Build/12075/artifact/patchprocess/testrun_hadoop-hdfs.txt |
| hadoop-hdfs-client test log | https://builds.apache.org/job/PreCommit-HDFS-Build/12075/artifact/patchprocess/testrun_hadoop-hdfs-client.txt |
| Test Results | https://builds.apache.org/job/PreCommit-HDFS-Build/12075/testReport/ |
| Java | 1.7.0_55 |
| uname | Linux asf906.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-HDFS-Build/12075/console |
This message was automatically generated.
+1. Will commit shortly.
Thanks a lot for working on this, Colin P. McCabe.
I committed this patch to trunk and branch-2.
FAILURE: Integrated in Hadoop-trunk-Commit #8336 (See https://builds.apache.org/job/Hadoop-trunk-Commit/8336/)
HDFS-8924. Add pluggable interface for reading replicas in DFSClient. (Colin Patrick McCabe via Lei Xu) (lei: rev 7087e700e032dabc174ecc12b62c12e7d49b995f)
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/ExternalBlockReader.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessorBuilder.java
- hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestExternalBlockReader.java
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/Op.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/impl/DfsClientConf.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessor.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/HdfsClientConfigKeys.java
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/BlockReaderFactory.java
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/Receiver.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/proto/datatransfer.proto
FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #292 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/292/)
HDFS-8924. Add pluggable interface for reading replicas in DFSClient. (Colin Patrick McCabe via Lei Xu) (lei: rev 7087e700e032dabc174ecc12b62c12e7d49b995f)
- hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestExternalBlockReader.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/proto/datatransfer.proto
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessor.java
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/Receiver.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessorBuilder.java
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/Op.java
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/BlockReaderFactory.java
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/ExternalBlockReader.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/HdfsClientConfigKeys.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/impl/DfsClientConf.java
FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #296 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/296/)
HDFS-8924. Add pluggable interface for reading replicas in DFSClient. (Colin Patrick McCabe via Lei Xu) (lei: rev 7087e700e032dabc174ecc12b62c12e7d49b995f)
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/HdfsClientConfigKeys.java
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/Receiver.java
- hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestExternalBlockReader.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessorBuilder.java
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/BlockReaderFactory.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/proto/datatransfer.proto
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/ExternalBlockReader.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessor.java
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/Op.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/impl/DfsClientConf.java
FAILURE: Integrated in Hadoop-Mapreduce-trunk #2241 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2241/)
HDFS-8924. Add pluggable interface for reading replicas in DFSClient. (Colin Patrick McCabe via Lei Xu) (lei: rev 7087e700e032dabc174ecc12b62c12e7d49b995f)
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/Receiver.java
- hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestExternalBlockReader.java
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/BlockReaderFactory.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/proto/datatransfer.proto
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessorBuilder.java
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/ExternalBlockReader.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessor.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/HdfsClientConfigKeys.java
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/Op.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/impl/DfsClientConf.java
SUCCESS: Integrated in Hadoop-Yarn-trunk #1026 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/1026/)
HDFS-8924. Add pluggable interface for reading replicas in DFSClient. (Colin Patrick McCabe via Lei Xu) (lei: rev 7087e700e032dabc174ecc12b62c12e7d49b995f)
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/ExternalBlockReader.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessorBuilder.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/impl/DfsClientConf.java
- hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestExternalBlockReader.java
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/BlockReaderFactory.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/proto/datatransfer.proto
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/Op.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessor.java
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/Receiver.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/HdfsClientConfigKeys.java
FAILURE: Integrated in Hadoop-Hdfs-trunk-Java8 #285 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/285/)
HDFS-8924. Add pluggable interface for reading replicas in DFSClient. (Colin Patrick McCabe via Lei Xu) (lei: rev 7087e700e032dabc174ecc12b62c12e7d49b995f)
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/proto/datatransfer.proto
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/impl/DfsClientConf.java
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/Receiver.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessor.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/HdfsClientConfigKeys.java
- hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestExternalBlockReader.java
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/Op.java
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/BlockReaderFactory.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessorBuilder.java
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/ExternalBlockReader.java
FAILURE: Integrated in Hadoop-Hdfs-trunk #2223 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2223/)
HDFS-8924. Add pluggable interface for reading replicas in DFSClient. (Colin Patrick McCabe via Lei Xu) (lei: rev 7087e700e032dabc174ecc12b62c12e7d49b995f)
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/proto/datatransfer.proto
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/Op.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessor.java
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/BlockReaderFactory.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/HdfsClientConfigKeys.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessorBuilder.java
- hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestExternalBlockReader.java
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/Receiver.java
- hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/ExternalBlockReader.java
- hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/impl/DfsClientConf.java
This patch adds a pluggable ReplicaAccessorBuilder class which can be used to create ReplicaAccessor objects. Unlike BlockReader, ReplicaAccessor is a stable API which is decoupled from internal implementation details and non-public classes. BlockReaderFactory will ask all of the configured ReplicaAccessorBuilder objects to create a new ReplicaAccessor. If none are configured, or none can create one, we use the existing block reader code. Otherwise, we create an ExternalBlockReader wrapping the ReplicaAccessor. I also added a reserved DataTransferProtocol opcode (127) in Op.java. This will ensure that anyone adding a custom opcode will not conflict with other new opcodes added upstream.