Skip to content

Conversation

@ruanwenjun
Copy link
Member

@ruanwenjun ruanwenjun commented Jun 6, 2024

Purpose of the pull request

close #16119

Brief change log

  • Change default sync request timeout to 10s
  • Set heartbeat interval to 10s, connectionIdleTime time to 60s
  • Expose metrics for rpc exception/cost

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(or)

If your pull request contain incompatible change, you should also add it to docs/docs/en/guide/upgrede/incompatible.md

@ruanwenjun ruanwenjun requested a review from caishunfeng as a code owner June 6, 2024 08:14
@ruanwenjun ruanwenjun self-assigned this Jun 6, 2024
@ruanwenjun ruanwenjun force-pushed the dev_wenjun_addRpcMetrics branch 2 times, most recently from 9a9742b to ed03189 Compare June 6, 2024 10:40
rickchengx
rickchengx previously approved these changes Jun 6, 2024
Copy link
Contributor

@rickchengx rickchengx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ruanwenjun ruanwenjun force-pushed the dev_wenjun_addRpcMetrics branch from 6e3b139 to 2880f36 Compare June 6, 2024 11:09
@ruanwenjun ruanwenjun added the improvement make more easy to user or prompt friendly label Jun 6, 2024
@ruanwenjun ruanwenjun added this to the 3.3.0 milestone Jun 6, 2024
Copy link
Member

@SbloodyS SbloodyS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI did not passed.

SbloodyS
SbloodyS previously approved these changes Jun 7, 2024
Copy link
Member

@SbloodyS SbloodyS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@codecov-commenter
Copy link

codecov-commenter commented Jun 7, 2024

Codecov Report

Attention: Patch coverage is 59.04762% with 43 lines in your changes missing coverage. Please review.

Project coverage is 40.70%. Comparing base (d6714bb) to head (455a049).

Current head 455a049 differs from pull request most recent head 7ef6c6c

Please upload reports for the commit 7ef6c6c to get more accurate results.

Files Patch % Lines
...duler/extract/base/client/NettyRemotingClient.java 43.24% 17 Missing and 4 partials ⚠️
...phinscheduler/extract/base/metrics/RpcMetrics.java 73.33% 11 Missing and 1 partial ⚠️
...tract/base/metrics/ClientSyncExceptionMetrics.java 0.00% 5 Missing ⚠️
...r/extract/base/server/JdkDynamicServerHandler.java 0.00% 4 Missing ⚠️
...eduler/extract/base/client/NettyClientHandler.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                dev   #16121      +/-   ##
============================================
+ Coverage     40.66%   40.70%   +0.03%     
- Complexity     5238     5245       +7     
============================================
  Files          1382     1385       +3     
  Lines         46026    46109      +83     
  Branches       4920     4923       +3     
============================================
+ Hits          18718    18770      +52     
- Misses        25384    25412      +28     
- Partials       1924     1927       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ruanwenjun ruanwenjun force-pushed the dev_wenjun_addRpcMetrics branch from 516c2d3 to 7ef6c6c Compare June 7, 2024 05:30
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 7, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

@ruanwenjun ruanwenjun requested a review from SbloodyS June 7, 2024 06:28
Copy link
Contributor

@rickchengx rickchengx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@ruanwenjun ruanwenjun merged commit fd5a182 into apache:dev Jun 11, 2024
@ruanwenjun ruanwenjun deleted the dev_wenjun_addRpcMetrics branch June 11, 2024 02:57
@SbloodyS SbloodyS changed the title Add RPC metrics [Improvement] Add RPC metrics Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3.3.0 backend document improvement make more easy to user or prompt friendly

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Improvement] Add metrics for RPC module

4 participants