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.

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

VerifyQueue: re_notify other Worker when OnlySmallCycleTx received a large cycle tx #4591

Merged
merged 2 commits into from
Aug 15, 2024

Conversation

eval-exec
Copy link
Collaborator

@eval-exec eval-exec commented Aug 15, 2024

What problem does this PR solve?

we found SendLargeCyclesTxToRelay is not stable, seems some large cycle tx is not properly processed when replay.

The root cause is when verify_queue wake up a OnlySmallCycleTx to large cycle tx, the worker will directly return, then other worker also miss the chance to pick it up.

Related changes

  • Let Worker::process_inner re_notify other workers if current worker is a OnlySmallCycleTx Worker, but there is large cycle tx in VerifyQueue

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code ci-runs-only: [ quick_checks,linters ]

Side effects

  • None

Release note

Title Only: Include only the PR title in the release note.

@eval-exec eval-exec marked this pull request as ready for review August 15, 2024 11:04
@eval-exec eval-exec requested a review from a team as a code owner August 15, 2024 11:04
@eval-exec eval-exec requested review from zhangsoledad, quake and driftluo and removed request for a team August 15, 2024 11:04
@eval-exec eval-exec added m:tx-pool t:test: Integration Test Type: This issue is related to test. labels Aug 15, 2024
@eval-exec eval-exec changed the title VerifyQueue: notify other worker when OnlySmallCycleTx received a large cycle tx VerifyQueue: re_notify other Worker when OnlySmallCycleTx received a large cycle tx Aug 15, 2024
@driftluo driftluo added this pull request to the merge queue Aug 15, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 15, 2024
@eval-exec
Copy link
Collaborator Author


--- STDOUT:              ckb-script verify::tests::ckb_2021::features_since_v2023::check_infinite_exec ---

running 1 test
test verify::tests::ckb_2021::features_since_v2023::check_infinite_exec has been running for over 60 seconds
test verify::tests::ckb_2021::features_since_v2023::check_infinite_exec ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 404 filtered out; finished in 71.69s


     SIGABRT [   3.389s] ckb-sync tests::synchronizer::functions::test_sync_process

--- STDOUT:              ckb-sync tests::synchronizer::functions::test_sync_process ---

running 1 test
test tests::synchronizer::functions::test_sync_process ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 68 filtered out; finished in 3.29s


--- STDERR:              ckb-sync tests::synchronizer::functions::test_sync_process ---
pthread lock: Invalid argument

@eval-exec
Copy link
Collaborator Author

eval-exec commented Aug 15, 2024

Retry merge...

@eval-exec eval-exec added this pull request to the merge queue Aug 15, 2024
@chenyukang
Copy link
Collaborator

emm, what this error...

@eval-exec
Copy link
Collaborator Author

eval-exec commented Aug 15, 2024 via email

Merged via the queue into nervosnetwork:develop with commit 3c9409e Aug 15, 2024
35 checks passed
@doitian doitian mentioned this pull request Aug 19, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
m:tx-pool t:test: Integration Test Type: This issue is related to test.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants