Skip to content

v3.0: ConsumeWorker: improve bank check/switch efficiency (backport of #8291)#8315

Closed
mergify[bot] wants to merge 2 commits intov3.0from
mergify/bp/v3.0/pr-8291
Closed

v3.0: ConsumeWorker: improve bank check/switch efficiency (backport of #8291)#8315
mergify[bot] wants to merge 2 commits intov3.0from
mergify/bp/v3.0/pr-8291

Conversation

@mergify
Copy link
Copy Markdown

@mergify mergify Bot commented Oct 2, 2025

Problem

Summary of Changes

  • Fix logic in switching/waiting for new banks
  • Only Arc::clone on actual bank switch.

Fixes #


This is an automatic backport of pull request #8291 done by [Mergify](https://mergify.com).

(cherry picked from commit a58c605)

# Conflicts:
#	core/src/replay_stage.rs
@mergify mergify Bot requested a review from a team as a code owner October 2, 2025 13:50
@mergify mergify Bot added the conflicts label Oct 2, 2025
@mergify
Copy link
Copy Markdown
Author

mergify Bot commented Oct 2, 2025

Cherry-pick of a58c605 has failed:

On branch mergify/bp/v3.0/pr-8291
Your branch is up to date with 'origin/v3.0'.

You are currently cherry-picking commit a58c605a0.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   core/src/banking_stage/consume_worker.rs
	modified:   core/src/banking_stage/decision_maker.rs
	modified:   poh/src/poh_recorder.rs

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   core/src/replay_stage.rs

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@t-nelson
Copy link
Copy Markdown

t-nelson commented Oct 2, 2025

this needs to ride 3.1

@apfitzge apfitzge force-pushed the mergify/bp/v3.0/pr-8291 branch from f13a03a to 7d0a48e Compare October 2, 2025 16:10
@apfitzge
Copy link
Copy Markdown

apfitzge commented Oct 2, 2025

this needs to ride 3.1

@t-nelson git freaked tf out due to a name change of a fn that was used in replay_stage in 3.1 but not in 3.0.
The actual diff is much smaller, which has now been pushed. Zero difference in replay_stage now.

This is fixing a bug that was causing us to load/clone arc-bank twice per batch in block-production in 3.0+. If you still think not worth risk lmk it's unlikely to make a huge impact on perf with current levels of traffic, but seems like a simple fix to me.

@steviez steviez requested a review from alessandrod October 2, 2025 16:14
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 59.09091% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.5%. Comparing base (ac923e4) to head (7d0a48e).

Additional details and impacted files
@@           Coverage Diff           @@
##             v3.0    #8315   +/-   ##
=======================================
  Coverage    83.4%    83.5%           
=======================================
  Files         810      810           
  Lines      365809   365827   +18     
=======================================
+ Hits       305445   305478   +33     
+ Misses      60364    60349   -15     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@t-nelson
Copy link
Copy Markdown

t-nelson commented Oct 2, 2025

well there are no tests either

@t-nelson
Copy link
Copy Markdown

t-nelson commented Oct 8, 2025

i think we only want the bank update logic inversion fix backported. can you take this back to master and put that in separately. other stuff can ride master

@apfitzge
Copy link
Copy Markdown

apfitzge commented Oct 8, 2025

i think we only want the bank update logic inversion fix backported. can you take this back to master and put that in separately. other stuff can ride master

I think without the other changes, just reversing the condition is not very useful. I'm actually fine to just let this ride to master - I hadn't seen this popping up on profiles yet anyway

@apfitzge apfitzge closed this Oct 8, 2025
@yihau yihau deleted the mergify/bp/v3.0/pr-8291 branch December 5, 2025 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants