Skip to content

Conversation

Oscarcheng0312
Copy link
Contributor

  • I have registered the PR changes.

Ⅰ. Describe what this PR did

This PR introduces a new SQL monitoring utility to the seata-common module:

  • SqlMonitor: a singleton class for collecting SQL execution statistics.
    • Tracks slow SQL (execution time > threshold), up to maxSlowEntries
    • Maintains histogram of transaction execution time
    • Maintains histogram of connection hold time
    • Thread-safe using concurrent data structures and explicit locking
    • Includes setters setSlowThreshold() and setMaxSlowEntries() for future dynamic configuration
  • SlowSqlEntry: a simple immutable model class holding slow SQL info (text, exec time, timestamp)

Ⅱ. Does this pull request fix one issue?

No. This is a new feature and the first step in the SQL monitoring implementation plan.

Ⅲ. Why don't you add test cases (unit test/integration test)?

Test cases are included:

  • SqlMonitorTest.java covers:
    • Slow SQL recording and eviction behavior
    • Histogram bucket logic for transaction/hold times
    • Fast SQL skip logic
    • Queue size trimming

Ⅳ. Describe how to verify it

  • Run SqlMonitorTest with JUnit
  • Verify correctness of slow SQL detection, histogram aggregation, and bounded queue behavior

Ⅴ. Special notes for reviews

  • This is PR 1/3 in the SQL monitoring feature set
  • Follow-up PRs will integrate this utility with seata-rm-datasource (recording) and seata-server (metrics exposure)
  • Configuration injection will be addressed later via application.yml or dynamic configuration center

@Oscarcheng0312
Copy link
Contributor Author

Hi @slievrly, this PR adds SqlMonitor and SlowSqlEntry to support SQL execution monitoring (slow SQL tracking + histograms).
Would you mind reviewing it when convenient? This is the first part of the GSoC connection pool monitoring plan. Thanks!

Copy link

codecov bot commented Jul 2, 2025

Codecov Report

❌ Patch coverage is 90.32258% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.40%. Comparing base (db73418) to head (74437dc).
⚠️ Report is 27 commits behind head on 2.x.

Files with missing lines Patch % Lines
...va/org/apache/seata/common/monitor/SqlMonitor.java 92.45% 4 Missing ⚠️
.../org/apache/seata/common/monitor/SlowSqlEntry.java 77.77% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                2.x    #7491      +/-   ##
============================================
+ Coverage     60.34%   60.40%   +0.05%     
  Complexity      658      658              
============================================
  Files          1284     1286       +2     
  Lines         48465    48527      +62     
  Branches       5694     5702       +8     
============================================
+ Hits          29247    29311      +64     
+ Misses        16600    16599       -1     
+ Partials       2618     2617       -1     
Files with missing lines Coverage Δ
.../org/apache/seata/common/monitor/SlowSqlEntry.java 77.77% <77.77%> (ø)
...va/org/apache/seata/common/monitor/SqlMonitor.java 92.45% <92.45%> (ø)

... and 9 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Oscarcheng0312 Oscarcheng0312 changed the title feat(common): add SqlMonitor and SlowSqlEntry for SQL execution monitoring feature: add SqlMonitor and SlowSqlEntry for SQL execution monitoring Jul 2, 2025
@Oscarcheng0312
Copy link
Contributor Author

@slievrly Hi, pls take a look when you are convenient.

return new LinkedHashMap<>(holdHistogram);
}

private String chooseTxnBucket(long ms) {
Copy link
Member

Choose a reason for hiding this comment

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

This is not a formed partial function, so the significance of the review is not great. You can first merge it into your own repository for development or merge it into the GSoC-conn branch of this repository. The time slice defined here is too rigid. Statistical indicators should be calculated in the console. The client side only needs to report the original values; otherwise, it will lead to distortion of data accuracy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants