You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by tillrohrmann <gi...@git.apache.org> on 2018/01/23 14:29:13 UTC

[GitHub] flink pull request #5341: [FLINK-8495] [flip6] Enable main cluster component...

GitHub user tillrohrmann opened a pull request:

    https://github.com/apache/flink/pull/5341

    [FLINK-8495] [flip6] Enable main cluster component's log and stdout file retrieval

    ## What is the purpose of the change
    
    This commit enables the log and stdout file retrieval of the cluster's main component
    via the web ui. This happens via the StaticFileServerHandler which serves the log
    and stdout file.
    
    ## Verifying this change
    
    - Tested manually
    
    ## Does this pull request potentially affect one of the following parts:
    
      - Dependencies (does it add or upgrade a dependency): (no)
      - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no)
      - The serializers: (no)
      - The runtime per-record code paths (performance sensitive): (no)
      - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)
      - The S3 file system connector: (no)
    
    ## Documentation
    
      - Does this pull request introduce a new feature? (no)
      - If yes, how is the feature documented? (not applicable)


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/tillrohrmann/flink enableWebLogRetrieval

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/5341.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #5341
    
----
commit a2672b713917f7ea09242684c0106aaf407cf867
Author: Till Rohrmann <tr...@...>
Date:   2018-01-23T14:17:16Z

    [FLINK-8495] [flip6] Enable main cluster component's log and stdout file retrieval
    
    This commit enables the log and stdout file retrieval of the cluster's main component
    via the web ui. This happens via the StaticFileServerHandler which serves the log
    and stdout file.

----


---

[GitHub] flink pull request #5341: [FLINK-8495] [flip6] Enable main cluster component...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/flink/pull/5341


---

[GitHub] flink pull request #5341: [FLINK-8495] [flip6] Enable main cluster component...

Posted by GJL <gi...@git.apache.org>.
Github user GJL commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5341#discussion_r165941154
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/webmonitor/WebMonitorEndpoint.java ---
    @@ -473,10 +476,55 @@ public WebMonitorEndpoint(
     		handlers.add(Tuple2.of(SubtaskCurrentAttemptDetailsHeaders.getInstance(), subtaskCurrentAttemptDetailsHandler));
     		handlers.add(Tuple2.of(JobVertexTaskManagersHeaders.getInstance(), jobVertexTaskManagersHandler));
     
    -		// This handler MUST be added last, as it otherwise masks all subsequent GET handlers
     		optWebContent.ifPresent(
     			webContent -> handlers.add(Tuple2.of(WebContentHandlerSpecification.getInstance(), webContent)));
     
    +		// load the log and stdout file handler for the main cluster component
    +		final WebMonitorUtils.LogFileLocation logFileLocation = WebMonitorUtils.LogFileLocation.find(clusterConfiguration);
    +
    +		final ChannelInboundHandler logFileHandler;
    +
    +		if (logFileLocation.logFile == null) {
    +			logFileHandler = new ConstantTextHandler("(log file unavailable)");
    +		} else {
    +			ChannelInboundHandler staticFileServerHandler;
    +			try {
    +				staticFileServerHandler = new StaticFileServerHandler<>(
    +					leaderRetriever,
    +					restAddressFuture,
    +					timeout,
    +					logFileLocation.logFile);
    +			} catch (IOException e) {
    +				log.info("Cannot load log file handler.", e);
    +				staticFileServerHandler = new ConstantTextHandler("(log file unavailable)");
    +			}
    +
    +			logFileHandler = staticFileServerHandler;
    +		}
    +
    +		final ChannelInboundHandler stdoutFileHandler;
    +
    +		if (logFileLocation.stdOutFile == null) {
    --- End diff --
    
    I would try to avoid code duplication, e.g.,
    ```
    final ChannelInboundHandler logFileHandler = createLogFileHandler(logFileLocation.logFile, restAddressFuture);
    
    final ChannelInboundHandler stdoutFileHandler = createLogFileHandler(logFileLocation.stdOutFile, restAddressFuture);
    ```
    
    ```
    private ChannelInboundHandler createLogFileHandler(
    			final File logFile, 
    			final CompletableFuture<String> restAddressFuture) {
    		if (logFile == null) {
    			return new ConstantTextHandler("(log file unavailable)");
    		} else {
    			ChannelInboundHandler staticFileServerHandler;
    			try {
    				staticFileServerHandler = new StaticFileServerHandler<>(
    					leaderRetriever,
    					restAddressFuture,
    					restConfiguration.getTimeout(),
    					logFile);
    			} catch (IOException e) {
    				log.info("Cannot load log file handler ().", logFile, e);
    				staticFileServerHandler = new ConstantTextHandler("(log file unavailable)");
    			}
    
    			return staticFileServerHandler;
    		}
    	}
    ```
    
    (untested)


---

[GitHub] flink pull request #5341: [FLINK-8495] [flip6] Enable main cluster component...

Posted by GJL <gi...@git.apache.org>.
Github user GJL commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5341#discussion_r165942186
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/webmonitor/WebMonitorEndpoint.java ---
    @@ -473,10 +476,55 @@ public WebMonitorEndpoint(
     		handlers.add(Tuple2.of(SubtaskCurrentAttemptDetailsHeaders.getInstance(), subtaskCurrentAttemptDetailsHandler));
     		handlers.add(Tuple2.of(JobVertexTaskManagersHeaders.getInstance(), jobVertexTaskManagersHandler));
     
    -		// This handler MUST be added last, as it otherwise masks all subsequent GET handlers
    --- End diff --
    
    is it not a problem anymore that 
    ```
    @Override
    	public String getTargetRestEndpointURL() {
    		return "/:*";
    	}
    ```
    masks all subsequent GET handlers?


---

[GitHub] flink pull request #5341: [FLINK-8495] [flip6] Enable main cluster component...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5341#discussion_r166222787
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/webmonitor/WebMonitorEndpoint.java ---
    @@ -473,10 +476,55 @@ public WebMonitorEndpoint(
     		handlers.add(Tuple2.of(SubtaskCurrentAttemptDetailsHeaders.getInstance(), subtaskCurrentAttemptDetailsHandler));
     		handlers.add(Tuple2.of(JobVertexTaskManagersHeaders.getInstance(), jobVertexTaskManagersHandler));
     
    -		// This handler MUST be added last, as it otherwise masks all subsequent GET handlers
     		optWebContent.ifPresent(
     			webContent -> handlers.add(Tuple2.of(WebContentHandlerSpecification.getInstance(), webContent)));
     
    +		// load the log and stdout file handler for the main cluster component
    +		final WebMonitorUtils.LogFileLocation logFileLocation = WebMonitorUtils.LogFileLocation.find(clusterConfiguration);
    +
    +		final ChannelInboundHandler logFileHandler;
    +
    +		if (logFileLocation.logFile == null) {
    +			logFileHandler = new ConstantTextHandler("(log file unavailable)");
    +		} else {
    +			ChannelInboundHandler staticFileServerHandler;
    +			try {
    +				staticFileServerHandler = new StaticFileServerHandler<>(
    +					leaderRetriever,
    +					restAddressFuture,
    +					timeout,
    +					logFileLocation.logFile);
    +			} catch (IOException e) {
    +				log.info("Cannot load log file handler.", e);
    +				staticFileServerHandler = new ConstantTextHandler("(log file unavailable)");
    +			}
    +
    +			logFileHandler = staticFileServerHandler;
    +		}
    +
    +		final ChannelInboundHandler stdoutFileHandler;
    +
    +		if (logFileLocation.stdOutFile == null) {
    --- End diff --
    
    True, will change it.


---