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

fix(test): truncate the number of transactions in send transaction test #4848

Merged
merged 2 commits into from
Aug 1, 2022
Merged

fix(test): truncate the number of transactions in send transaction test #4848

merged 2 commits into from
Aug 1, 2022

Conversation

oxarbitrage
Copy link
Contributor

Motivation

We are seeing some full queue errors in some runs of the send transaction test. Close #4847

Solution

Truncate the transactions sent in this test. I think this should work but i need to test in CI.

Review

Anyone can review.

Reviewer Checklist

  • send_transaction_test pass
  • All the other tests pass

@oxarbitrage oxarbitrage requested review from a team as code owners July 31, 2022 22:10
@oxarbitrage oxarbitrage requested review from teor2345 and removed request for a team July 31, 2022 22:10
teor2345
teor2345 approved these changes Jul 31, 2022
View reviewed changes
Copy link
Contributor

@teor2345 teor2345 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The error message:

transaction dropped because the queue is full

comes from the RPC queue, not the mempool queue.

So I think we should limit it based on both queues.
I pushed a commit cfe3ebf.

@teor2345 teor2345 added C-bug Category: This is a bug P-Critical 🚑 I-integration-fail Continuous integration fails, including build and test failures C-testing Category: These are tests A-rpc Area: Remote Procedure Call interfaces lightwalletd any work associated with lightwalletd labels Jul 31, 2022
@teor2345 teor2345 mentioned this pull request Jul 31, 2022
Closed
3 tasks
@codecov
Copy link

codecov bot commented Jul 31, 2022 •
edited
Loading

Codecov Report

Merging #4848 (cfe3ebf) into main (61f3639) will decrease coverage by 0.09%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #4848      +/-   ##
==========================================
- Coverage   78.84%   78.74%   -0.10%     
==========================================
  Files         305      305              
  Lines       38730    38742      +12     
==========================================
- Hits        30537    30508      -29     
- Misses       8193     8234      +41

mergify bot added a commit that referenced this pull request Aug 1, 2022
@mergify
Merge of #4848
043c18e
@mergify mergify bot mentioned this pull request Aug 1, 2022
Closed
@mergify mergify bot merged commit a6f15e8 into main Aug 1, 2022
@mergify mergify bot deleted the issue4847 branch August 1, 2022 03:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@teor2345 teor2345 teor2345 approved these changes

Assignees

@teor2345 teor2345

@oxarbitrage oxarbitrage

Labels
A-rpc Area: Remote Procedure Call interfaces C-bug Category: This is a bug C-testing Category: These are tests I-integration-fail Continuous integration fails, including build and test failures lightwalletd any work associated with lightwalletd
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Limit the number of transactions sent in the lightwalletd send transaction test
2 participants
@oxarbitrage @teor2345