You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@flink.apache.org by "yunfengzhou-hub (via GitHub)" <gi...@apache.org> on 2025/08/28 07:52:33 UTC

[PR] [FLINK-38291][REST] Reduce thread lock overhead for REST handlers [flink]

yunfengzhou-hub opened a new pull request, #26951:
URL: https://github.com/apache/flink/pull/26951

## What is the purpose of the change

This PR optimizes the latency of Flink REST handlers used to generate the DAG in Flink UI.

In the current implementation, REST handlers like `JobDetailsHandle`r would iterate through all vertexes of a job, and invoke `MetricStore#getSubtaskAttemptMetricStore` during each iteration. Given that this is a synchronized method, invocations to this method could possibly be blocked until other threads finished invoking other synchronized methods. This blocking overhead is accumulated with the for loop, resulting in high latency when Flink UI tries to render the status of a Flink job through `JobDetailsHandler`.

In order to solve this problem, this PR proposes to reduce the number of synchronized invocations in REST handlers. A snapshot of the MetricStore jobs is acquired for each handler (and the synchronization overhead is accumulated only once here), and the snapshot is then reused in the for loops. The snapshot is read only so it needs not be synchronized.

As for benchmark results, we manually measured the latency for the Flink UI to display the DAG of a sophisticated Flink job in our company. Before optimization, the Flink UI needs more than 1 minute to finish the display. After the optimization, the latency decreased to less than 10 seconds.

## Brief change log

- Introduce MetricStore.MetricStoreJobs to manage a snapshot of all jobs in the MetricStore. Compared with original implementation to operate on MetricStore jobs, the new implementation does not need synchronized keywords on the methods.

## Verifying this change

The correctness of this PR is covered by existing tests, such as JobDetailsHandlerTest and MetricStoreTest.

## 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, Kubernetes/Yarn, 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

--
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-unsubscribe@flink.apache.org

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

Re: [PR] [FLINK-38291][REST] Reduce thread lock overhead for REST handlers [flink]

Posted by "yunfengzhou-hub (via GitHub)" <gi...@apache.org>.

yunfengzhou-hub commented on PR #26951:
URL: https://github.com/apache/flink/pull/26951#issuecomment-3252515638

   @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-unsubscribe@flink.apache.org

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

Re: [PR] [FLINK-38291][REST] Reduce thread lock overhead for REST handlers [flink]

Posted by "yunfengzhou-hub (via GitHub)" <gi...@apache.org>.

yunfengzhou-hub commented on PR #26951:
URL: https://github.com/apache/flink/pull/26951#issuecomment-3235376543

   Hi @Sxnan could you please 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-unsubscribe@flink.apache.org

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

Re: [PR] [FLINK-38291][REST] Reduce thread lock overhead for REST handlers [flink]

Posted by "Sxnan (via GitHub)" <gi...@apache.org>.

Sxnan commented on PR #26951:
URL: https://github.com/apache/flink/pull/26951#issuecomment-3251370204

   @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-unsubscribe@flink.apache.org

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

Re: [PR] [FLINK-38291][REST] Reduce thread lock overhead for REST handlers [flink]

Posted by "yunfengzhou-hub (via GitHub)" <gi...@apache.org>.

yunfengzhou-hub commented on PR #26951:
URL: https://github.com/apache/flink/pull/26951#issuecomment-3248715529

   Thanks for the comments @Sxnan . I have updated the PR according to the comments.


-- 
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-unsubscribe@flink.apache.org

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

Re: [PR] [FLINK-38291][REST] Reduce thread lock overhead for REST handlers [flink]

Posted by "Sxnan (via GitHub)" <gi...@apache.org>.

Sxnan merged PR #26951:
URL: https://github.com/apache/flink/pull/26951


-- 
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-unsubscribe@flink.apache.org

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

Re: [PR] [FLINK-38291][REST] Reduce thread lock overhead for REST handlers [flink]

Posted by "flinkbot (via GitHub)" <gi...@apache.org>.

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

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "628864570c6824ec15bc2c944e82afccfc395a48",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "628864570c6824ec15bc2c944e82afccfc395a48",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 628864570c6824ec15bc2c944e82afccfc395a48 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run azure` re-run the last Azure build
   </details>


-- 
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-unsubscribe@flink.apache.org

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

Re: [PR] [FLINK-38291][REST] Reduce thread lock overhead for REST handlers [flink]

Posted by "Sxnan (via GitHub)" <gi...@apache.org>.

Sxnan commented on code in PR #26951:
URL: https://github.com/apache/flink/pull/26951#discussion_r2317644556


##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/handler/legacy/metrics/MetricStore.java:
##########
@@ -689,4 +650,57 @@ private static SubtaskMetricStore unmodifiable(SubtaskMetricStore source) {
                     unmodifiableMap(source.metrics), unmodifiableMap(source.attempts));
         }
     }
+
+    /** Sub-structure containing a snapshot of all jobs in the metric store. */
+    @ThreadSafe
+    public static class MetricStoreJobs {

Review Comment:
   Maybe name it `JobMetricStores` or `JobMetricStoresSnapshot`. 



##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/handler/job/JobDetailsHandler.java:
##########
@@ -100,7 +102,7 @@ public Collection<ArchivedJson> archiveJsonWithPath(AccessExecutionGraph graph)
     }
 
     private static JobDetailsInfo createJobDetailsInfo(
-            AccessExecutionGraph executionGraph, @Nullable MetricFetcher metricFetcher) {
+            AccessExecutionGraph executionGraph, @Nullable MetricStore.MetricStoreJobs jobs) {

Review Comment:
   The name `jobs` is too general and may be confusing. The `MetricsStoreJobs` is actually a snapshot of the metrics of all jobs. We may name it `jobMetrics`.



-- 
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-unsubscribe@flink.apache.org

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

Re: [PR] [FLINK-38291][REST] Reduce thread lock overhead for REST handlers [flink]

Posted by "yunfengzhou-hub (via GitHub)" <gi...@apache.org>.

yunfengzhou-hub commented on PR #26951:
URL: https://github.com/apache/flink/pull/26951#issuecomment-3251721084

   @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-unsubscribe@flink.apache.org

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