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

chore: update atlas config #3612

Merged
merged 3 commits into from
Apr 14, 2023
Merged

chore: update atlas config #3612

merged 3 commits into from
Apr 14, 2023

Conversation

lgalabru
Copy link
Contributor

In this PR, we're bumping max_uninstantiated_attachments to 50k and uninstantiated_attachments_expire_after to 1 day.

Copy link
Member

@jcnelson jcnelson left a comment

Choose a reason for hiding this comment

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

LGTM. @CharlieC3 @deantchi when deploying, can you keep some nodes with the old setting, just in case this puts too much load on the p2p threads of the new nodes?

@CharlieC3
Copy link
Member

@jcnelson Are these settings configurable in the config.toml? It would be a big help if they are.
cc @lgalabru

@codecov
Copy link

codecov bot commented Mar 14, 2023

Codecov Report

Merging #3612 (70e0483) into develop (04a504b) will increase coverage by 1.68%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #3612      +/-   ##
===========================================
+ Coverage    83.80%   85.49%   +1.68%     
===========================================
  Files          297      298       +1     
  Lines       276103   276674     +571     
===========================================
+ Hits        231397   236550    +5153     
+ Misses       44706    40124    -4582     
Impacted Files Coverage Δ
src/net/atlas/mod.rs 74.11% <ø> (ø)

... and 55 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@lgalabru
Copy link
Contributor Author

I agree that ideally, this settings would be exposed in the config.
Could someone working full-time on the blockchain team could follow up?
It sounds like a good first issue (cc @igorsyl / @kantai / @jcnelson for dispatch).

@pavitthrap
Copy link
Contributor

From blockchain meeting: test this live on mainnet for a week

@kantai
Copy link
Member

kantai commented Mar 22, 2023

Now that we have a configurable version of this PR (#3618), should we close this one?

@jbencin
Copy link
Contributor

jbencin commented Mar 22, 2023

Now that we have a configurable version of this PR (#3618), should we close this one?

PR #3618 is still using the old default values. If the testing by @CharlieC3 and @zone117x show these values to be an improvement, we can merge this PR to update the defaults

@pavitthrap
Copy link
Contributor

Blockchain meeting: testing halfway complete, need to see how these values affect attachment performance

@jbencin jbencin merged commit d8baa1c into develop Apr 14, 2023
@CharlieC3 CharlieC3 deleted the chore/update-atlas-config branch April 17, 2023 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

6 participants