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

Set default transaction pool size limit to 100MB. #9172

Merged

Conversation

aborg-dev
Copy link
Contributor

@aborg-dev aborg-dev commented Jun 12, 2023

This PR enables the limit discussed in #3284.

I've considered another approach to rolling this out by changing the value in config.json distributed through S3, but that would require more work both on our side and on validators without adding much benefit.
Specifically, I've checked that we have a good safety margin here, as over the last month on the testnet, the max size of the transaction pool on the validators was < 40 KB: https://nearinc.grafana.net/goto/AhZFN__4R?orgId=1.

@nikurt What would be a good place to document this field for validators? I saw https://near-nodes.io/, but couldn't find the appropriate section there.

@aborg-dev aborg-dev added the A-congestion Work aimed at ensuring good system performance under congestion label Jun 12, 2023
@aborg-dev aborg-dev requested a review from nikurt June 12, 2023 09:52
@aborg-dev aborg-dev requested a review from a team as a code owner June 12, 2023 09:52
@nikurt
Copy link
Contributor

nikurt commented Jun 12, 2023

@nikurt What would be a good place to document this field for validators? I saw https://near-nodes.io/, but couldn't find the appropriate section there.

@Akashin At a minimum it should be mentioned in the release notes. Please consider creating a Jira task. We don't have a comprehensive documentation for config.json. One of the best option is to provide human-readable comments in the code.
cc: @marcelo-gonzalez

@aborg-dev
Copy link
Contributor Author

@nikurt What would be a good place to document this field for validators? I saw https://near-nodes.io/, but couldn't find the appropriate section there.

@Akashin At a minimum it should be mentioned in the release notes. Please consider creating a Jira task. We don't have a comprehensive documentation for config.json. One of the best option is to provide human-readable comments in the code. cc: @marcelo-gonzalez

I've added longer documentation in the field docstring.

I wasn't able to find where exactly I need to create a task in Jira, is this a part of some release process?
We have a tracking bug on GitHub #3284 that is mirrored in Jira: https://pagodaplatform.atlassian.net/browse/DVPT-522 - does it make sense to reuse it? Should I reassign it to the next release engineer? (presumably @marcelo-gonzalez ?)

@aborg-dev aborg-dev force-pushed the transaction_pool_limit_changelog branch from 39e74fa to 950a42b Compare June 12, 2023 15:26
@near-bulldozer near-bulldozer bot merged commit 4525821 into near:master Jun 12, 2023
nikurt pushed a commit that referenced this pull request Jun 13, 2023
This PR enables the limit discussed in #3284.

I've [considered](https://near.zulipchat.com/#narrow/stream/297873-pagoda.2Fnode/topic/Adding.20a.20new.20field.20to.20config.2Ejson/near/358955785) another approach to rolling this out by changing the value in `config.json` distributed through S3, but that would require more work both on our side and on validators without adding much benefit.
Specifically, I've checked that we have a good safety margin here, as over the last month on the testnet, the max size of the transaction pool on the validators was < 40 KB: https://nearinc.grafana.net/goto/AhZFN__4R?orgId=1.

@nikurt What would be a good place to document this field for validators? I saw https://near-nodes.io/, but couldn't find the appropriate section there.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-congestion Work aimed at ensuring good system performance under congestion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants