Skip to content

Conversation

d4mr
Copy link
Member

@d4mr d4mr commented Oct 3, 2025

Summary by CodeRabbit

  • Bug Fixes

    • Prevents premature job deletion by checking active, pending, and delayed states before pruning.
    • Adds lane-aware pruning to avoid removing in-flight lane jobs.
    • Returns actual deletions count for accurate metrics and observability.
    • Improves stability under concurrent processing and aggressive pruning.
  • Refactor

    • Consolidates error handling and balance-threshold updates for cleaner, equivalent behavior.
  • Tests

    • Adds a race-condition test simulating concurrent workers and pruning to validate safety.

…ng, or delayed jobs. Update EOA executor error handling for balance threshold updates.
Copy link

coderabbitai bot commented Oct 3, 2025

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately reflects the primary objective of the changeset, which is to address a concurrency issue in the TWMQ pruning logic by introducing guards against race conditions during job deletion, and it does so in a concise and clear manner.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ 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 pb/twmq-pruning-fix

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 7810d96 and 94de992.

📒 Files selected for processing (4)
  • executors/src/eoa/worker/send.rs (2 hunks)
  • twmq/src/lib.rs (4 hunks)
  • twmq/src/multilane.rs (3 hunks)
  • twmq/tests/prune_race_condition.rs (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-20T05:30:35.171Z
Learnt from: joaquim-verges
PR: thirdweb-dev/engine-core#48
File: executors/src/eoa/worker/send.rs:20-21
Timestamp: 2025-09-20T05:30:35.171Z
Learning: In executors/src/eoa/worker/send.rs, there is a critical bug where HEALTH_CHECK_INTERVAL is defined as 300 seconds but compared against millisecond timestamps, causing balance checks to occur every 300ms instead of every 5 minutes (1000x more frequent than intended).

Applied to files:

  • executors/src/eoa/worker/send.rs
🧬 Code graph analysis (2)
executors/src/eoa/worker/send.rs (1)
executors/src/eoa/worker/error.rs (1)
  • should_update_balance_threshold (215-235)
twmq/tests/prune_race_condition.rs (2)
twmq/src/lib.rs (8)
  • redis (1221-1223)
  • redis (1229-1229)
  • redis (1321-1323)
  • redis (1329-1329)
  • redis (1359-1361)
  • redis (1367-1369)
  • job (157-162)
  • new (132-151)
twmq/src/multilane.rs (6)
  • redis (1271-1273)
  • redis (1278-1278)
  • redis (1378-1380)
  • redis (1385-1385)
  • new (47-64)
  • id (1435-1438)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test
  • GitHub Check: coverage
🔇 Additional comments (1)
executors/src/eoa/worker/send.rs (1)

16-56: Five-minute refresh now aligns with ms timestamps

Thanks for converting the health check interval to milliseconds; this closes out the earlier 300 ms misfire we tracked for this worker. Based on learnings


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

@d4mr d4mr merged commit 3774a4b into main Oct 3, 2025
4 of 5 checks passed
@d4mr d4mr deleted the pb/twmq-pruning-fix branch October 3, 2025 10:47
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.

1 participant