Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Enhancement] Introduce TxnStateDispatcher for merge commit sync mode #55001 #55071

Merged
merged 6 commits into from
Jan 15, 2025
Merged

[Enhancement] Introduce TxnStateDispatcher for merge commit sync mode #55001 #55071

merged 6 commits into from
Jan 15, 2025

Conversation

banmoy
Copy link
Contributor

@banmoy banmoy commented Jan 14, 2025 •
edited
Loading

Why I'm doing:

This is the second PR of merge commit sync mode optimization #54995. Introduce TxnStateDispatcher on FE side. You can see #54995 for details

What I'm doing:

Fixes #issue

What type of PR is this:

  • BugFix
  • Feature
  • Enhancement
  • Refactor
  • UT
  • Doc
  • Tool

Does this PR entail a change in behavior?

  • Yes, this PR will result in a change in behavior.
  • No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

  • Interface/UI changes: syntax, type conversion, expression evaluation, display information
  • Parameter changes: default values, similar parameters but with different default values
  • Policy changes: use new policy to replace old one, functionality automatically enabled
  • Feature removed
  • Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

  • I have added test cases for my bug fix or my new feature
  • This pr needs user documentation (for new or modified features or behaviors)
    • I have added documentation for my new feature or new function
  • This is a backport pr

Bugfix cherry-pick branch check:

  • I have checked the version labels which the pr will be auto-backported to the target branch
    • 3.4
    • 3.3
    • 3.2
    • 3.1
    • 3.0

@banmoy banmoy requested a review from a team as a code owner January 14, 2025 10:53
@mergify mergify bot assigned banmoy Jan 14, 2025
@banmoy banmoy mentioned this pull request Jan 14, 2025
Open
@wanpengfei-git wanpengfei-git requested a review from a team January 14, 2025 10:59
@banmoy banmoy force-pushed the txn_state_dispatcher branch from dd6e11c to 2136860 Compare January 14, 2025 11:58
@banmoy banmoy force-pushed the txn_state_dispatcher branch from 2136860 to 7f09a8f Compare January 14, 2025 12:22
gengjun-git
gengjun-git reviewed Jan 15, 2025
View reviewed changes
fe/fe-core/src/main/java/com/starrocks/common/Config.java
@ConfField(mutable = true)
public static int merge_commit_txn_state_dispatch_retry_interval_ms = 200;

public static int merge_commit_be_assigner_schedule_interval_ms = 5000;
Copy link
Contributor

Choose a reason for hiding this comment

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

Must add @confField

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

wyb
wyb approved these changes Jan 15, 2025
View reviewed changes
fe/fe-core/src/main/java/com/starrocks/load/batchwrite/TxnStateDispatcher.java
Comment on lines +30 to +31
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

@wyb wyb enabled auto-merge (squash) January 15, 2025 07:03
banmoy added 6 commits January 15, 2025 15:25
@banmoy
Add FE implementation
00493f7 
Signed-off-by: PengFei Li <[email protected]>
@banmoy
FE rename some configs
81490e4 
Signed-off-by: PengFei Li <[email protected]>
@banmoy
FE fix some core case
9ce1a43 
Signed-off-by: PengFei Li <[email protected]>
@banmoy
Add proto
02c3ba1 
Signed-off-by: PengFei Li <[email protected]>
@banmoy
Add TODO
9f9e80a 
Signed-off-by: PengFei Li <[email protected]>
@banmoy
Fix ConfField
26e5e5a 
Signed-off-by: PengFei Li <[email protected]>
auto-merge was automatically disabled January 15, 2025 07:25

Head branch was pushed to by a user without write access

@banmoy banmoy force-pushed the txn_state_dispatcher branch from d77f521 to 26e5e5a Compare January 15, 2025 07:25
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Failed Quality Gate failed

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

See analysis details on SonarQube Cloud

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

@github-actions GitHub Actions
Copy link

[Java-Extensions Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

@github-actions GitHub Actions
Copy link

[FE Incremental Coverage Report]

✅ pass : 166 / 183 (90.71%)

file detail

path covered_line new_line coverage not_covered_line_detail
🔵 com/starrocks/transaction/TransactionStatus.java 3 8 37.50% [78, 81, 87, 90, 92]
🔵 com/starrocks/load/batchwrite/IsomorphicBatchWrite.java 10 14 71.43% [254, 271, 272, 273]
🔵 com/starrocks/load/batchwrite/LoadExecutor.java 10 11 90.91% [269]
🔵 com/starrocks/load/batchwrite/TxnStateDispatcher.java 131 138 94.93% [68, 89, 90, 122, 123, 203, 206]
🔵 com/starrocks/common/Config.java 7 7 100.00% []
🔵 com/starrocks/load/batchwrite/BatchWriteMgr.java 3 3 100.00% []
🔵 com/starrocks/load/batchwrite/CoordinatorBackendAssignerImpl.java 2 2 100.00% []

@github-actions GitHub Actions
Copy link

[BE Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

@gengjun-git gengjun-git merged commit d650aa8 into StarRocks:main Jan 15, 2025
48 of 49 checks passed
banmoy added a commit to banmoy/starrocks that referenced this pull request Jan 15, 2025
@banmoy
[Enhancement] Introduce TxnStateDispatcher for merge commit sync mode S…
6b5ce72 
…tarRocks#55001 (StarRocks#55071)

This is the second PR of merge commit sync mode optimization StarRocks#54995. Introduce TxnStateDispatcher on FE side. You can see StarRocks#54995 for details

Signed-off-by: PengFei Li <[email protected]>
@banmoy banmoy mentioned this pull request Jan 15, 2025
Merged
24 tasks
banmoy added a commit to banmoy/starrocks that referenced this pull request Jan 15, 2025
@banmoy
[Enhancement] Introduce TxnStateDispatcher for merge commit sync mode S…
5af0671 
…tarRocks#55001 (StarRocks#55071)

This is the second PR of merge commit sync mode optimization StarRocks#54995. Introduce TxnStateDispatcher on FE side. You can see StarRocks#54995 for details

Signed-off-by: PengFei Li <[email protected]>
@banmoy banmoy mentioned this pull request Jan 15, 2025
Merged
24 tasks
gengjun-git pushed a commit that referenced this pull request Jan 15, 2025
@banmoy
[Enhancement] Introduce TxnStateDispatcher for merge commit sync mode…
65807f7 
… (backport #55071)  (#55115)

Signed-off-by: PengFei Li <[email protected]>
gengjun-git pushed a commit that referenced this pull request Jan 15, 2025
@banmoy
[Enhancement] Introduce TxnStateDispatcher for merge commit sync mode…
d3ce81f 
… (backport #55071) (#55114)

Signed-off-by: PengFei Li <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@starrocks-cr starrocks-cr[bot] starrocks-cr[bot] left review comments

@gengjun-git gengjun-git gengjun-git approved these changes

@wyb wyb wyb approved these changes

Assignees

@banmoy banmoy

Labels
3.4-merged PROTO-REVIEW
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@banmoy @wyb @gengjun-git @wanpengfei-git