near / nearcore

Reference client for NEAR Protocol
https://near.org
GNU General Public License v3.0
2.31k stars 615 forks source link

fix(view client): remember tx status requests across view client threads #11796

Closed marcelo-gonzalez closed 2 months ago

marcelo-gonzalez commented 2 months ago

The ViewClientRequestManager is meant to remember what tx status requests we've sent to other nodes so we can serve future view client requests after receiving it. But before this change, each view client has its own cache. This means that tx status responses are only available to the same view client thread, which might not have been the one that made the request. So if we share it across threads, we fix this issue

Also in this change we wait a bit in single_shard_tracking.py before sending transactions because otherwise the first block after the localnet is started often has no new chunks and the transaction is rejected by congestion control because the computed current height - height included (which is the genesis height) is too high

marcelo-gonzalez commented 2 months ago

you can see that it's failing due to congestion control because of missing chunks by printing out the status here and running without the sleep added in this PR. What I saw was that it was rejected with missing_chunks: 3. And then looking at the blocks on disk, we see that the first block in the network had height 3 and no chunks, so the included genesis chunks have height 0 and we reject the tx.

Then after you apply that sleep you can see that the request manager stuff is the culprit by just printing out debug info where the response is received and the cached response is checked and you see that we get no cached response directly after saving it. Printing out the current thread id will show that they're different view client actors

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 71.74%. Comparing base (54bfccb) to head (9bb2610). Report is 3 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #11796 +/- ## ========================================== - Coverage 71.77% 71.74% -0.04% ========================================== Files 796 796 Lines 162935 162996 +61 Branches 162935 162996 +61 ========================================== - Hits 116949 116943 -6 - Misses 40933 40998 +65 - Partials 5053 5055 +2 ``` | [Flag](https://app.codecov.io/gh/near/nearcore/pull/11796/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | Coverage Δ | | |---|---|---| | [backward-compatibility](https://app.codecov.io/gh/near/nearcore/pull/11796/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `0.23% <0.00%> (-0.01%)` | :arrow_down: | | [db-migration](https://app.codecov.io/gh/near/nearcore/pull/11796/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `0.23% <0.00%> (-0.01%)` | :arrow_down: | | [genesis-check](https://app.codecov.io/gh/near/nearcore/pull/11796/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `1.35% <0.00%> (-0.01%)` | :arrow_down: | | [integration-tests](https://app.codecov.io/gh/near/nearcore/pull/11796/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `37.90% <100.00%> (+0.05%)` | :arrow_up: | | [linux](https://app.codecov.io/gh/near/nearcore/pull/11796/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `71.26% <100.00%> (-0.04%)` | :arrow_down: | | [linux-nightly](https://app.codecov.io/gh/near/nearcore/pull/11796/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `71.34% <100.00%> (-0.02%)` | :arrow_down: | | [macos](https://app.codecov.io/gh/near/nearcore/pull/11796/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `54.52% <100.00%> (-0.03%)` | :arrow_down: | | [pytests](https://app.codecov.io/gh/near/nearcore/pull/11796/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `1.58% <0.00%> (-0.01%)` | :arrow_down: | | [sanity-checks](https://app.codecov.io/gh/near/nearcore/pull/11796/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `1.38% <0.00%> (-0.01%)` | :arrow_down: | | [unittests](https://app.codecov.io/gh/near/nearcore/pull/11796/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `66.18% <100.00%> (-0.04%)` | :arrow_down: | | [upgradability](https://app.codecov.io/gh/near/nearcore/pull/11796/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `0.28% <0.00%> (-0.01%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.