Uploaded image for project: 'Hadoop YARN'
  1. Hadoop YARN
  2. YARN-2246

Job History Link in RM UI is redirecting to the URL which contains Job Id twice

    Details

    • Target Version/s:
    • Hadoop Flags:
      Reviewed

      Description

      http://xx.x.x.x:19888/jobhistory/job/job_1332435449546_0001/jobhistory/job/job_1332435449546_0001
      
      1. YARN-2246-4.patch
        6 kB
        Devaraj K
      2. YARN-2246-3.patch
        4 kB
        Devaraj K
      3. YARN-2246.patch
        5 kB
        Devaraj K
      4. YARN-2246.2.patch
        2 kB
        Zhijie Shen
      5. MAPREDUCE-4064-1.patch
        7 kB
        Devaraj K
      6. MAPREDUCE-4064.patch
        6 kB
        Devaraj K

        Activity

        Hide
        vinodkv Vinod Kumar Vavilapalli added a comment -

        Pulled this into 2.6.1. Ran compilation and TestRMAppAttemptTransitions before the push. Patch applied cleanly.

        Show
        vinodkv Vinod Kumar Vavilapalli added a comment - Pulled this into 2.6.1. Ran compilation and TestRMAppAttemptTransitions before the push. Patch applied cleanly.
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Hadoop-Hdfs-trunk-Java8 #97 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/97/)
        YARN-2246. Made the proxy tracking URL always be http(s)://proxy addr:port/proxy/<appId> to avoid duplicate sections. Contributed by Devaraj K. (zjshen: rev d5855c0e46404cfc1b5a63e59015e68ba668f0ea)

        • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/TestRMAppAttemptTransitions.java
        • hadoop-yarn-project/CHANGES.txt
        • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptImpl.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-Hdfs-trunk-Java8 #97 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Java8/97/ ) YARN-2246 . Made the proxy tracking URL always be http(s)://proxy addr:port/proxy/<appId> to avoid duplicate sections. Contributed by Devaraj K. (zjshen: rev d5855c0e46404cfc1b5a63e59015e68ba668f0ea) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/TestRMAppAttemptTransitions.java hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptImpl.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk #2052 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2052/)
        YARN-2246. Made the proxy tracking URL always be http(s)://proxy addr:port/proxy/<appId> to avoid duplicate sections. Contributed by Devaraj K. (zjshen: rev d5855c0e46404cfc1b5a63e59015e68ba668f0ea)

        • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptImpl.java
        • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/TestRMAppAttemptTransitions.java
        • hadoop-yarn-project/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #2052 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/2052/ ) YARN-2246 . Made the proxy tracking URL always be http(s)://proxy addr:port/proxy/<appId> to avoid duplicate sections. Contributed by Devaraj K. (zjshen: rev d5855c0e46404cfc1b5a63e59015e68ba668f0ea) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptImpl.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/TestRMAppAttemptTransitions.java hadoop-yarn-project/CHANGES.txt
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #102 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/102/)
        YARN-2246. Made the proxy tracking URL always be http(s)://proxy addr:port/proxy/<appId> to avoid duplicate sections. Contributed by Devaraj K. (zjshen: rev d5855c0e46404cfc1b5a63e59015e68ba668f0ea)

        • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptImpl.java
        • hadoop-yarn-project/CHANGES.txt
        • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/TestRMAppAttemptTransitions.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk-Java8 #102 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Java8/102/ ) YARN-2246 . Made the proxy tracking URL always be http(s)://proxy addr:port/proxy/<appId> to avoid duplicate sections. Contributed by Devaraj K. (zjshen: rev d5855c0e46404cfc1b5a63e59015e68ba668f0ea) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptImpl.java hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/TestRMAppAttemptTransitions.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Hdfs-trunk #2033 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2033/)
        YARN-2246. Made the proxy tracking URL always be http(s)://proxy addr:port/proxy/<appId> to avoid duplicate sections. Contributed by Devaraj K. (zjshen: rev d5855c0e46404cfc1b5a63e59015e68ba668f0ea)

        • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/TestRMAppAttemptTransitions.java
        • hadoop-yarn-project/CHANGES.txt
        • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptImpl.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Hdfs-trunk #2033 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/2033/ ) YARN-2246 . Made the proxy tracking URL always be http(s)://proxy addr:port/proxy/<appId> to avoid duplicate sections. Contributed by Devaraj K. (zjshen: rev d5855c0e46404cfc1b5a63e59015e68ba668f0ea) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/TestRMAppAttemptTransitions.java hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptImpl.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Yarn-trunk #835 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/835/)
        YARN-2246. Made the proxy tracking URL always be http(s)://proxy addr:port/proxy/<appId> to avoid duplicate sections. Contributed by Devaraj K. (zjshen: rev d5855c0e46404cfc1b5a63e59015e68ba668f0ea)

        • hadoop-yarn-project/CHANGES.txt
        • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptImpl.java
        • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/TestRMAppAttemptTransitions.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk #835 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/835/ ) YARN-2246 . Made the proxy tracking URL always be http(s)://proxy addr:port/proxy/<appId> to avoid duplicate sections. Contributed by Devaraj K. (zjshen: rev d5855c0e46404cfc1b5a63e59015e68ba668f0ea) hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptImpl.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/TestRMAppAttemptTransitions.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #101 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/101/)
        YARN-2246. Made the proxy tracking URL always be http(s)://proxy addr:port/proxy/<appId> to avoid duplicate sections. Contributed by Devaraj K. (zjshen: rev d5855c0e46404cfc1b5a63e59015e68ba668f0ea)

        • hadoop-yarn-project/CHANGES.txt
        • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptImpl.java
        • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/TestRMAppAttemptTransitions.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk-Java8 #101 (See https://builds.apache.org/job/Hadoop-Yarn-trunk-Java8/101/ ) YARN-2246 . Made the proxy tracking URL always be http(s)://proxy addr:port/proxy/<appId> to avoid duplicate sections. Contributed by Devaraj K. (zjshen: rev d5855c0e46404cfc1b5a63e59015e68ba668f0ea) hadoop-yarn-project/CHANGES.txt hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptImpl.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/TestRMAppAttemptTransitions.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in Hadoop-trunk-Commit #7065 (See https://builds.apache.org/job/Hadoop-trunk-Commit/7065/)
        YARN-2246. Made the proxy tracking URL always be http(s)://proxy addr:port/proxy/<appId> to avoid duplicate sections. Contributed by Devaraj K. (zjshen: rev d5855c0e46404cfc1b5a63e59015e68ba668f0ea)

        • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptImpl.java
        • hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/TestRMAppAttemptTransitions.java
        • hadoop-yarn-project/CHANGES.txt
        Show
        hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #7065 (See https://builds.apache.org/job/Hadoop-trunk-Commit/7065/ ) YARN-2246 . Made the proxy tracking URL always be http(s)://proxy addr:port/proxy/<appId> to avoid duplicate sections. Contributed by Devaraj K. (zjshen: rev d5855c0e46404cfc1b5a63e59015e68ba668f0ea) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptImpl.java hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/TestRMAppAttemptTransitions.java hadoop-yarn-project/CHANGES.txt
        Hide
        zjshen Zhijie Shen added a comment -

        Committed to trunk and branch-2. Thanks Devaraj for the patch, and Jason and Jonathan for the review!

        Show
        zjshen Zhijie Shen added a comment - Committed to trunk and branch-2. Thanks Devaraj for the patch, and Jason and Jonathan for the review!
        Hide
        jlowe Jason Lowe added a comment -

        +1 lgtm. Feel free to commit.

        Show
        jlowe Jason Lowe added a comment - +1 lgtm. Feel free to commit.
        Hide
        zjshen Zhijie Shen added a comment -

        Thanks for the confirmation, Jonathan! I'll commit the patch a bit later to give Jason sometime to look at it too.

        Show
        zjshen Zhijie Shen added a comment - Thanks for the confirmation, Jonathan! I'll commit the patch a bit later to give Jason sometime to look at it too.
        Hide
        jeagles Jonathan Eagles added a comment -

        I think this is going to fix my issue.

        Show
        jeagles Jonathan Eagles added a comment - I think this is going to fix my issue.
        Hide
        devaraj.k Devaraj K added a comment -
        org.apache.hadoop.yarn.server.resourcemanager.TestRM.testNMTokenSentForNormalContainer[1]
        
        Failing for the past 1 build (Since Failed#6580 )
        Took 20 sec.
        Error Message
        
        test timed out after 20000 milliseconds
        

        This test failure is unrelated to the patch. It passes in my local.

        Show
        devaraj.k Devaraj K added a comment - org.apache.hadoop.yarn.server.resourcemanager.TestRM.testNMTokenSentForNormalContainer[1] Failing for the past 1 build (Since Failed#6580 ) Took 20 sec. Error Message test timed out after 20000 milliseconds This test failure is unrelated to the patch. It passes in my local.
        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/12697819/YARN-2246-4.patch
        against trunk revision 3f5431a.

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

        +1 tests included. The patch appears to include 1 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-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager:

        org.apache.hadoop.yarn.server.resourcemanager.TestRM

        Test results: https://builds.apache.org/job/PreCommit-YARN-Build/6580//testReport/
        Console output: https://builds.apache.org/job/PreCommit-YARN-Build/6580//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/12697819/YARN-2246-4.patch against trunk revision 3f5431a. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 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-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager: org.apache.hadoop.yarn.server.resourcemanager.TestRM Test results: https://builds.apache.org/job/PreCommit-YARN-Build/6580//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/6580//console This message is automatically generated.
        Hide
        zjshen Zhijie Shen added a comment -

        Correct, but that's MapReduce's fault and not YARN's.

        Agree, we may want to file a separate MR jira for this issue.

        Am I missing something?

        I think your inference is right.

        Thanks for the patch, Devaraj! It looks good to me. Jonathan Eagles, from the perspective of TEZ, is it going to fix the history URL?

        Show
        zjshen Zhijie Shen added a comment - Correct, but that's MapReduce's fault and not YARN's. Agree, we may want to file a separate MR jira for this issue. Am I missing something? I think your inference is right. Thanks for the patch, Devaraj! It looks good to me. Jonathan Eagles , from the perspective of TEZ, is it going to fix the history URL?
        Hide
        devaraj.k Devaraj K added a comment -

        I have updated the patch with test changes.

        Show
        devaraj.k Devaraj K added a comment - I have updated the patch with test changes.
        Hide
        devaraj.k Devaraj K added a comment -

        Thanks Jason Lowe for looking into the patch and confirming the approach. I will update the patch with the tests.

        Show
        devaraj.k Devaraj K added a comment - Thanks Jason Lowe for looking into the patch and confirming the approach. I will update the patch with the tests.
        Hide
        jlowe Jason Lowe added a comment -

        Do you suggest that we should use the original tracking url directly instead of proxy url on the web UI?

        Not sure if it's always OK to use the raw history tracking URL, as there might be some setups where the client can reach the RM but can't reach the history tracking URL directly. However I think it's OK to always have the RM advertise the original proxy tracking URL (i.e.: http://<rmaddr>/proxy/<appid>) to clients through its UI. The AM can redefine what that proxy redirects to, but the RM should never tack on paths to that proxy URI when advertising it. In other words, clients visiting http://<rmaddr>/proxy<appid> will always reach the UI (either AM or history), and if the AM and history have properly mirrored UIs then it should be seamless to transition between the two when the AM unregisters and redefines the tracking URL to point to the history server.

        seamless view between the AM UI and history UI is not possible nowadays.

        Correct, but that's MapReduce's fault and not YARN's. If the RM handles the proxy properly then it should be possible for an app framework to implement a properly mirrored UI between the AM and the history server.

        In general, seamless view will still be difficult with the aforementioned solution between two tracking URLs. For example, tracking URL is http://t1:p1/a/b first, and I'm visiting the path at http://t1:p1/a/b/x/y/z. When the tracking URL becomes http://t2:p2/c/d/e, I refresh the package and am redirected http://t2:p2/c/d/e/a/b/x/y/z. Without mapping between original tracking url and proxy url, we don't know /a/b is part of tracking url base, and it shouldn't be carried on.

        Not sure I'm following the example because there's no proxy URLs in it. The client should always be using the proxy URL for this discussion. If I follow the example correctly, the original tracking URL is http://t1:p1/a/b, and the proxy URL is rooted there (i.e.: proxy/<appid> -> t1:p1/a/b). So I'm visiting proxy/<appid>/x/y/z and then the AM unregisters with a new tracking URL of t2:p2/c/d/e. Then the proxy servlet should redirect that same proxy/<appid>/x/y/z request to t2:p2/c/d/e/x/y/z which seems correct to me. It's just taking the path underneath the proxy address (i.e.: everything after proxy/<appid>) and tacking it on the specified tracking URL. The same subpath is seen by both the AM and history URIs, assuming a/b is the root of the AM UI and c/d/e is the root of the history UI (for that app). So it seems this works as I would expect. Am I missing something?

        I have updated the generateProxyUriWithScheme() in the latest patch.

        Thanks for updating the patch, Devaraj. I think it looks good, although it would be nice to have some regression tests to verify that if the app changes the tracking URL that the proxy URL doesn't update like it used to.

        Show
        jlowe Jason Lowe added a comment - Do you suggest that we should use the original tracking url directly instead of proxy url on the web UI? Not sure if it's always OK to use the raw history tracking URL, as there might be some setups where the client can reach the RM but can't reach the history tracking URL directly. However I think it's OK to always have the RM advertise the original proxy tracking URL (i.e.: http://<rmaddr>/proxy/<appid>) to clients through its UI. The AM can redefine what that proxy redirects to, but the RM should never tack on paths to that proxy URI when advertising it. In other words, clients visiting http://<rmaddr>/proxy<appid> will always reach the UI (either AM or history), and if the AM and history have properly mirrored UIs then it should be seamless to transition between the two when the AM unregisters and redefines the tracking URL to point to the history server. seamless view between the AM UI and history UI is not possible nowadays. Correct, but that's MapReduce's fault and not YARN's. If the RM handles the proxy properly then it should be possible for an app framework to implement a properly mirrored UI between the AM and the history server. In general, seamless view will still be difficult with the aforementioned solution between two tracking URLs. For example, tracking URL is http://t1:p1/a/b first, and I'm visiting the path at http://t1:p1/a/b/x/y/z . When the tracking URL becomes http://t2:p2/c/d/e , I refresh the package and am redirected http://t2:p2/c/d/e/a/b/x/y/z . Without mapping between original tracking url and proxy url, we don't know /a/b is part of tracking url base, and it shouldn't be carried on. Not sure I'm following the example because there's no proxy URLs in it. The client should always be using the proxy URL for this discussion. If I follow the example correctly, the original tracking URL is http://t1:p1/a/b , and the proxy URL is rooted there (i.e.: proxy/<appid> -> t1:p1/a/b). So I'm visiting proxy/<appid>/x/y/z and then the AM unregisters with a new tracking URL of t2:p2/c/d/e. Then the proxy servlet should redirect that same proxy/<appid>/x/y/z request to t2:p2/c/d/e/x/y/z which seems correct to me. It's just taking the path underneath the proxy address (i.e.: everything after proxy/<appid>) and tacking it on the specified tracking URL. The same subpath is seen by both the AM and history URIs, assuming a/b is the root of the AM UI and c/d/e is the root of the history UI (for that app). So it seems this works as I would expect. Am I missing something? I have updated the generateProxyUriWithScheme() in the latest patch. Thanks for updating the patch, Devaraj. I think it looks good, although it would be nice to have some regression tests to verify that if the app changes the tracking URL that the proxy URL doesn't update like it used to.
        Hide
        devaraj.k Devaraj K added a comment -

        I have updated the generateProxyUriWithScheme() in the latest patch.

        Show
        devaraj.k Devaraj K added a comment - I have updated the generateProxyUriWithScheme() in the latest patch.
        Hide
        devaraj.k Devaraj K added a comment -

        I agree that this needs to be handled in RMAttemptImpl before exposing the proxy URL to the users.

        Thanks for your patch Zhijie Shen. I have tried this patch, it works fine. I think generateProxyUriWithScheme() can be updated according the patch changes.

        Show
        devaraj.k Devaraj K added a comment - I agree that this needs to be handled in RMAttemptImpl before exposing the proxy URL to the users. Thanks for your patch Zhijie Shen . I have tried this patch, it works fine. I think generateProxyUriWithScheme() can be updated according the patch changes.
        Hide
        zjshen Zhijie Shen added a comment -

        It aims to solve the non-root path. Root path works fine even currently.

        http://tezui.example.com:4080/tez/#/?appid=application_1423069770246_0026

        In this case, the original url will be http://tezui.example.com:4080/tez/#/?appid=application_1423069770246_0026, and proxy url will be http://rm addr:port/proxy/application_1423069770246_0026. When clicking the proxy url, WebAppProxyServlet will translate it into the given original url. I think it should work fine. The problem is with http://rm addr:port/proxy/application_1423069770246_0026/x/y/z. The query will appear before x/y/z, then. But it should also be wrong with current translation way.

        Show
        zjshen Zhijie Shen added a comment - It aims to solve the non-root path. Root path works fine even currently. http://tezui.example.com:4080/tez/#/?appid=application_1423069770246_0026 In this case, the original url will be http://tezui.example.com:4080/tez/#/?appid=application_1423069770246_0026 , and proxy url will be http://rm addr:port/proxy/application_1423069770246_0026 . When clicking the proxy url, WebAppProxyServlet will translate it into the given original url. I think it should work fine. The problem is with http://rm addr:port/proxy/application_1423069770246_0026/x/y/z . The query will appear before x/y/z, then. But it should also be wrong with current translation way.
        Show
        jeagles Jonathan Eagles added a comment - Zhijie Shen Will a tracking url with a non-root path work with this patch? http://tezui.example.com:4080/tez/#/?appid=application_1423069770246_0026 The current behavior is to redirect to an address with extra "tez" path. http://tezui.example.com:4080/tez/tez/#/?appid=application_1423069770246_0026 Source https://issues.apache.org/jira/browse/TEZ-2018?focusedCommentId=14308028&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14308028
        Hide
        zjshen Zhijie Shen added a comment - - edited

        In fact, it seems we don't need to fix RMAppAttemptImpl#generateProxyUriWithScheme, and even don't need to update the proxy tracking url at all when registering and unregistering. At different stage of the app, it will be translated to different given original tracking url or "unassigned". Make simple patch. I've tried it locally and it seemed to work fine. Jason Lowe, would you mind taking a look?

        Show
        zjshen Zhijie Shen added a comment - - edited In fact, it seems we don't need to fix RMAppAttemptImpl#generateProxyUriWithScheme, and even don't need to update the proxy tracking url at all when registering and unregistering. At different stage of the app, it will be translated to different given original tracking url or "unassigned". Make simple patch. I've tried it locally and it seemed to work fine. Jason Lowe , would you mind taking a look?
        Hide
        zjshen Zhijie Shen added a comment -

        Jason Lowe, I think we're on the same page about what's reason of duplicate section of the tracking url.

        RMAppAttemptImpl is the one tacking on the extra, redundant portion based on the path the AM unregistered.

        Are you suggesting no matter how many sections after authority in the track url like http(s)://tracking authority/S1/Sk/.../Sk, we always generates the proxy url like http(s)://proxy authority/proxy/<appId>?

        Do you suggest that we should use the original tracking url directly instead of proxy url on the web UI? I think this should work and we may simply change the code in RMAppAttemptImpl#generateProxyUriWithScheme:

              URI result = ProxyUriUtils.getProxyUri(null, proxyUri,
                  applicationAttemptId.getApplicationId());
        

        I believe it will be translated properly, as it works today

        Maybe I was not clear before. I didn't meant MR AM UI will not work, but seamless view between the AM UI and history UI is not possible nowadays. For example, I'm at http://zshens-macbook-pro.local:8088/proxy/application_1423523667600_0001/mapreduce/task/task_1423523667600_0001_m_000000 when the app is running. After the app is completed, I refresh the page and am redirected to http://10.22.2.92:19888/jobhistory/job/job_1423523667600_0001/mapreduce/task/task_1423523667600_0001_m_000000. It's no longer the mapper page but the app summary page, because mapreduce/task/task_1423523667600_0001_m_000000 are useless sections in JHS.

        Think it out loudly. In general, seamless view will still be difficult with the aforementioned solution between two tracking URLs. For example, tracking URL is http://t1:p1/a/b first, and I'm visiting the path at http://t1:p1/a/b/x/y/z. When the tracking URL becomes http://t2:p2/c/d/e, I refresh the package and am redirected http://t2:p2/c/d/e/a/b/x/y/z. Without mapping between original tracking url and proxy url, we don't know /a/b is part of tracking url base, and it shouldn't be carried on.

        Show
        zjshen Zhijie Shen added a comment - Jason Lowe , I think we're on the same page about what's reason of duplicate section of the tracking url. RMAppAttemptImpl is the one tacking on the extra, redundant portion based on the path the AM unregistered. Are you suggesting no matter how many sections after authority in the track url like http(s)://tracking authority/S1/Sk/.../Sk , we always generates the proxy url like http(s)://proxy authority/proxy/<appId> ? Do you suggest that we should use the original tracking url directly instead of proxy url on the web UI? I think this should work and we may simply change the code in RMAppAttemptImpl#generateProxyUriWithScheme: URI result = ProxyUriUtils.getProxyUri( null , proxyUri, applicationAttemptId.getApplicationId()); I believe it will be translated properly, as it works today Maybe I was not clear before. I didn't meant MR AM UI will not work, but seamless view between the AM UI and history UI is not possible nowadays. For example, I'm at http://zshens-macbook-pro.local:8088/proxy/application_1423523667600_0001/mapreduce/task/task_1423523667600_0001_m_000000 when the app is running. After the app is completed, I refresh the page and am redirected to http://10.22.2.92:19888/jobhistory/job/job_1423523667600_0001/mapreduce/task/task_1423523667600_0001_m_000000 . It's no longer the mapper page but the app summary page, because mapreduce/task/task_1423523667600_0001_m_000000 are useless sections in JHS. Think it out loudly. In general, seamless view will still be difficult with the aforementioned solution between two tracking URLs. For example, tracking URL is http://t1:p1/a/b first, and I'm visiting the path at http://t1:p1/a/b/x/y/z . When the tracking URL becomes http://t2:p2/c/d/e , I refresh the package and am redirected http://t2:p2/c/d/e/a/b/x/y/z . Without mapping between original tracking url and proxy url, we don't know /a/b is part of tracking url base, and it shouldn't be carried on.
        Hide
        jlowe Jason Lowe added a comment -

        But, if we don't fix the url translation problem, generated url will be http://<histaddr>/job/<jobid>/job/<jobid>/x/y/, which should also be wrong.

        IIUC the only reason we are going to http://<histaddr>/job/<jobid>/job/<jobid>/x/y/ is because after the AM unregisters with a history URL of http://<histaddr>/job/<jobid> it is advertising the proxified app tracking URL on the RM UI as http://<rmaddr>/proxy/<appid>/job/<jobid>. That link is just wrong. When the web proxy sees that link, it (correctly, IMHO) translates it to http://<histaddr>/job/<jobid>/job/<jobid>/x/y/. RMAppAttemptImpl is the one tacking on the extra, redundant portion based on the path the AM unregistered.

        We can do that, but for the running app, we will enter http://<rmaddr>/proxy/<appid>, then go to http://<rmaddr>/proxy/<appid>/x, then go to http://<rmaddr>/proxy/<appid>/x/y and so on. In these cases, we need to make sure it is translated properly.

        I believe it will be translated properly, as it works today (i.e.: one can successfully navigate deeper in the MapReduce UI on the AM while the job is running). As long as we don't change the web proxy servlet, that should continue to work. I think the bug is simply how RMAppAttemptImpl handles the web proxy when the app unregisters, specifically in RMAppAttemptImpl#generateProxyUriWithScheme. It is trying to do a replace the authority of the tracking URL with <rmaddr>/proxy/<appid> when it should just make the proxied tracking url always http://<rmaddr>/proxy/<appid>.

        Show
        jlowe Jason Lowe added a comment - But, if we don't fix the url translation problem, generated url will be http://<histaddr>/job/<jobid>/job/<jobid>/x/y/, which should also be wrong. IIUC the only reason we are going to http://<histaddr>/job/<jobid>/job/<jobid>/x/y/ is because after the AM unregisters with a history URL of http://<histaddr>/job/<jobid> it is advertising the proxified app tracking URL on the RM UI as http://<rmaddr>/proxy/<appid>/job/<jobid>. That link is just wrong. When the web proxy sees that link, it (correctly, IMHO) translates it to http://<histaddr>/job/<jobid>/job/<jobid>/x/y/. RMAppAttemptImpl is the one tacking on the extra, redundant portion based on the path the AM unregistered. We can do that, but for the running app, we will enter http://<rmaddr>/proxy/<appid>, then go to http://<rmaddr>/proxy/<appid>/x, then go to http://<rmaddr>/proxy/<appid>/x/y and so on. In these cases, we need to make sure it is translated properly. I believe it will be translated properly, as it works today (i.e.: one can successfully navigate deeper in the MapReduce UI on the AM while the job is running). As long as we don't change the web proxy servlet, that should continue to work. I think the bug is simply how RMAppAttemptImpl handles the web proxy when the app unregisters, specifically in RMAppAttemptImpl#generateProxyUriWithScheme. It is trying to do a replace the authority of the tracking URL with <rmaddr>/proxy/<appid> when it should just make the proxied tracking url always http://<rmaddr>/proxy/<appid>.
        Hide
        zjshen Zhijie Shen added a comment -

        With this patch, it looks like it's going to send me to http://<histaddr>/job/<jobid> and strip off the x/y/z part.

        Nice catch! But, if we don't fix the url translation problem, generated url will be http://<histaddr>/job/<jobid>/job/<jobid>/x/y/, which should also be wrong.

        Instead I think it would be better if the proxy servlet preserved the existing behavior and instead in the RM UI we never advertised a tracking URL other than http://<rmaddr>/proxy/<appid>.

        We can do that, but for the running app, we will enter http://<rmaddr>/proxy/<appid>, then go to http://<rmaddr>/proxy/<appid>/x, then go to http://<rmaddr>/proxy/<appid>/x/y and so on. In these cases, we need to make sure it is translated properly.

        Show
        zjshen Zhijie Shen added a comment - With this patch, it looks like it's going to send me to http://<histaddr>/job/<jobid> and strip off the x/y/z part. Nice catch! But, if we don't fix the url translation problem, generated url will be http://<histaddr>/job/<jobid>/job/<jobid>/x/y/, which should also be wrong. Instead I think it would be better if the proxy servlet preserved the existing behavior and instead in the RM UI we never advertised a tracking URL other than http://<rmaddr>/proxy/<appid>. We can do that, but for the running app, we will enter http://<rmaddr>/proxy/<appid>, then go to http://<rmaddr>/proxy/<appid>/x, then go to http://<rmaddr>/proxy/<appid>/x/y and so on. In these cases, we need to make sure it is translated properly.
        Hide
        zjshen Zhijie Shen added a comment - - edited

        Deverej, thanks for the explanation! It makes sense to me. I've tried the patch, and it seems to work fine for MR case. However, I'm wondering it's a general solution for all applications.

        Now in the patch, res will still be appended to the tracking url for running. It's correct because MR set the tracking url to http(s)://am host:am port. The tracking url just has the authority, but no section else afterwards. So when you requesting resource under http(s)://proxy host:proxy port/proxy/appId/static/yarn.css, it will be translated to http(s)://am host:am port/static/yarn.css, which is correct. However, if unfortunately your resource is not at the root, but at http(s)://am host:am port/base, and you use it as the tracking url, your proxy url will become http(s)://proxy host:proxy port/proxy/appId/base/static/yarn.css, and toFetch will be finally translated to http(s)://am host:am port/base/base/static/yarn.css, which has double "base" and is wrong. This is why the tracking url is wrong for the completed apps, which has been updated to http(s)://jhs host:jhs port/jobhistory/job/jobId.

        IMHO, the proper fix should be that: If the tracking url is http(s)://tracking authority/S1/Sk/.../Sk, it should be still translated to the proxy url http(s)://proxy authority/proxy/appId/S1/Sk/.../Sk. When the proxy url is translated to the track url, we should not just replace authority, but mapping the whole http(s)://proxy authority/proxy/appId/S1/Sk/.../Sk' to http(s)://tracking authority/S1/Sk/.../Sk' when k' <= k, and to http(s)://tracking authority/S1/Sk/.../Sk/Sk+1/Sk+2/.../Sk' when k' > k.

        Show
        zjshen Zhijie Shen added a comment - - edited Deverej, thanks for the explanation! It makes sense to me. I've tried the patch, and it seems to work fine for MR case. However, I'm wondering it's a general solution for all applications. Now in the patch, res will still be appended to the tracking url for running. It's correct because MR set the tracking url to http(s)://am host:am port . The tracking url just has the authority, but no section else afterwards. So when you requesting resource under http(s)://proxy host:proxy port/proxy/appId/static/yarn.css , it will be translated to http(s)://am host:am port/static/yarn.css , which is correct. However, if unfortunately your resource is not at the root, but at http(s)://am host:am port/base , and you use it as the tracking url, your proxy url will become http(s)://proxy host:proxy port/proxy/appId/base/static/yarn.css , and toFetch will be finally translated to http(s)://am host:am port/base/base/static/yarn.css , which has double "base" and is wrong. This is why the tracking url is wrong for the completed apps, which has been updated to http(s)://jhs host:jhs port/jobhistory/job/jobId . IMHO, the proper fix should be that: If the tracking url is http(s)://tracking authority/S1/Sk/.../Sk , it should be still translated to the proxy url http(s)://proxy authority/proxy/appId/S1/Sk/.../Sk . When the proxy url is translated to the track url, we should not just replace authority, but mapping the whole http(s)://proxy authority/proxy/appId/S1/Sk/.../Sk' to http(s)://tracking authority/S1/Sk/.../Sk' when k' <= k , and to http(s)://tracking authority/S1/Sk/.../Sk/Sk+1/Sk+2/.../Sk' when k' > k .
        Hide
        jlowe Jason Lowe added a comment -

        Thanks for the patch, Devaraj! However I'm not sure changing WebAppProxyServlet is the best fix. For example, if I navigate through an active applications UI (via the proxy link), I might end up at a place like http://<rmaddr>/proxy/<appid>/x/y/z. Then the application completes and sets a history tracking URL to http://<histaddr>/job/<jobid>. If I refresh that page then I would expect, if the application framework properly designed a seamless view between the AM UI and history UI, that the history page would serve up the same information at http://<histaddr>/job/<jobid>/x/y/z. With this patch, it looks like it's going to send me to http://<histaddr>/job/<jobid> and strip off the x/y/z part.

        Instead I think it would be better if the proxy servlet preserved the existing behavior and instead in the RM UI we never advertised a tracking URL other than http://<rmaddr>/proxy/<appid>. So I think the real bug is in RMAppImpl rather than the WebAppProxyServlet per my previous comment above. In addition I don't think it's safe to assume that a query string ripped off of an arbitrary path underneath the proxy URL will be OK to apply at the top level of the history URL. In many cases there isn't going to be a query string, and the path underneath the proxy URL is significant. The query string is often going to be context-sensitive to the path, and we would be ignoring that path.

        Show
        jlowe Jason Lowe added a comment - Thanks for the patch, Devaraj! However I'm not sure changing WebAppProxyServlet is the best fix. For example, if I navigate through an active applications UI (via the proxy link), I might end up at a place like http://<rmaddr>/proxy/<appid>/x/y/z. Then the application completes and sets a history tracking URL to http://<histaddr>/job/<jobid>. If I refresh that page then I would expect, if the application framework properly designed a seamless view between the AM UI and history UI, that the history page would serve up the same information at http://<histaddr>/job/<jobid>/x/y/z. With this patch, it looks like it's going to send me to http://<histaddr>/job/<jobid> and strip off the x/y/z part. Instead I think it would be better if the proxy servlet preserved the existing behavior and instead in the RM UI we never advertised a tracking URL other than http://<rmaddr>/proxy/<appid>. So I think the real bug is in RMAppImpl rather than the WebAppProxyServlet per my previous comment above. In addition I don't think it's safe to assume that a query string ripped off of an arbitrary path underneath the proxy URL will be OK to apply at the top level of the history URL. In many cases there isn't going to be a query string, and the path underneath the proxy URL is significant. The query string is often going to be context-sensitive to the path, and we would be ignoring that path.
        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/12697162/YARN-2246.patch
        against trunk revision cfb829e.

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

        +1 tests included. The patch appears to include 1 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-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy.

        Test results: https://builds.apache.org/job/PreCommit-YARN-Build/6548//testReport/
        Console output: https://builds.apache.org/job/PreCommit-YARN-Build/6548//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/12697162/YARN-2246.patch against trunk revision cfb829e. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 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-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-web-proxy. Test results: https://builds.apache.org/job/PreCommit-YARN-Build/6548//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/6548//console This message is automatically generated.
        Hide
        devaraj.k Devaraj K added a comment -

        But do you know why we use proxyLink for running app instead of redirect the request?

        It is to avoid the AM URL becoming stale/unusable when the application complete. If the proxy server redirects the URL to AM when application is running, the redirected URL will work till AM is running and when the AM goes down the URL will not work anymore if the user continues accessing the same or refreshing the browser. Now with the current proxy server, proxy server gets the latest application report and decides whether to get the Job details from the AM or redirect the URL to history based on the application state.

        Show
        devaraj.k Devaraj K added a comment - But do you know why we use proxyLink for running app instead of redirect the request? It is to avoid the AM URL becoming stale/unusable when the application complete. If the proxy server redirects the URL to AM when application is running, the redirected URL will work till AM is running and when the AM goes down the URL will not work anymore if the user continues accessing the same or refreshing the browser. Now with the current proxy server, proxy server gets the latest application report and decides whether to get the Job details from the AM or redirect the URL to history based on the application state.
        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/12697162/YARN-2246.patch
        against trunk revision da2fb2b.

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

        +1 tests included. The patch appears to include 1 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 failed to build 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 .

        Test results: https://builds.apache.org/job/PreCommit-YARN-Build/6542//testReport/
        Console output: https://builds.apache.org/job/PreCommit-YARN-Build/6542//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/12697162/YARN-2246.patch against trunk revision da2fb2b. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 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 failed to build 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 . Test results: https://builds.apache.org/job/PreCommit-YARN-Build/6542//testReport/ Console output: https://builds.apache.org/job/PreCommit-YARN-Build/6542//console This message is automatically generated.
        Hide
        zjshen Zhijie Shen added a comment -

        For running applications 'res' needs to be appended to 'trackingUri' because it is trying to load the files like

        I see. But do you know why we use proxyLink for running app instead of redirect the request?

        Show
        zjshen Zhijie Shen added a comment - For running applications 'res' needs to be appended to 'trackingUri' because it is trying to load the files like I see. But do you know why we use proxyLink for running app instead of redirect the request?
        Hide
        devaraj.k Devaraj K added a comment -

        As Zhijie mentioned in the above comment, not appending the 'res' to the end of the 'trackingUri' in the case of completed applications. For running applications 'res' needs to be appended to 'trackingUri' because it is trying to load the files like http://host1:35456/static/yarn.css, http://hos1:35456/static/jquery/themes-1.9.1/base/jquery-ui.css and etc.

        Here the approach is similar to the already provided patch in this jira with the updated test case. Can you have a look into the updated patch? Thanx.

        Show
        devaraj.k Devaraj K added a comment - As Zhijie mentioned in the above comment, not appending the 'res' to the end of the 'trackingUri' in the case of completed applications. For running applications 'res' needs to be appended to 'trackingUri' because it is trying to load the files like http://host1:35456/static/yarn.css , http://hos1:35456/static/jquery/themes-1.9.1/base/jquery-ui.css and etc. Here the approach is similar to the already provided patch in this jira with the updated test case. Can you have a look into the updated patch? Thanx.
        Hide
        devaraj.k Devaraj K added a comment -

        Jason Lowe, Zhijie Shen Thanks for your inputs.

        Jason Lowe, I have started working on this, will provide patch today. Thanks

        Show
        devaraj.k Devaraj K added a comment - Jason Lowe , Zhijie Shen Thanks for your inputs. Jason Lowe , I have started working on this, will provide patch today. Thanks
        Hide
        jlowe Jason Lowe added a comment -

        Devaraj K are you still planning to address this issue? It's a benign problem with the history server UI since it ignores the extra components of the URL, but there are some use cases with Tez and other instances where this needs to be fixed.

        Show
        jlowe Jason Lowe added a comment - Devaraj K are you still planning to address this issue? It's a benign problem with the history server UI since it ignores the extra components of the URL, but there are some use cases with Tez and other instances where this needs to be fixed.
        Hide
        zjshen Zhijie Shen added a comment -

        IMHO the proxy URL advertised to clients should always be http://rmaddr/proxy/appid, without other stuff tacked on the end.

        +1 for the idea. In WebAppProxyServlet, we don't need to append rest to the end of trackingUri, but just include the query param.

        BTW, this may assume the framework has done proper multiplexing on the tracking URL for each application, which means app1 has the track URL http://x/y/app1, while app2 has http://x/y/app2. It seems that both MR and TEZ (TEZ-2018) are composing the track URL in this way.

        Show
        zjshen Zhijie Shen added a comment - IMHO the proxy URL advertised to clients should always be http://rmaddr/proxy/appid , without other stuff tacked on the end. +1 for the idea. In WebAppProxyServlet, we don't need to append rest to the end of trackingUri , but just include the query param. BTW, this may assume the framework has done proper multiplexing on the tracking URL for each application, which means app1 has the track URL http://x/y/app1 , while app2 has http://x/y/app2 . It seems that both MR and TEZ ( TEZ-2018 ) are composing the track URL in this way.
        Hide
        jlowe Jason Lowe added a comment -

        I think the bug is in RMAppAttemptImpl. When the AM unregisters, RMAppAttemptImpl.generateProxyUriWithScheme will take whatever URL the AM specified and replace the server with the proxy URL, e.g.: tracking URL http://x/y/z becomes http://rmaddr/proxy/appid/y/z. Then when the webproxy processes that URL it just replaces http://rmaddr/proxy/appid with the tracking URL, leading to http://x/y/z/y/z. IMHO the proxy URL advertised to clients should always be http://rmaddr/proxy/appid, without other stuff tacked on the end. That can map to whatever tracking URL the app provided when processed by the webproxy.

        I also wonder if we should list the final tracking URL on the RM UI rather than the proxy URL. Seems simpler to just direct them to the final tracking URL rather than through the RM proxy, unless there's a use case where the client can't reach the final tracking URL directly and needs to go through the proxy. I haven't heard of such a setup.

        Show
        jlowe Jason Lowe added a comment - I think the bug is in RMAppAttemptImpl. When the AM unregisters, RMAppAttemptImpl.generateProxyUriWithScheme will take whatever URL the AM specified and replace the server with the proxy URL, e.g.: tracking URL http://x/y/z becomes http://rmaddr/proxy/appid/y/z . Then when the webproxy processes that URL it just replaces http://rmaddr/proxy/appid with the tracking URL, leading to http://x/y/z/y/z . IMHO the proxy URL advertised to clients should always be http://rmaddr/proxy/appid , without other stuff tacked on the end. That can map to whatever tracking URL the app provided when processed by the webproxy. I also wonder if we should list the final tracking URL on the RM UI rather than the proxy URL. Seems simpler to just direct them to the final tracking URL rather than through the RM proxy, unless there's a use case where the client can't reach the final tracking URL directly and needs to go through the proxy. I haven't heard of such a setup.
        Hide
        zjshen Zhijie Shen added a comment -

        Move the ticket to YARN, as the root cause sound like the YARN issue.

        Show
        zjshen Zhijie Shen added a comment - Move the ticket to YARN, as the root cause sound like the YARN issue.
        Hide
        zjshen Zhijie Shen added a comment -

        The bug still happens on trunk. I did some investigation:

        1. When generating proxyTrackingUrl from originalTrackUrl, the original host section is replaced by "[proxy host]:[port]/proxy/[application id]", while the following sections are kept. For example,

        http://192.168.1.108:19888/jobhistory/job/job_1404265212778_0003
        

        in translated into

        https://0.0.0.0:8088/proxy/application_1404265212778_0003/jobhistory/job/job_1404265212778_0003
        

        2. On the other hand, in WebAppProxyServlet, "[application id]" section is used to search for originalTrackingURL, and the following part is appended to the end of originalTrackingURL. For example, we will see

        http://192.168.1.108:19888/jobhistory/job/job_1404265212778_0001/jobhistory/job/job_1404265212778_0001
        

        Fortunately, JHS will simply ignore the additional sections, such that we won't see an apparent error.

        Therefore, with the current logic, whenever AM provides a tracking url that has more sections after host:port, it will be finally translated into an url that has duplicate sections after host:port.

        It seems we should either hide all the rest sections in proxyTrackingURL, or not append them when WebAppProxyServlet recovers the originalTrackingURL.

        Show
        zjshen Zhijie Shen added a comment - The bug still happens on trunk. I did some investigation: 1. When generating proxyTrackingUrl from originalTrackUrl, the original host section is replaced by " [proxy host] : [port] /proxy/ [application id] ", while the following sections are kept. For example, http: //192.168.1.108:19888/jobhistory/job/job_1404265212778_0003 in translated into https: //0.0.0.0:8088/proxy/application_1404265212778_0003/jobhistory/job/job_1404265212778_0003 2. On the other hand, in WebAppProxyServlet, " [application id] " section is used to search for originalTrackingURL, and the following part is appended to the end of originalTrackingURL. For example, we will see http: //192.168.1.108:19888/jobhistory/job/job_1404265212778_0001/jobhistory/job/job_1404265212778_0001 Fortunately, JHS will simply ignore the additional sections, such that we won't see an apparent error. Therefore, with the current logic, whenever AM provides a tracking url that has more sections after host:port, it will be finally translated into an url that has duplicate sections after host:port. It seems we should either hide all the rest sections in proxyTrackingURL, or not append them when WebAppProxyServlet recovers the originalTrackingURL.
        Hide
        revans2 Robert Joseph Evans added a comment -

        Having the changes only apply when doing a redirect will fix the problem for Map Reduce, but I think the underlying cause of the issue is not with the proxy itself. I believe that the proxy is working as designed, the problem is with the code that is producing the URI.

        I believe that the issue is with ProxyUriUtils.getProxyUri(). It is adding on the extra path part that should be removed. It should always act as if the trackingUri parameter is null. And if that is the case that parameter should just be removed, which will simplify things in RMAppAttemptImpl.java because we will not have to ever regenerate the proxiedTrackingUrl.

        I am fairly sure that I put this bug in so thank you for finding it and trying to fix it.

        Show
        revans2 Robert Joseph Evans added a comment - Having the changes only apply when doing a redirect will fix the problem for Map Reduce, but I think the underlying cause of the issue is not with the proxy itself. I believe that the proxy is working as designed, the problem is with the code that is producing the URI. I believe that the issue is with ProxyUriUtils.getProxyUri(). It is adding on the extra path part that should be removed. It should always act as if the trackingUri parameter is null. And if that is the case that parameter should just be removed, which will simplify things in RMAppAttemptImpl.java because we will not have to ever regenerate the proxiedTrackingUrl. I am fairly sure that I put this bug in so thank you for finding it and trying to fix it.
        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/12520094/MAPREDUCE-4064-1.patch
        against trunk revision .

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

        +1 tests included. The patch appears to include 3 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

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

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

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

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

        +1 core tests. The patch passed unit tests in .

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

        Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2107//testReport/
        Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2107//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/12520094/MAPREDUCE-4064-1.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in . +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2107//testReport/ Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2107//console This message is automatically generated.
        Hide
        devaraj.k Devaraj K added a comment -

        Thank you Robert for looking into the patch.

        Before posting I tested the patch including all the links in the AM/History UI but I couldn't find out the cosmetic problems(caused by previous patch) in the AM UI due to the issue MAPREDUCE-3173.

        Now I have updated the patch with the change applies to only redirected history URL. Can you please review this patch?

        Show
        devaraj.k Devaraj K added a comment - Thank you Robert for looking into the patch. Before posting I tested the patch including all the links in the AM/History UI but I couldn't find out the cosmetic problems(caused by previous patch) in the AM UI due to the issue MAPREDUCE-3173 . Now I have updated the patch with the change applies to only redirected history URL. Can you please review this patch?
        Hide
        revans2 Robert Joseph Evans added a comment -

        You took out the rest of the URL, so the only thing that you can get through the proxy is the base URL.

        http://<SOMETHING>/proxy/application_.../some/important/path

        will get translated into

        http://<HISTORY>/jobhistory/job/job_.../

        or more importantly

        http://<MR_AM>/

        "/some/important/path" will be thrown away. This may not be that critical for the history server where we redirect, but no AM UI that actually goes through the proxy is going to work properly after this change.

        Show
        revans2 Robert Joseph Evans added a comment - You took out the rest of the URL, so the only thing that you can get through the proxy is the base URL. http://<SOMETHING>/proxy/application_.../some/important/path will get translated into http://<HISTORY>/jobhistory/job/job_.../ or more importantly http://<MR_AM>/ "/some/important/path" will be thrown away. This may not be that critical for the history server where we redirect, but no AM UI that actually goes through the proxy is going to work properly after this change.
        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/12519974/MAPREDUCE-4064.patch
        against trunk revision .

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

        +1 tests included. The patch appears to include 3 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

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

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

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

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

        +1 core tests. The patch passed unit tests in .

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

        Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2101//testReport/
        Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2101//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/12519974/MAPREDUCE-4064.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in . +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2101//testReport/ Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/2101//console This message is automatically generated.
        Hide
        devaraj.k Devaraj K added a comment -

        I have attached patch to fix this. Please review this.

        Show
        devaraj.k Devaraj K added a comment - I have attached patch to fix this. Please review this.

          People

          • Assignee:
            devaraj.k Devaraj K
            Reporter:
            devaraj.k Devaraj K
          • Votes:
            0 Vote for this issue
            Watchers:
            11 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development