search

near / nearcore

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

feat(congestion): More info on shard congested RPC error #11716

Closed jakmeier closed 2 weeks ago

jakmeier commented 2 weeks ago

With the congestion level already included in the response, we clients don't have to query the congestion info afterwards if they want to use it to determine a fitting timeout for a retry.

We could also provide more detailed information, like which specific limit of congestion was hit but I don't think it's appropriate for the RPC API. I would say this is implementation detail and we want to be able to change it without worrying about what is exposed on the API.

However, since "missed chunk congestion" is fundamentally different, I think we should provide a separate error for it. I've added ShardStuck error for it, which includes the number of missed chunks in the error message rather than the congestion level.

jakmeier commented 2 weeks ago

This is my suggestion from what we discussed yesterday to add more info to the error. (cc @wacban and @bowenwang1996 )

Review hint: The first commit is a separate PR bumping borsh (#11715) for compatibility with ordered-float. I don't want to bump it as a side-effect of this PR, hence the separate PR.

codecov[bot] commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 78.33333% with 13 lines in your changes missing coverage. Please review.

Project coverage is 71.72%. Comparing base (a39a6ee) to head (fd6028f).

Files Patch % Lines
core/primitives/src/errors.rs 0.00% 7 Missing :warning:
chain/chain/src/runtime/mod.rs 66.66% 4 Missing :warning:
core/primitives/src/congestion_info.rs 95.12% 1 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #11716 +/- ## ========================================== - Coverage 71.74% 71.72% -0.02% ========================================== Files 791 791 Lines 161943 161984 +41 Branches 161943 161984 +41 ========================================== + Hits 116178 116180 +2 - Misses 40724 40760 +36 - Partials 5041 5044 +3 ``` | [Flag](https://app.codecov.io/gh/near/nearcore/pull/11716/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/11716/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/11716/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/11716/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/11716/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `38.00% <56.66%> (+0.03%)` | :arrow_up: | | [linux](https://app.codecov.io/gh/near/nearcore/pull/11716/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `71.53% <78.33%> (+<0.01%)` | :arrow_up: | | [linux-nightly](https://app.codecov.io/gh/near/nearcore/pull/11716/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `71.31% <78.33%> (-0.01%)` | :arrow_down: | | [macos](https://app.codecov.io/gh/near/nearcore/pull/11716/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `53.94% <68.33%> (+1.02%)` | :arrow_up: | | [pytests](https://app.codecov.io/gh/near/nearcore/pull/11716/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/11716/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/11716/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `66.12% <68.33%> (-0.01%)` | :arrow_down: | | [upgradability](https://app.codecov.io/gh/near/nearcore/pull/11716/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.