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

Problems with the congestion_control.py test #12305

Closed
jancionear opened this issue Oct 24, 2024 · 8 comments
Closed

Problems with the congestion_control.py test #12305

jancionear opened this issue Oct 24, 2024 · 8 comments
Assignees

Comments

@jancionear
Copy link
Contributor

jancionear commented Oct 24, 2024

Description

The pytest/tests/sanity/congestion_control.py test seems to be flaky.
It failed in my nayduck run, so I tried to run it locally, but I can't even get it to pass on master (b27295b).

I did a test on n2d-standard8 GCP VM:

git clone https://github.com/near/nearcore
cd nearcore/
python3 -m venv venv
source venv/bin/activate
pip install -r pytest/requirements.txt
cargo build -pneard --features test_features,rosetta_rpc
cargo build -pgenesis-populate -prestaked -pnear-test-contracts
python3 pytest/tests/sanity/congestion_control.py

I ran the test three times and each time it failed with:

======================================================================
FAIL: test (__main__.CongestionControlTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/tmp/nearcore/pytest/tests/sanity/congestion_control.py", line 83, in test
    self.__run(node, accounts)
  File "/tmp/nearcore/pytest/tests/sanity/congestion_control.py", line 93, in __run
    self.__run_under_congestion(node)
  File "/tmp/nearcore/pytest/tests/sanity/congestion_control.py", line 115, in __run_under_congestion
    self.assertGreaterEqual(gas_used, 1000 * TGAS)
AssertionError: 12831110967700 not greater than or equal to 1000000000000000

----------------------------------------------------------------------
Ran 1 test in 14.342s

In my nayduck run It failed with a different error:

F
======================================================================
FAIL: test (__main__.CongestionControlTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/datadrive/nearcore/pytest/tests/sanity/congestion_control.py", line 83, in test
    self.__run(node, accounts)
  File "/datadrive/nearcore/pytest/tests/sanity/congestion_control.py", line 99, in __run
    self.__check_txs(node)
  File "/datadrive/nearcore/pytest/tests/sanity/congestion_control.py", line 195, in __check_txs
    self.assertGreater(rejected_count, 0)
AssertionError: 0 not greater than 0

----------------------------------------------------------------------
Ran 1 test in 69.434s
@wacban
Copy link
Contributor

wacban commented Oct 25, 2024

Thanks, it's probably caused by relaxing the congestion control parameters recently. I'll have a look.

@wacban
Copy link
Contributor

wacban commented Oct 25, 2024

cargo build -pgenesis-populate -prestaked -pnear-test-contracts

Not sure if related but the test contracts should be built with test_features enabled to have the burn_gas method available. If you happen to have the vm around, can you try again after building with that flag?

@wacban
Copy link
Contributor

wacban commented Oct 25, 2024

I should probably add that it passes on my personal machine

@wacban
Copy link
Contributor

wacban commented Oct 25, 2024

Yeah, I can confirm the test fails with the following error when the test contracts are built without test_features. It's kind of annoying so I'll see if I can detect it and fail fast.

======================================================================
FAIL: test (__main__.CongestionControlTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/wacban-near/near/nearcore/pytest/tests/sanity/congestion_control.py", line 83, in test
    self.__run(node, accounts)
  File "/home/wacban-near/near/nearcore/pytest/tests/sanity/congestion_control.py", line 93, in __run
    self.__run_under_congestion(node)
  File "/home/wacban-near/near/nearcore/pytest/tests/sanity/congestion_control.py", line 115, in __run_under_congestion
    self.assertGreaterEqual(gas_used, 1000 * TGAS)
AssertionError: 22689805060680 not greater than or equal to 1000000000000000

@jancionear
Copy link
Contributor Author

jancionear commented Oct 25, 2024

Yeah, I can confirm the test fails with the following error when the test contracts are built without test_features. It's kind of annoying so I'll see if I can detect it and fail fast.

Ah, I took the commands to run the test from nayduck:

image

@jancionear
Copy link
Contributor Author

With test_features enabled I don't get the first error, but I still get the rejected count equal to zero error sometimes.
Doesn't happen every time, but every few runs I see:

======================================================================
FAIL: test (__main__.CongestionControlTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/tmp/nearcore/pytest/tests/sanity/congestion_control.py", line 83, in test
    self.__run(node, accounts)
  File "/tmp/nearcore/pytest/tests/sanity/congestion_control.py", line 99, in __run
    self.__check_txs(node)
  File "/tmp/nearcore/pytest/tests/sanity/congestion_control.py", line 195, in __check_txs
    self.assertGreater(rejected_count, 0)
AssertionError: 0 not greater than 0

----------------------------------------------------------------------

(Tested on gcp VM)

github-merge-queue bot pushed a commit that referenced this issue Oct 28, 2024
Added a fail fast check at the beginning to the test and a nice comment
explaining the most common reason - the test contract built without the
test_features enabled.
 
This addresses the first part of #12305
@wacban
Copy link
Contributor

wacban commented Oct 28, 2024

yeah, I have an idea on how to improve it but I need to code it. I think the bottleneck is the lone node that is taking in all the transactions. I hope that by adding more nodes I can make the chain process more transactions and get congested more reliably.

@jancionear
Copy link
Contributor Author

Fixed in #12331

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

No branches or pull requests

2 participants