Skip to content

Update txworker pool definition#153

Merged
yangsw26 merged 1 commit into
mainfrom
update_txworker_pool
Oct 9, 2025
Merged

Update txworker pool definition#153
yangsw26 merged 1 commit into
mainfrom
update_txworker_pool

Conversation

@yangsw26

@yangsw26 yangsw26 commented Oct 9, 2025

Copy link
Copy Markdown
Collaborator

In the function signature, add a size_t parameter to identify the worker id.

Summary by CodeRabbit

  • New Features

    • None.
  • Refactor

    • Unified background task execution to include worker identifiers, aligning all internal jobs with a consistent signature. No user-visible behavior changes.
  • Stability

    • Harmonized asynchronous operations across components to reduce inconsistencies and potential edge cases.
  • Chores

    • Standardized internal worker submission interfaces across modules for improved maintainability.

@coderabbitai

coderabbitai Bot commented Oct 9, 2025

Copy link
Copy Markdown

Walkthrough

The worker pool API changed to require work items with a size_t parameter, propagating to the internal queue and worker-thread invocation. Implementation passes each worker’s ID to the work item. All call sites updated to provide lambdas accepting size_t; lambda bodies remain unchanged otherwise.

Changes

Cohort / File(s) Summary of edits
Public API: TxWorkerPool signature and queue type
include/tx_worker_pool.h
SubmitWork signature changed from std::function<void()> to std::function<void(size_t)>. Internal work_queue_ element type updated accordingly.
Worker pool implementation
src/tx_worker_pool.cpp
Worker thread now takes and passes worker_id to work items; queue element type updated to std::function<void(size_t)>; SubmitWork updated to new signature; threads launched with index argument.
Call-site lambda signature updates (Tx worker submissions)
src/fault/cc_node.cpp, src/sharder.cpp, src/store/snapshot_manager.cpp, src/tx_operation.cpp
Lambdas passed to SubmitWork (and related worker submissions) now accept a size_t parameter; bodies unchanged, parameter often unused.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Caller
  participant P as TxWorkerPool
  participant Q as Work Queue
  participant W as Worker Thread (id)
  participant F as Work Item (fn(size_t))

  C->>P: SubmitWork(F)
  P->>Q: Enqueue F
  Note over P,Q: Queue stores std::function<void(size_t)>

  par Workers
    loop Each worker thread
      W->>Q: Dequeue F (if available)
      Note over W: worker_id = thread index
      W->>F: Invoke F(worker_id)
    end
  end

  Note over F: Call sites may ignore the parameter
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

A bunny cues the threads in line,
“Pass the ID, and all is fine!”
With tiny paws, I tweak each call,
size_t crumbs for workers all.
The queue now hums in numbered cheer—
hop, hop, ship! The change is clear. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The description does not follow the repository’s required template and is missing the checklist items for adding tests, documenting changes, referencing the related issue or RFC, and confirming that the test suite passes. Please update the description to include the repository’s checklist from the template, such as adding tests, documenting the change, linking to the relevant issue or RFC, and confirming that all tests pass.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title “Update txworker pool definition” refers to the primary change of modifying the TxWorkerPool interface, but it is somewhat broad and does not explicitly mention the addition of the worker_id parameter or the SubmitWork signature change.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch update_txworker_pool

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4c571f8 and 6952cc5.

📒 Files selected for processing (6)
  • include/tx_worker_pool.h (2 hunks)
  • src/fault/cc_node.cpp (1 hunks)
  • src/sharder.cpp (2 hunks)
  • src/store/snapshot_manager.cpp (1 hunks)
  • src/tx_operation.cpp (2 hunks)
  • src/tx_worker_pool.cpp (4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
include/tx_worker_pool.h (1)
src/tx_worker_pool.cpp (2)
  • SubmitWork (80-89)
  • SubmitWork (80-80)
🔇 Additional comments (10)
src/store/snapshot_manager.cpp (1)

652-653: LGTM! Lambda signature updated correctly.

The lambda signature is updated to accept the new size_t worker ID parameter. Since HandleBackupTask doesn't require the worker ID, the parameter is correctly left unnamed.

src/fault/cc_node.cpp (1)

700-701: LGTM! Lambda signature updated correctly.

The lambda signature is updated to accept the new size_t worker ID parameter. Since SubscribePrimaryNode doesn't require the worker ID, the parameter is correctly left unnamed.

include/tx_worker_pool.h (2)

44-44: LGTM! Public API updated to pass worker ID.

The SubmitWork method signature is updated to accept std::function<void(size_t)>, allowing work items to receive the worker ID when executed.


55-55: LGTM! Internal queue type matches the updated API.

The work_queue_ type is updated to std::deque<std::function<void(size_t)>>, consistent with the new SubmitWork signature.

src/tx_worker_pool.cpp (4)

34-34: LGTM! Worker lambda signature updated to accept worker ID.

The worker lambda now accepts a size_t worker_id parameter, which is passed from the thread constructor and used to invoke work items with the worker's ID.


52-57: LGTM! Work execution updated to pass worker ID.

The work item type and invocation are correctly updated to pass the worker_id to each submitted work function.


68-69: LGTM! Thread construction passes worker ID.

The thread is now constructed with the loop index i as an argument, which becomes the worker_id parameter for the worker lambda. This ensures each worker has a unique ID.


80-80: LGTM! Implementation matches header declaration.

The SubmitWork implementation signature is updated to match the header declaration, accepting std::function<void(size_t)>.

src/sharder.cpp (2)

668-668: LGTM!

The lambda signature correctly updated to accept the new size_t worker ID parameter. The parameter is appropriately unused here since this task doesn't require knowledge of which worker executes it.


1429-1429: LGTM!

The lambda signature correctly updated to accept the new size_t worker ID parameter. The parameter is appropriately unused since the sharder worker pool is single-threaded (line 227 shows size 1), so the worker ID would always be 0.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@yangsw26 yangsw26 requested a review from liunyl October 9, 2025 05:02
@yangsw26 yangsw26 merged commit 055e500 into main Oct 9, 2025
4 checks passed
@yangsw26 yangsw26 deleted the update_txworker_pool branch October 9, 2025 08:47
@coderabbitai coderabbitai Bot mentioned this pull request Oct 20, 2025
5 tasks
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