Re: [PR] [FLINK-33434][runtime-web] Support invoke async-profiler on TaskManager via REST API [flink]

2024-01-18 Thread via GitHub 


Myasuka merged PR #24041:
URL: https://github.com/apache/flink/pull/24041


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [FLINK-33434][runtime-web] Support invoke async-profiler on TaskManager via REST API [flink]

2024-01-17 Thread via GitHub 


yuchen-ecnu commented on PR #24041:
URL: https://github.com/apache/flink/pull/24041#issuecomment-1897790800

   @flinkbot run azure


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [FLINK-33434][runtime-web] Support invoke async-profiler on TaskManager via REST API [flink]

2024-01-17 Thread via GitHub 


yuchen-ecnu commented on PR #24041:
URL: https://github.com/apache/flink/pull/24041#issuecomment-1895891972

   Hi @Myasuka , I have added two more tests for `TaskManagerProfilingHandler` 
and `TaskManagerProfilingListHandler`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [FLINK-33434][runtime-web] Support invoke async-profiler on TaskManager via REST API [flink]

2024-01-16 Thread via GitHub 


yuchen-ecnu commented on PR #24041:
URL: https://github.com/apache/flink/pull/24041#issuecomment-1893540501

   Hi @Myasuka , I have reverted the changes of the deprecated function 
`requestTaskManagerFileUploadByName`. But in the 
`TaskManagerProfilingFileHandler`, the `Time timeout` was used by the base 
class `LeaderRetrievalHandler`.  And it was used by almost all handlers, so I 
think maybe we can replace that in a separate PR in the future.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [FLINK-33434][runtime-web] Support invoke async-profiler on TaskManager via REST API [flink]

2024-01-15 Thread via GitHub 


Myasuka commented on code in PR #24041:
URL: https://github.com/apache/flink/pull/24041#discussion_r1452465323


##
flink-runtime/src/main/java/org/apache/flink/runtime/rest/handler/taskmanager/TaskManagerProfilingFileHandler.java:
##
@@ -0,0 +1,82 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.runtime.rest.handler.taskmanager;
+
+import org.apache.flink.api.common.time.Time;
+import org.apache.flink.api.java.tuple.Tuple2;
+import org.apache.flink.runtime.blob.TransientBlobKey;
+import org.apache.flink.runtime.blob.TransientBlobService;
+import org.apache.flink.runtime.clusterframework.types.ResourceID;
+import org.apache.flink.runtime.resourcemanager.ResourceManagerGateway;
+import org.apache.flink.runtime.rest.handler.HandlerRequest;
+import org.apache.flink.runtime.rest.messages.EmptyRequestBody;
+import org.apache.flink.runtime.rest.messages.ProfilingFileNamePathParameter;
+import org.apache.flink.runtime.rest.messages.UntypedResponseMessageHeaders;
+import 
org.apache.flink.runtime.rest.messages.taskmanager.TaskManagerProfilingFileMessageParameters;
+import org.apache.flink.runtime.taskexecutor.FileType;
+import org.apache.flink.runtime.taskexecutor.TaskExecutor;
+import org.apache.flink.runtime.webmonitor.RestfulGateway;
+import org.apache.flink.runtime.webmonitor.retriever.GatewayRetriever;
+
+import javax.annotation.Nonnull;
+
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+
+/** Rest handler which serves the profiling result file of the {@link 
TaskExecutor}. */
+public class TaskManagerProfilingFileHandler

Review Comment:
   It seems you forget to change this class.



##
flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManagerGateway.java:
##
@@ -225,16 +226,38 @@ CompletableFuture 
requestTaskManagerFileUploadByType(
 
 /**
  * Request the file upload from the given {@link TaskExecutor} to the 
cluster's {@link
- * BlobServer}. The corresponding {@link TransientBlobKey} is returned.
+ * BlobServer}. The corresponding {@link TransientBlobKey} is returned. To 
support different
+ * type file upload with name, using {@link
+ * ResourceManager#requestTaskManagerFileUploadByNameAndType} as instead.
  *
  * @param taskManagerId identifying the {@link TaskExecutor} to upload the 
specified file
  * @param fileName name of the file to upload
  * @param timeout for the asynchronous operation
  * @return Future which is completed with the {@link TransientBlobKey} 
after uploading the file
  * to the {@link BlobServer}.
+ * @deprecated use {@link 
#requestTaskManagerFileUploadByNameAndType(ResourceID, String,
+ * FileType, Duration)} as instead.
  */
+@Deprecated
 CompletableFuture requestTaskManagerFileUploadByName(
-ResourceID taskManagerId, String fileName, @RpcTimeout Time 
timeout);
+ResourceID taskManagerId, String fileName, @RpcTimeout Duration 
timeout);

Review Comment:
   We do not need to change the class from `Time` to `Duration` for a 
deprecated interface.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [FLINK-33434][runtime-web] Support invoke async-profiler on TaskManager via REST API [flink]

2024-01-15 Thread via GitHub 


yuchen-ecnu commented on PR #24041:
URL: https://github.com/apache/flink/pull/24041#issuecomment-1892255488

   Hi @Myasuka , I have replaced `Time` with `Duraion` in the updated code. 
Please have a look if there are any other problems. Thanks.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [FLINK-33434][runtime-web] Support invoke async-profiler on TaskManager via REST API [flink]

2024-01-14 Thread via GitHub 


Myasuka commented on code in PR #24041:
URL: https://github.com/apache/flink/pull/24041#discussion_r1451675133


##
flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManagerGateway.java:
##
@@ -225,17 +225,34 @@ CompletableFuture 
requestTaskManagerFileUploadByType(
 
 /**
  * Request the file upload from the given {@link TaskExecutor} to the 
cluster's {@link
- * BlobServer}. The corresponding {@link TransientBlobKey} is returned.
+ * BlobServer}. The corresponding {@link TransientBlobKey} is returned. To 
support different
+ * type file upload with name, using {@link
+ * ResourceManager#requestTaskManagerFileUploadByNameAndType} as instead.
  *
  * @param taskManagerId identifying the {@link TaskExecutor} to upload the 
specified file
  * @param fileName name of the file to upload
  * @param timeout for the asynchronous operation
  * @return Future which is completed with the {@link TransientBlobKey} 
after uploading the file
  * to the {@link BlobServer}.
  */
+@Deprecated
 CompletableFuture requestTaskManagerFileUploadByName(
 ResourceID taskManagerId, String fileName, @RpcTimeout Time 
timeout);
 
+/**
+ * Request the file upload from the given {@link TaskExecutor} to the 
cluster's {@link
+ * BlobServer}. The corresponding {@link TransientBlobKey} is returned.
+ *
+ * @param taskManagerId identifying the {@link TaskExecutor} to upload the 
specified file
+ * @param fileName name of the file to upload
+ * @param fileType type of the file to upload
+ * @param timeout for the asynchronous operation
+ * @return Future which is completed with the {@link TransientBlobKey} 
after uploading the file
+ * to the {@link BlobServer}.
+ */
+CompletableFuture 
requestTaskManagerFileUploadByNameAndType(
+ResourceID taskManagerId, String fileName, FileType fileType, 
@RpcTimeout Time timeout);

Review Comment:
   Same here, I prefer to use `Duration` instead of `Time` here, just as 
[FLINK-14819](https://issues.apache.org/jira/browse/FLINK-14819) did.



##
flink-runtime-web/web-dashboard/src/app/pages/task-manager/profiler/task-manager-profiler.component.html:
##
@@ -0,0 +1,91 @@
+
+
+
+  
+
+  
+Profiling Duration
+
+  
+
+  
+  
+
+  
+Create Profiling Instance
+  
+
+  
+
+ requestThreadDump(
  */
 CompletableFuture 
requestTaskExecutorThreadInfoGateway(
 ResourceID taskManagerId, @RpcTimeout Time timeout);
+
+/**
+ * Request profiling list from the given {@link TaskExecutor}.
+ *
+ * @param taskManagerId identifying the {@link TaskExecutor} to get 
profiling list from
+ * @param timeout for the asynchronous operation
+ * @return Future which is completed with the historical profiling list
+ */
+CompletableFuture> 
requestTaskManagerProfilingList(
+ResourceID taskManagerId, @RpcTimeout Time timeout);
+
+/**
+ * Requests the profiling instance from the given {@link TaskExecutor}.
+ *
+ * @param taskManagerId taskManagerId identifying the {@link TaskExecutor} 
to get the profiling
+ * from
+ * @param duration profiling duration
+ * @param mode profiling mode {@link ProfilingMode}
+ * @param timeout timeout of the asynchronous operation
+ * @return Future containing the created profiling information
+ */
+CompletableFuture requestProfiling(
+ResourceID taskManagerId,
+int duration,
+ProfilingInfo.ProfilingMode mode,
+@RpcTimeout Time timeout);

Review Comment:
   Same suggestion here.



##
flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/TaskExecutorGateway.java:
##
@@ -257,6 +257,18 @@ CompletableFuture 
requestFileUploadByType(
 CompletableFuture requestFileUploadByName(
 String fileName, @RpcTimeout Time timeout);
 
+/**
+ * Requests the file upload of the specified name and file type to the 
cluster's {@link
+ * BlobServer}.
+ *
+ * @param fileName to upload
+ * @param fileType to upload
+ * @param timeout for the asynchronous operation
+ * @return Future which is completed with the {@link TransientBlobKey} of 
the uploaded file.
+ */
+CompletableFuture requestFileUploadByNameAndType(
+String fileName, FileType fileType, @RpcTimeout Time timeout);

Review Comment:
   For newly added interface, I prefer to use `Duration` instead of `Time`, 
just as FLINK-14819 did.



##
flink-runtime/src/main/java/org/apache/flink/runtime/resourcemanager/ResourceManagerGateway.java:
##
@@ -282,4 +284,30 @@ CompletableFuture requestThreadDump(
  */
 CompletableFuture 
requestTaskExecutorThreadInfoGateway(
 ResourceID taskManagerId,

Re: [PR] [FLINK-33434][runtime-web] Support invoke async-profiler on TaskManager via REST API [flink]

2024-01-07 Thread via GitHub 


flinkbot commented on PR #24041:
URL: https://github.com/apache/flink/pull/24041#issuecomment-1880408069

   
   ## CI report:
   
   * 785c67752eaef9d7a13fdd48f613190c9e6ecb82 UNKNOWN
   
   
   Bot commands
 The @flinkbot bot supports the following commands:
   
- `@flinkbot run azure` re-run the last Azure build
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [FLINK-33434][runtime-web] Support invoke async-profiler on TaskManager via REST API [flink]

2024-01-07 Thread via GitHub 


yuchen-ecnu commented on PR #24041:
URL: https://github.com/apache/flink/pull/24041#issuecomment-1880405288

   Hi @Myasuka , do you have time to help review this PR?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org