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

Fix two_node_network connection failure in app_test #2374

Merged
merged 7 commits into from
Mar 9, 2021
Merged

Conversation

abitmore
Copy link
Member

@abitmore abitmore commented Mar 9, 2021

Fixes #39.

By the way, there is another issue in app_test that sometimes the transaction may fail to broadcast / propagate (see #2376 and #2378). Since it's hard to reproduce, it won't be fixed in this pull request if it didn't appear when testing. A new pull request will be needed when it appears again.

@abitmore abitmore added this to the 5.2.0 - Feature Release milestone Mar 9, 2021
@abitmore abitmore linked an issue Mar 9, 2021 that may be closed by this pull request
@abitmore abitmore changed the title Fix app_test Fix two_node_network connection failure in app_test Mar 9, 2021
@abitmore abitmore marked this pull request as ready for review March 9, 2021 15:35
@sonarcloud
Copy link

sonarcloud bot commented Mar 9, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@abitmore
Copy link
Member Author

abitmore commented Mar 9, 2021

The transaction propagation issue appeared.

No propagation:

2021-03-09T17:29:48.0288452Z Creating transfer tx
2021-03-09T17:29:48.0337249Z Pushing tx locally on db1
2021-03-09T17:29:48.0339773Z Broadcasting tx
2021-03-09T17:29:48.0356979Z 1788034ms p2p node.cpp:4682 broadcast ] broadcasting trx: {"trx":{"ref_block_num":0,"ref_block_prefix":0,"expiration":"2021-03-09T17:30:35","operations":[[37,{"fee":{"amount":0,"asset_id":"1.3.0"},"deposit_to_account":"1.2.17","balance_to_claim":"1.15.0","balance_owner_key":"BTS6MRyAjQq8ud7hVNYcfnVPJqcVpscN5So8BhtHuGYqET5GDW5CV","total_claimed":{"amount":"1000000000000000","asset_id":"1.3.0"}}],[0,{"fee":{"amount":2000000,"asset_id":"1.3.0"},"from":"1.2.17","to":"1.2.3","amount":{"amount":1000000,"asset_id":"1.3.0"},"extensions":[]}]],"extensions":[],"signatures":["205d73bd5e245ccf4ce3ffadb164fe77c322d9677a585835a85ac8193a38619e320e971e068e2475cd821d7934f8165505c86db194bcfc8b2c03f7abfdb1a785f8"]}}
2021-03-09T17:29:48.0365201Z 1788035ms p2p node.cpp:780 advertise_inventory_ ] beginning an iteration of advertise inventory
2021-03-09T17:29:48.0366973Z 1788035ms p2p node.cpp:794 advertise_inventory_ ] peer->peer_needs_sync_items_from_us: true
2021-03-09T17:29:48.0368501Z 1788035ms p2p peer_connection.cpp:497 clear_old_inventory ] Expiring old inventory for peer 127.0.0.1:40475: removing 0 items advertised to peer (0 left), and 0 advertised to us (0 left)

Normal propagation:

2021-03-09T17:21:10.0929382Z Creating transfer tx
2021-03-09T17:21:10.0975194Z Pushing tx locally on db1
2021-03-09T17:21:10.0988200Z 1270097ms p2p node.cpp:4682 broadcast ] broadcasting trx: {"trx":{"ref_block_num":0,"ref_block_prefix":0,"expiration":"2021-03-09T17:21:55","operations":[[37,{"fee":{"amount":0,"asset_id":"1.3.0"},"deposit_to_account":"1.2.17","balance_to_claim":"1.15.0","balance_owner_key":"BTS6MRyAjQq8ud7hVNYcfnVPJqcVpscN5So8BhtHuGYqET5GDW5CV","total_claimed":{"amount":"1000000000000000","asset_id":"1.3.0"}}],[0,{"fee":{"amount":2000000,"asset_id":"1.3.0"},"from":"1.2.17","to":"1.2.3","amount":{"amount":1000000,"asset_id":"1.3.0"},"extensions":[]}]],"extensions":[],"signatures":["200408e85aabcd7a46808f4d7e35436bbb7fbc15403052cc92ccae374ff43ca53779c59ffac4fafa36232381a15552830fbdad2ecf61b433ace79243234eeff4bd"]}}
2021-03-09T17:21:10.0997024Z 1270097ms p2p node.cpp:780 advertise_inventory_ ] beginning an iteration of advertise inventory
2021-03-09T17:21:10.1005865Z 1270097ms p2p node.cpp:794 advertise_inventory_ ] peer->peer_needs_sync_items_from_us: false
2021-03-09T17:21:10.1009207Z 1270097ms p2p node.cpp:802 advertise_inventory_ ] inventory_to_advertise: [{"item_type":1000,"item_hash":"f790d80149664991aabe2eaf2a601c139e8026d7"}]
2021-03-09T17:21:10.1012511Z Broadcasting tx
2021-03-09T17:21:10.1013603Z 1270097ms p2p node.cpp:816 advertise_inventory_ ] advertising item f790d80149664991aabe2eaf2a601c139e8026d7 to peer 127.0.0.1:36159
2021-03-09T17:21:10.1018860Z 1270097ms p2p node.cpp:829 advertise_inventory_ ] advertising 1 new item(s) of 1 type(s) to peer 127.0.0.1:36159

@abitmore
Copy link
Member Author

abitmore commented Mar 9, 2021

UPDATE: created issue #2378 for the transaction propagation issue occurred in the last comment, it will be fixed by #2377, but it's not caused by the reason described below. Created issue #2376 for the assumption described below.


The transaction propagation issue is due to the use of std::unordered_set which is not thread-safe.

        std::unordered_set<item_id> inventory_to_advertise;
        inventory_to_advertise.swap(_new_inventory);

Will create another pull request to address it.

BTW #490 is related.

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.

Intermittent connection failure in app_test
1 participant