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

Remove two sources of truth for default trie cache size #7066

Closed
matklad opened this issue Jun 20, 2022 · 3 comments
Closed

Remove two sources of truth for default trie cache size #7066

matklad opened this issue Jun 20, 2022 · 3 comments
Labels
C-housekeeping Category: Refactoring, cleanups, code quality T-core Team: issues relevant to the core team

Comments

@matklad
Copy link
Contributor

matklad commented Jun 20, 2022

However, the overall code does feel a bit "bolted on". I would like to fix the following things in the follow up:

  • make sure that default and overriden cache capacity is handled in the same way. It doesn't make sense that the former is hard-coded, but the latter is overridable

  • Cleanup defaults in StoreConfig, it feels messy that the source of truth are awkward functions which exists solely for serde

  • Replace TrieFactory with just TrieConfig -- we don't need doer object here, just a bag of values

  • revisit the format of the config,

    "trie_cache_capacities": [
      [
        {
          "version": 1,
          "shard_id": 2
        },
        2000000
      ]
    ]
    

    doesn't look like a good config format, something like { "1:2": "2000000" } would'd be much user friendlier.

Originally posted by @matklad in #7022 (comment)

@matklad matklad added C-housekeeping Category: Refactoring, cleanups, code quality T-core Team: issues relevant to the core team labels Jun 20, 2022
@matklad
Copy link
Contributor Author

matklad commented Jun 20, 2022

cc @Longarithm

@matklad
Copy link
Contributor Author

matklad commented Sep 7, 2022

cc @jakmeier I think this will be closed by one of your PRs

@jakmeier
Copy link
Contributor

jakmeier commented Jan 5, 2023

Solved by #7566 + #7578

@jakmeier jakmeier closed this as completed Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-housekeeping Category: Refactoring, cleanups, code quality T-core Team: issues relevant to the core team
Projects
None yet
Development

No branches or pull requests

2 participants