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

feat: implement inlining for small values in flat storage #8988

Merged
merged 1 commit into from
May 3, 2023

Conversation

pugachAG
Copy link
Contributor

@pugachAG pugachAG commented Apr 30, 2023

Part of #8243.

This PR implements value inlining MVP for state sync:

  • add FlatStateValue::Inlined variant to store inlined values as part of FlatState and FlatStateChanges on disk.
  • change flat storage API to return FlatStateValue instead of ValueRef.

The following will be implemented separately:

  • Migration for existing FlatState values. This is required for state sync, but quite involved, so decided to keep it separately.
  • Inlining for cached flat state deltas: for now we keep those as ValueRef.
  • Using inlined values for transaction processing: for now we convert inlined values to ValueRef.

@pugachAG pugachAG force-pushed the inline-flat-storage-values branch 6 times, most recently from 2a10dc8 to c984bea Compare May 1, 2023 14:30
@pugachAG pugachAG requested a review from Longarithm May 1, 2023 14:31
@pugachAG pugachAG marked this pull request as ready for review May 1, 2023 14:31
@pugachAG pugachAG requested a review from a team as a code owner May 1, 2023 14:31
@pugachAG pugachAG force-pushed the inline-flat-storage-values branch 2 times, most recently from 4ae35ac to 0a137ca Compare May 1, 2023 16:22
core/store/src/flat/types.rs Show resolved Hide resolved
#[derive(BorshSerialize, BorshDeserialize, Debug, Clone, PartialEq, Eq)]
pub enum FlatStateValue {
Ref(ValueRef),
// TODO(8243): add variant here for the inlined value
Inlined(Vec<u8>),
Copy link
Member

Choose a reason for hiding this comment

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

I have a little doubt if we should introduce it as a term in codebase. But it makes sense because our default behaviour is "not-inlined".

@pugachAG pugachAG force-pushed the inline-flat-storage-values branch 4 times, most recently from 4111011 to 9629ed1 Compare May 3, 2023 11:40
@pugachAG pugachAG added A-storage Area: storage and databases S-automerge labels May 3, 2023
@pugachAG pugachAG force-pushed the inline-flat-storage-values branch from 9629ed1 to f3707c9 Compare May 3, 2023 11:44
@pugachAG pugachAG force-pushed the inline-flat-storage-values branch from f3707c9 to 103ae59 Compare May 3, 2023 12:25
@near-bulldozer near-bulldozer bot merged commit 22c1466 into near:master May 3, 2023
pugachAG added a commit to pugachAG/nearcore that referenced this pull request May 4, 2023
near-bulldozer bot pushed a commit that referenced this pull request May 4, 2023
…9005)

This reverts #8988 as it results in node crashing: [zulip](https://near.zulipchat.com/#narrow/stream/345766-pagoda.2Fstorage.2Fflat-storage/topic/Canary.20crashes/near/355722627).

The original PR changes data format for delta changes on disk, so binary with the new code failed to deserialise deltas written by the old code.

I suggest we revert the change for now and then re-publish it along with the db migration.
nikurt pushed a commit that referenced this pull request May 4, 2023
Part of #8243.

This PR implements value inlining MVP for state sync:
* add `FlatStateValue::Inlined` variant to store inlined values as part of `FlatState` and `FlatStateChanges` on disk.
* change flat storage API to return `FlatStateValue` instead of `ValueRef`.

The following will be implemented separately:
* Migration for existing `FlatState` values. This is required for state sync, but quite involved, so decided to keep it separately.
* Inlining for cached flat state deltas: for now we keep those as `ValueRef`.
* Using inlined values for transaction processing: for now we convert inlined values to `ValueRef`.
nikurt pushed a commit that referenced this pull request May 4, 2023
…9005)

This reverts #8988 as it results in node crashing: [zulip](https://near.zulipchat.com/#narrow/stream/345766-pagoda.2Fstorage.2Fflat-storage/topic/Canary.20crashes/near/355722627).

The original PR changes data format for delta changes on disk, so binary with the new code failed to deserialise deltas written by the old code.

I suggest we revert the change for now and then re-publish it along with the db migration.
near-bulldozer bot pushed a commit that referenced this pull request May 8, 2023
Part of #8243.

This PR consists of 2 parts:
* 08ecc51: un-revert #9005, see #8988 for the detailed description of the changes.
* 3d29347: db migration for flat state delta changes to avoid backward compatibility issues
nikurt pushed a commit that referenced this pull request May 10, 2023
Part of #8243.

This PR consists of 2 parts:
* 08ecc51: un-revert #9005, see #8988 for the detailed description of the changes.
* 3d29347: db migration for flat state delta changes to avoid backward compatibility issues
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-storage Area: storage and databases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants