Uploaded image for project: 'Hadoop HDFS'
  1. Hadoop HDFS
  2. HDFS-13403

libhdfs++: Use hdfs::IoService object rather than asio::io_service

Details

    • Task
    • Status: Resolved
    • Critical
    • Resolution: Fixed
    • None
    • 3.2.0, 3.3.0
    • None
    • None

    Description

      At the moment the hdfs::IoService is a simple wrapper over asio's io_service object. I'd like to make this smarter and have it do things like track which tasks are queued, validate that dependencies of tasks exist, and monitor ioservice throughput and contention. In order to get there we need to use have all components in the library to go through the hdfs::IoService rather than directly interacting with the asio::io_service. The only time the asio::io_service should be used is when calling things like asio::async_write that need an io_service&. HDFS-11884 will be able get rid of those remaining instances once this work is in place.

      Attachments

        1. HDFS-13403.000.patch
          93 kB
          James Clampffer
        2. build_fixes.patch
          3 kB
          James Clampffer

        Issue Links

          Activity

            Attached a patch. Not a whole lot going on other than making uses of asio::io_service in arguments and member variables instances of shared_ptr<hdfs::IoService>.

            Since this already touched a lot of files I cleaned up a few things as I went:
            -Clear out includes in headers that weren't necessary.
            -Pull a few method implementations out of header files.
            -Rename the MutableBuffers typedef to MutableBuffer to avoid confusion with BufferSequences which aren't supported in the FileSystem API yet.

            James C James Clampffer added a comment - Attached a patch. Not a whole lot going on other than making uses of asio::io_service in arguments and member variables instances of shared_ptr<hdfs::IoService>. Since this already touched a lot of files I cleaned up a few things as I went: -Clear out includes in headers that weren't necessary. -Pull a few method implementations out of header files. -Rename the MutableBuffers typedef to MutableBuffer to avoid confusion with BufferSequences which aren't supported in the FileSystem API yet.
            genericqa genericqa added a comment -
            -1 overall



            Vote Subsystem Runtime Comment
            0 reexec 0m 35s Docker mode activated.
                  Prechecks
            +1 @author 0m 0s The patch does not contain any @author tags.
            -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
                  trunk Compile Tests
            +1 mvninstall 26m 5s trunk passed
            +1 compile 15m 28s trunk passed
            +1 mvnsite 7m 57s trunk passed
            +1 shadedclient 59m 36s branch has no errors when building and testing our client artifacts.
                  Patch Compile Tests
            +1 mvninstall 7m 46s the patch passed
            +1 compile 15m 25s the patch passed
            +1 cc 15m 25s the patch passed
            +1 javac 15m 25s the patch passed
            +1 mvnsite 7m 38s the patch passed
            +1 whitespace 0m 0s The patch has no whitespace issues.
            +1 shadedclient 11m 59s patch has no errors when building and testing our client artifacts.
                  Other Tests
            +1 unit 21m 29s hadoop-hdfs-native-client in the patch passed.
            +1 asflicense 0m 23s The patch does not generate ASF License warnings.
            125m 57s



            Subsystem Report/Notes
            Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8620d2b
            JIRA Issue HDFS-13403
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12918183/HDFS-13403.000.patch
            Optional Tests asflicense compile cc mvnsite javac unit
            uname Linux 725a072e8a3c 3.13.0-143-generic #192-Ubuntu SMP Tue Feb 27 10:45:36 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
            Build tool maven
            Personality /testptch/patchprocess/precommit/personality/provided.sh
            git revision trunk / ac32b35
            maven version: Apache Maven 3.3.9
            Default Java 1.8.0_162
            Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/23840/testReport/
            Max. process+thread count 361 (vs. ulimit of 10000)
            modules C: hadoop-hdfs-project/hadoop-hdfs-native-client U: hadoop-hdfs-project/hadoop-hdfs-native-client
            Console output https://builds.apache.org/job/PreCommit-HDFS-Build/23840/console
            Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org

            This message was automatically generated.

            genericqa genericqa added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 35s Docker mode activated.       Prechecks +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.       trunk Compile Tests +1 mvninstall 26m 5s trunk passed +1 compile 15m 28s trunk passed +1 mvnsite 7m 57s trunk passed +1 shadedclient 59m 36s branch has no errors when building and testing our client artifacts.       Patch Compile Tests +1 mvninstall 7m 46s the patch passed +1 compile 15m 25s the patch passed +1 cc 15m 25s the patch passed +1 javac 15m 25s the patch passed +1 mvnsite 7m 38s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 shadedclient 11m 59s patch has no errors when building and testing our client artifacts.       Other Tests +1 unit 21m 29s hadoop-hdfs-native-client in the patch passed. +1 asflicense 0m 23s The patch does not generate ASF License warnings. 125m 57s Subsystem Report/Notes Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hadoop:8620d2b JIRA Issue HDFS-13403 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12918183/HDFS-13403.000.patch Optional Tests asflicense compile cc mvnsite javac unit uname Linux 725a072e8a3c 3.13.0-143-generic #192-Ubuntu SMP Tue Feb 27 10:45:36 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/patchprocess/precommit/personality/provided.sh git revision trunk / ac32b35 maven version: Apache Maven 3.3.9 Default Java 1.8.0_162 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/23840/testReport/ Max. process+thread count 361 (vs. ulimit of 10000) modules C: hadoop-hdfs-project/hadoop-hdfs-native-client U: hadoop-hdfs-project/hadoop-hdfs-native-client Console output https://builds.apache.org/job/PreCommit-HDFS-Build/23840/console Powered by Apache Yetus 0.8.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
            mtracy Mitchell Tracy added a comment -

            +1 on HDFS-13403.000.patch

            mtracy Mitchell Tracy added a comment - +1 on HDFS-13403 .000.patch

            Thanks for the review mtracy! I've committed this to trunk.

            James C James Clampffer added a comment - Thanks for the review mtracy ! I've committed this to trunk.
            hudson Hudson added a comment -

            FAILURE: Integrated in Jenkins build Hadoop-trunk-Commit #13969 (See https://builds.apache.org/job/Hadoop-trunk-Commit/13969/)
            HDFS-13403: libhdfs++ Use hdfs::IoService object rather than (jhc: rev eefe2a147c83dbb62c4021b67d59d3b9f065f890)

            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/reader/block_reader.h
            • (add) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/ioservice_impl.h
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/rpc/rpc_connection.h
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/include/hdfspp/hdfspp.h
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/util.cc
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/rpc/rpc_connection_impl.cc
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/rpc/namenode_tracker.cc
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/rpc/rpc_engine.h
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/bindings/c/hdfs.cc
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/fs/filesystem.h
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/bad_datanode_test.cc
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/mock_connection.h
            • (delete) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/hdfs_ioservice.h
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/fs/filehandle.cc
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/namenode_info.h
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/fs/filehandle.h
            • (add) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/include/hdfspp/ioservice.h
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/async_stream.h
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/fs/namenode_operations.h
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/reader/block_reader.cc
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/connection/datanodeconnection.h
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/reader/datatransfer.h
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/CMakeLists.txt
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/rpc/request.cc
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/namenode_info.cc
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/hdfs_ioservice_test.cc
            • (add) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/ioservice_impl.cc
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/continuation/asio.h
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/rpc/rpc_connection_impl.h
            • (delete) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/hdfs_ioservice.cc
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/util.h
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/fs/filesystem.cc
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/rpc_engine_test.cc
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/rpc/rpc_engine.cc
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/logging.h
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/rpc/namenode_tracker.h
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/remote_block_reader_test.cc
            • (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/connection/datanodeconnection.cc
            hudson Hudson added a comment - FAILURE: Integrated in Jenkins build Hadoop-trunk-Commit #13969 (See https://builds.apache.org/job/Hadoop-trunk-Commit/13969/ ) HDFS-13403 : libhdfs++ Use hdfs::IoService object rather than (jhc: rev eefe2a147c83dbb62c4021b67d59d3b9f065f890) (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/reader/block_reader.h (add) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/ioservice_impl.h (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/rpc/rpc_connection.h (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/include/hdfspp/hdfspp.h (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/util.cc (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/rpc/rpc_connection_impl.cc (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/rpc/namenode_tracker.cc (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/rpc/rpc_engine.h (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/bindings/c/hdfs.cc (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/fs/filesystem.h (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/bad_datanode_test.cc (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/mock_connection.h (delete) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/hdfs_ioservice.h (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/fs/filehandle.cc (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/namenode_info.h (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/fs/filehandle.h (add) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/include/hdfspp/ioservice.h (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/async_stream.h (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/fs/namenode_operations.h (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/reader/block_reader.cc (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/connection/datanodeconnection.h (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/reader/datatransfer.h (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/CMakeLists.txt (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/rpc/request.cc (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/namenode_info.cc (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/hdfs_ioservice_test.cc (add) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/ioservice_impl.cc (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/continuation/asio.h (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/rpc/rpc_connection_impl.h (delete) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/hdfs_ioservice.cc (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/util.h (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/fs/filesystem.cc (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/rpc_engine_test.cc (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/rpc/rpc_engine.cc (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/logging.h (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/rpc/namenode_tracker.h (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/remote_block_reader_test.cc (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/connection/datanodeconnection.cc

            Hello James C,

            I have run into an error when I tried to build the project on my Mac with `mvn clean install -Pnative -DskipTests`. The compile failed at ioservice.h with the following error:

            {{ [exec] [ 30%] Building CXX object main/native/libhdfspp/lib/common/CMakeFiles/common_obj.dir/ioservice_impl.cc.o}}
            {{ [exec] In file included from /Users/xxxx/IdeaProjects/hadoop-apache/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/ioservice_impl.cc:19:}}
            {{ [exec] In file included from /Users/xxxx/IdeaProjects/hadoop-apache/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/ioservice_impl.h:22:}}
            {{ [exec] /Users/xxxx/IdeaProjects/hadoop-apache/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/include/hdfspp/ioservice.h:109:30: error: no template named 'function' in namespace 'std'}}
            {{ [exec] virtual void PostTask(std::function<void(void)> asyncTask) = 0;}}
            {{ [exec] ~~~~~^}}
            {{ }}

             

            There are further compile errors as well, if needed I can provide a full output.

            After adding #include <functional> to the ioservice.h the error goes away. I am not sure if it is something in my environment, but the Jenkins build failed somewhere else, though as I see it have skipped the Apache Hadoop HDFS Native Client package.

            pifta István Fajth added a comment - Hello James C , I have run into an error when I tried to build the project on my Mac with `mvn clean install -Pnative -DskipTests`. The compile failed at ioservice.h with the following error: {{ [exec] [ 30%] Building CXX object main/native/libhdfspp/lib/common/CMakeFiles/common_obj.dir/ioservice_impl.cc.o}} {{ [exec] In file included from /Users/xxxx/IdeaProjects/hadoop-apache/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/ioservice_impl.cc:19:}} {{ [exec] In file included from /Users/xxxx/IdeaProjects/hadoop-apache/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/ioservice_impl.h:22:}} {{ [exec] /Users/xxxx/IdeaProjects/hadoop-apache/hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/include/hdfspp/ioservice.h:109:30: error: no template named 'function' in namespace 'std'}} {{ [exec] virtual void PostTask(std::function<void(void)> asyncTask) = 0;}} {{ [exec] ~~~~~^}} {{ }}   There are further compile errors as well, if needed I can provide a full output. After adding #include <functional> to the ioservice.h the error goes away. I am not sure if it is something in my environment, but the Jenkins build failed somewhere else, though as I see it have skipped the Apache Hadoop HDFS Native Client package.

            Hi pifta

            Thanks for pointing this out! I'm not sure why the missing include isn't causing issues when I build on GCC and Clang in the docker container but functional should be included in ioservice.h. I do see some errors in the clang build due to -Winconsistent-missing-override that don't show up when using GCC. Are those what you're seeing? I'll get patch up with the include and virtual override warning fixes tomorrow.

            The post-commit jenkins build bailed out before it hit hadoop_hdfs_native_client so I don't think that's related.

            James C James Clampffer added a comment - Hi pifta Thanks for pointing this out! I'm not sure why the missing include isn't causing issues when I build on GCC and Clang in the docker container but functional should be included in ioservice.h. I do see some errors in the clang build due to -Winconsistent-missing-override that don't show up when using GCC. Are those what you're seeing? I'll get patch up with the include and virtual override warning fixes tomorrow. The post-commit jenkins build bailed out before it hit hadoop_hdfs_native_client so I don't think that's related.

            Hi James C,

            yes, those are one type of the warnings, also I see warnings due to -Wunused-lambda-capture though after checking a few instance of this warning, they not seems to be related to this patch, though I just vaguely checked, but most likely they were there before as well.

            With the patch applied, I do not see inconsistent-missing-override warnings, and the include helped as well to get over the compile error.

            pifta István Fajth added a comment - Hi James C , yes, those are one type of the warnings, also I see warnings due to -Wunused-lambda-capture though after checking a few instance of this warning, they not seems to be related to this patch, though I just vaguely checked, but most likely they were there before as well. With the patch applied, I do not see inconsistent-missing-override warnings, and the include helped as well to get over the compile error.

            Hi James C,

            I am wondering if the build_fixes.patch can be committed to the trunk? As I see it is still missing, I just ran into it again

            Thank you in advance!

            pifta István Fajth added a comment - Hi James C , I am wondering if the build_fixes.patch can be committed to the trunk? As I see it is still missing, I just ran into it again Thank you in advance!
            bibinchundatt Bibin Chundatt added a comment -

            James C

            For version gcc 7 native compilation fails

            https://bugzilla.redhat.com/show_bug.cgi?id=1417383

            For reference, the <future>, <mutex>, and <regex> headers used to include the whole of <functional> (thousands of lines), but now they don't. This is a Good Thing™.
            
            I'll make a note of this in the GCC 7 "porting to" doc.
            

            Seems we have to explicitly include <functional>

            bibinchundatt Bibin Chundatt added a comment - James C For version gcc 7 native compilation fails https://bugzilla.redhat.com/show_bug.cgi?id=1417383 For reference, the < future >, <mutex>, and <regex> headers used to include the whole of <functional> (thousands of lines), but now they don't. This is a Good Thing™. I'll make a note of this in the GCC 7 "porting to" doc. Seems we have to explicitly include <functional>

            Thanks for finding that bibinchundatt! pifta I posted a patch on HDFS-13534 to fix this if you'd like to review.

            James C James Clampffer added a comment - Thanks for finding that bibinchundatt ! pifta I posted a patch on HDFS-13534 to fix this if you'd like to review.
            sunilg Sunil G added a comment -

            Hi James C pls help to check the Fixed Version.

            sunilg Sunil G added a comment - Hi James C pls help to check the Fixed Version.

            People

              James C James Clampffer
              James C James Clampffer
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: