Skip to content

StatefulStorage: bump max page id from 16 to 32#1959

Merged
aramikm merged 3 commits intomainfrom
max_storage_page_id
May 2, 2024
Merged

StatefulStorage: bump max page id from 16 to 32#1959
aramikm merged 3 commits intomainfrom
max_storage_page_id

Conversation

@aramikm
Copy link
Copy Markdown
Collaborator

@aramikm aramikm commented May 1, 2024

Goal

The goal of this PR is to make sure we are not going to have any issue to be able to store at 7k private follows

Closes #1958

Discussions

  • This might have some effect on the weights that will be applied once we run DB benchmarks on our existing chain data.

Checklist

  • Chain spec updated

@codecov
Copy link
Copy Markdown

codecov bot commented May 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.13%. Comparing base (3097ede) to head (97db69d).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1959   +/-   ##
=======================================
  Coverage   83.13%   83.13%           
=======================================
  Files          56       56           
  Lines        4530     4530           
=======================================
  Hits         3766     3766           
  Misses        764      764           

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

@github-actions github-actions bot added the metadata-changed Metadata has changed since the latest full release label May 1, 2024
@JoeCap08055
Copy link
Copy Markdown
Collaborator

Just noting that we chose the page limit we did initially because we determined that number of follows is unrealistic. (Ignoring the fact that this is exactly one of MeWe's test cases, but even they recognize that it's not a realistic scenario.)

So, do we want to allow anyone to consume that much on-chain storage, or should we keep a smaller limit? In reality only a bot or poorly designed provider platform would generate that many follows. (Though that does raise the question: MeWe actually allows that many follows, even though it's unrealistic...)

I guess maybe we bump the limit for now to, but maybe we should consider other solutions in the future (like perhaps a "soft" limit that can be overridden by governance on a per-user basis).

Copy link
Copy Markdown
Collaborator

@wilwade wilwade left a comment

Choose a reason for hiding this comment

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

I am good with these changes.

Copy link
Copy Markdown
Collaborator

@shannonwells shannonwells left a comment

Choose a reason for hiding this comment

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

ship it

@github-actions github-actions bot added metadata-changed Metadata has changed since the latest full release and removed metadata-changed Metadata has changed since the latest full release labels May 2, 2024
@aramikm aramikm merged commit 6885dbc into main May 2, 2024
@aramikm aramikm deleted the max_storage_page_id branch May 2, 2024 21:32
aramikm added a commit to ProjectLibertyLabs/graph-sdk that referenced this pull request Jun 4, 2024
# Goal
The goal of this PR is address the max page id issue discovered via
running the simulation

Closes #198 

# WARNING
This should not get merged until
frequency-chain/frequency#1959 is merged and
Deployed on Frequency mainnet

# Discussion
- This is companion PR related to the change on frequency chain
frequency-chain/frequency#1959

# Checklist
- [x] Tests updated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

metadata-changed Metadata has changed since the latest full release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Increase MaxPaginatedPageId

5 participants