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

feat: congestion info calculations #11195

Merged
merged 2 commits into from
May 1, 2024
Merged

Conversation

jakmeier
Copy link
Contributor

@jakmeier jakmeier commented May 1, 2024

Define all congestion evaluations as defined in the current NEP-539 proposal.

This is still not actually using the values in the runtime but the contained tests should be sufficient to show that the values are calculated as defined in the NEP.

Define all congestion evaluations as defined in the current NEP-539 proposal.

This is still not actually using the values in the runtime
but the contained tests should be sufficient to show
that the values are calculated as defined in the NEP.
@jakmeier jakmeier requested a review from wacban May 1, 2024 08:41
@jakmeier jakmeier requested a review from a team as a code owner May 1, 2024 08:41
Copy link

codecov bot commented May 1, 2024

Codecov Report

Attention: Patch coverage is 97.24409% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 71.20%. Comparing base (2feec32) to head (f9bd36e).
Report is 1 commits behind head on master.

Files Patch % Lines
core/primitives/src/congestion_info.rs 97.24% 1 Missing and 6 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #11195      +/-   ##
==========================================
+ Coverage   71.13%   71.20%   +0.06%     
==========================================
  Files         783      783              
  Lines      155438   155691     +253     
  Branches   155438   155691     +253     
==========================================
+ Hits       110571   110852     +281     
+ Misses      40440    40402      -38     
- Partials     4427     4437      +10     
Flag Coverage Δ
backward-compatibility 0.24% <0.00%> (-0.01%) ⬇️
db-migration 0.24% <0.00%> (-0.01%) ⬇️
genesis-check 1.41% <0.00%> (-0.01%) ⬇️
integration-tests 37.04% <0.00%> (-0.08%) ⬇️
linux 69.29% <97.24%> (+0.08%) ⬆️
linux-nightly 70.63% <97.24%> (+0.04%) ⬆️
macos 53.14% <97.24%> (+1.71%) ⬆️
pytests 1.63% <0.00%> (-0.01%) ⬇️
sanity-checks 1.42% <0.00%> (-0.01%) ⬇️
unittests 65.82% <97.24%> (+0.08%) ⬆️
upgradability 0.29% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@wacban wacban left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines 80 to 83
/// How much gas in delayed receipts a shard can tolerate before it stops all
/// shards from accepting new transactions with the receiver set to the
/// congested shard.
const REJECT_TX_CONGESTION_THRESHOLD: f64 = 0.25;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: The comment says it's measured in gas but the type and value is the congestion level.

todo!()
pub fn outgoing_limit(&self, sender_shard: ShardId) -> Gas {
match self {
CongestionInfo::V1(inner) => inner.outgoing_limit(sender_shard),
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't account for the missing chunks and the full info required for that is only available in the chunk header. I'm totally fine leaving that as is for now but eventually we'll need to combine both pieces.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I think we discussed previously that we want to try and hide this detail from the runtime.

I though we can make necessary adjustments outside the runtime, when reading the congestion information from the header. Then we can set it to max congestion if we missed >3 chunks. That way, the code in this PR shouldn't need changes to provide the desired behaviour, I think.

}
}

pub fn congestion_level(&self) -> f64 {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Can you add a small comment and mention that congestion level is in the [0, 1] range?

Comment on lines 388 to 389
assert_eq!(MAX_GAS_FORWARDING, congestion_info.outgoing_limit(0));
assert_eq!(MAX_TX_GAS, congestion_info.process_tx_limit());
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: It would be nice to have the same naming used for the constants and the limits e.g.
MAX_OUTGOING_GAS - outgoing_gas_limit()
MAX_TX_GAS - tx_gas_limit()

// processing to other shards is not restricted by memory congestion
assert_eq!(MAX_TX_GAS, congestion_info.process_tx_limit());

// remove halve the congestion
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: "remove half" or just "halve"

@jakmeier jakmeier enabled auto-merge May 1, 2024 10:40
@jakmeier jakmeier added this pull request to the merge queue May 1, 2024
Merged via the queue into near:master with commit 5a4ae5b May 1, 2024
29 checks passed
@jakmeier jakmeier deleted the congestion-info-calc branch May 1, 2024 11:23
jakmeier added a commit to jakmeier/nearcore that referenced this pull request May 1, 2024
This PR connects the changes of previous PRs
(mainly near#11128, near#11173, and  near#11195)
to apply congestion control rules as defined in NEP-539.

Builds with a stable protocol version are unaffected by
this PR, since `own_congestion_info` will return None
and the `ReceiptSinkV1` unconditionally forwards
all receipts exactly as it is done today.

Most components combined in this PR are already tested
in unit tests. The new features added with this PR are
best tested with integration tests, which will follow in future PRs.
github-merge-queue bot pushed a commit that referenced this pull request May 6, 2024
This PR connects the changes of previous PRs
(mainly #11128, #11173, and  #11195)
to apply congestion control rules as defined in
[NEP-539](near/NEPs#539).

Builds with a stable protocol version are unaffected by
this PR, since `own_congestion_info` will return None
and the `ReceiptSinkV1` unconditionally forwards 
all receipts exactly as it is done today.

Most components combined in this PR are already tested
in unit tests. The new features added with this PR are
best tested with integration tests, which will follow in future PRs.

The PR has two commits, it makes sense to review each on its own.
jancionear pushed a commit to jancionear/nearcore that referenced this pull request May 13, 2024
<p>This PR was automatically created by Snyk using the credentials of a
real user.</p><br /><h3>Snyk has created this PR to upgrade react-router
from 6.21.1 to 6.21.3.</h3>

:information_source: Keep your dependencies up-to-date. This makes it
easier to fix existing vulnerabilities and to more quickly identify and
fix newly disclosed vulnerabilities when they affect your project.
<hr/>

- The recommended version is **4 versions** ahead of your current
version.
- The recommended version was released **a month ago**, on 2024-01-18.


<details>
<summary><b>Release notes</b></summary>
<br/>
  <details>
    <summary>Package name: <b>react-router</b></summary>
    <ul>
      <li>
<b>6.21.3</b> - <a
href="https://snyk.io/redirect/github/remix-run/react-router/releases/tag/react-router-native%406.21.3">2024-01-18</a></br><p>[email protected]</p>
      </li>
      <li>
<b>6.21.3-pre.0</b> - <a
href="https://snyk.io/redirect/github/remix-run/react-router/releases/tag/react-router-native%406.21.3-pre.0">2024-01-16</a></br><p>[email protected]</p>
      </li>
      <li>
        <b>6.21.2</b> - 2024-01-11
      </li>
      <li>
        <b>6.21.2-pre.0</b> - 2024-01-09
      </li>
      <li>
        <b>6.21.1</b> - 2023-12-21
      </li>
    </ul>
from <a
href="https://snyk.io/redirect/github/remix-run/react-router/releases">react-router
GitHub release notes</a>
  </details>
</details>


<details>
  <summary><b>Commit messages</b></summary>
  </br>
  <details>
    <summary>Package name: <b>react-router</b></summary>
    <ul>
<li><a
href="https://snyk.io/redirect/github/remix-run/react-router/commit/44b391ad27dacc07bb1298bd79315c00845d5a2c">44b391a</a>
chore: Update version for release (near#11203)</li>
<li><a
href="https://snyk.io/redirect/github/remix-run/react-router/commit/82c48435350d239b376e435e8693ea51296c5848">82c4843</a>
Exit prerelease mode</li>
<li><a
href="https://snyk.io/redirect/github/remix-run/react-router/commit/1551285c2e821994d01b0a86cd2839c63898dabc">1551285</a>
Change release date</li>
<li><a
href="https://snyk.io/redirect/github/remix-run/react-router/commit/8fc701a910788d69c358565c974665384ca462fb">8fc701a</a>
Prep release notes</li>
<li><a
href="https://snyk.io/redirect/github/remix-run/react-router/commit/b4bf9271bc3e2a5a1fec73485392563cf3c8a7e2">b4bf927</a>
chore: Update version for release (pre) (near#11196)</li>
<li><a
href="https://snyk.io/redirect/github/remix-run/react-router/commit/84e5fb9f6df09e8324e43541d2ab660c383a025e">84e5fb9</a>
Enter prerelease mode</li>
<li><a
href="https://snyk.io/redirect/github/remix-run/react-router/commit/858f6f125d4dad94483f87fdfb8cc23cbbe68d59">858f6f1</a>
Merge branch &#x27;main&#x27; into release-next</li>
<li><a
href="https://snyk.io/redirect/github/remix-run/react-router/commit/b541b8dd5a582b1e95a2fd0111eeac2ab92f738b">b541b8d</a>
Fix NavLink isPending with a basename (near#11195)</li>
<li><a
href="https://snyk.io/redirect/github/remix-run/react-router/commit/552662ab867fd09e865adb33bb6631207a16ca53">552662a</a>
Remove leftover &#x60;unstable_&#x60; prefix from
&#x60;Blocker&#x60;/&#x60;BlockerFunction&#x60; types (near#11187)</li>
<li><a
href="https://snyk.io/redirect/github/remix-run/react-router/commit/04d653bcdb5b641485e4b9081fe196cfeac12716">04d653b</a>
Copy remix script to remove prereleases from changelogs (near#11183)</li>
<li><a
href="https://snyk.io/redirect/github/remix-run/react-router/commit/6837a1111a8ae1c60ab665b9ff2d9eb20b2f2f40">6837a11</a>
Fix date in changelog</li>
<li><a
href="https://snyk.io/redirect/github/remix-run/react-router/commit/c418410cb8b7277655d51e8ad2756059ad19fbeb">c418410</a>
Merge branch &#x27;release-next&#x27; into dev</li>
<li><a
href="https://snyk.io/redirect/github/remix-run/react-router/commit/274027eba64e614442106678c5a0676834202491">274027e</a>
Fix date in changelog</li>
<li><a
href="https://snyk.io/redirect/github/remix-run/react-router/commit/6be8e6c57d347c2dd975fa76b5aef6d39fd71d0e">6be8e6c</a>
Merge branch &#x27;release-next&#x27;</li>
<li><a
href="https://snyk.io/redirect/github/remix-run/react-router/commit/4e528c0bcee338c8f26d948affb3092257f6ee85">4e528c0</a>
chore: Update version for release (near#11182)</li>
<li><a
href="https://snyk.io/redirect/github/remix-run/react-router/commit/c8742d464d539bfdad452d59b7d5ce55b3628980">c8742d4</a>
Exit prerelease mode</li>
<li><a
href="https://snyk.io/redirect/github/remix-run/react-router/commit/f9780210d84257c21f628c31de165e91f3474263">f978021</a>
Prep release notes</li>
<li><a
href="https://snyk.io/redirect/github/remix-run/react-router/commit/cfce7e61655b46f7894fa8cadd336a66d58d1a00">cfce7e6</a>
Unbold date</li>
<li><a
href="https://snyk.io/redirect/github/remix-run/react-router/commit/038ad94452d81d882f1042395b28b0d80c3b4f40">038ad94</a>
Add dates to prior releases in changelog</li>
<li><a
href="https://snyk.io/redirect/github/remix-run/react-router/commit/c2c0826c6e4a9e213b4f43bc4e95d4f85bf2d96e">c2c0826</a>
chore: Update version for release (pre) (near#11178)</li>
<li><a
href="https://snyk.io/redirect/github/remix-run/react-router/commit/5cd3a45d625d947c795bd85aab8026a5e6b29614">5cd3a45</a>
Enter prerelease mode</li>
<li><a
href="https://snyk.io/redirect/github/remix-run/react-router/commit/03ce0377f5bed5fca3eb94d875c33eab9414b116">03ce037</a>
Merge branch &#x27;main&#x27; into release-next</li>
<li><a
href="https://snyk.io/redirect/github/remix-run/react-router/commit/0ef18963d274ea78592479cab9a48be2c5ec3c6a">0ef1896</a>
Do not attempt to deserialize empty json responses (near#11164)</li>
<li><a
href="https://snyk.io/redirect/github/remix-run/react-router/commit/e0d106bbe765ef12d3ad31e223127e2d08cab8e1">e0d106b</a>
Leverage useId for internal fetcher keys when available (near#11166)</li>
    </ul>

<a
href="https://snyk.io/redirect/github/remix-run/react-router/compare/08cda17f450ebe19481be3fc080d243ec5ef509f...44b391ad27dacc07bb1298bd79315c00845d5a2c">Compare</a>
  </details>
</details>
<hr/>

**Note:** *You are seeing this because you or someone else with access
to this repository has authorized Snyk to open upgrade PRs.*

For more information: <img
src="https://api.segment.io/v1/pixel/track?data=eyJ3cml0ZUtleSI6InJyWmxZcEdHY2RyTHZsb0lYd0dUcVg4WkFRTnNCOUEwIiwiYW5vbnltb3VzSWQiOiI1YjE0ODcwNy0yNzZhLTQ2M2EtOGU0Mi01YTQ2ZGRlOWViNWEiLCJldmVudCI6IlBSIHZpZXdlZCIsInByb3BlcnRpZXMiOnsicHJJZCI6IjViMTQ4NzA3LTI3NmEtNDYzYS04ZTQyLTVhNDZkZGU5ZWI1YSJ9fQ=="
width="0" height="0"/>

🧐 [View latest project
report](https://app.snyk.io/org/ecp88/project/98480bdc-d80b-4fd1-89d7-c4c56a706763?utm_source&#x3D;github&amp;utm_medium&#x3D;referral&amp;page&#x3D;upgrade-pr)

🛠 [Adjust upgrade PR
settings](https://app.snyk.io/org/ecp88/project/98480bdc-d80b-4fd1-89d7-c4c56a706763/settings/integration?utm_source&#x3D;github&amp;utm_medium&#x3D;referral&amp;page&#x3D;upgrade-pr)

🔕 [Ignore this dependency or unsubscribe from future upgrade
PRs](https://app.snyk.io/org/ecp88/project/98480bdc-d80b-4fd1-89d7-c4c56a706763/settings/integration?pkg&#x3D;react-router&amp;utm_source&#x3D;github&amp;utm_medium&#x3D;referral&amp;page&#x3D;upgrade-pr#auto-dep-upgrades)

<!---
(snyk:metadata:{"prId":"5b148707-276a-463a-8e42-5a46dde9eb5a","prPublicId":"5b148707-276a-463a-8e42-5a46dde9eb5a","dependencies":[{"name":"react-router","from":"6.21.1","to":"6.21.3"}],"packageManager":"npm","type":"auto","projectUrl":"https://app.snyk.io/org/ecp88/project/98480bdc-d80b-4fd1-89d7-c4c56a706763?utm_source=github&utm_medium=referral&page=upgrade-pr","projectPublicId":"98480bdc-d80b-4fd1-89d7-c4c56a706763","env":"prod","prType":"upgrade","vulns":[],"issuesToFix":[],"upgrade":[],"upgradeInfo":{"versionsDiff":4,"publishedDate":"2024-01-18T19:49:25.374Z"},"templateVariants":[],"hasFixes":false,"isMajorUpgrade":false,"isBreakingChange":false,"priorityScoreList":[]})
--->

Co-authored-by: snyk-bot <[email protected]>
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.

2 participants