refactor: use shard uid in flat state keys #8685
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pugachAG
reviewed
Mar 7, 2023
| /// | ||
| /// WARNING: flat storage keeps changing, so the results might be inconsistent, unless you're running | ||
| /// this method on the shapshot of the data. | ||
| // TODO(#8676): Support non-trivial ranges. | ||
| pub fn iter_flat_state_entries<'a>( |
There was a problem hiding this comment.
it looks like we also need to chage this method to accept shard_uid instead of shard_id and do a proper prefix-read? or do you please to do that as a separate PR?
There was a problem hiding this comment.
Yeah, I would do it separately. Maybe even outside MVP, because we don't use this logic much.
|
also update |
|
@pugachAG: please ping me when you are satisfied with this PR and it is ready to be merged. |
pugachAG
approved these changes
Mar 8, 2023
akhi3030
approved these changes
Mar 8, 2023
nikurt
pushed a commit
to nikurt/nearcore
that referenced
this pull request
Mar 15, 2023
Prepend `ShardUId` to flat state keys, as motivated in near#8670. Ideally we need to replace `shard_id` with `shard_uid` everywhere, including other DB keys and `RuntimeAdapter` flat storage methods. But this change would be too involved, so let's do it incrementally. The biggest structural consequence is that `FlatStorage` now stores `shard_uid`, which makes sense, and later it will be used for writing deltas as well. It's not a big deal, but because of that changes are scattered throughout the code and may look scary. ## Testing * `test_flat_storage_iter` checks that new `key_belongs_to_shard` impl is aligned with shard layout; also we can see that the change takes place in this failing test: https://buildkite.com/nearprotocol/nearcore/builds/25676#0186bc8f-12a2-410a-b74c-e326f3b963fa * https://nayduck.near.org/#/run/2898
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prepend
ShardUIdto flat state keys, as motivated in #8670.Ideally we need to replace
shard_idwithshard_uideverywhere, including other DB keys andRuntimeAdapterflat storage methods. But this change would be too involved, so let's do it incrementally.The biggest structural consequence is that
FlatStoragenow storesshard_uid, which makes sense, and later it will be used for writing deltas as well. It's not a big deal, but because of that changes are scattered throughout the code and may look scary.Testing
test_flat_storage_iterchecks that newkey_belongs_to_shardimpl is aligned with shard layout; also we can see that the change takes place in this failing test: https://buildkite.com/nearprotocol/nearcore/builds/25676#0186bc8f-12a2-410a-b74c-e326f3b963fa