Skip to content

refactor(metadata): migrate feature states to static metadata [part 5/12]#1018

Open
glevco wants to merge 1 commit intomasterfrom
refactor/metadata/migrate-feature-states-5
Open

refactor(metadata): migrate feature states to static metadata [part 5/12]#1018
glevco wants to merge 1 commit intomasterfrom
refactor/metadata/migrate-feature-states-5

Conversation

@glevco
Copy link
Contributor

@glevco glevco commented May 3, 2024

Depends on #1017


Motivation

Continuing with the static metadata refactor introduced in PRs #1013 and #1014, the next step is to move the feature_states attribute from metadata to static metadata. This PR implements this, finishing the static metadata refactor.

This results in very beneficial changes for Feature Activation, because now feature states are only calculated once when a block is received and then persisted in static metadata, meaning we don't have to deal with complications introduced by mutable states.

For instance, in order to get the state of a feature for a block before this PR, we would need to call feature_service.get_state() and it would either get it from the cache (in metadata) or recalculate it. Now, since an attribute is permanently available in static_metadata, retrieving feature states becomes as simple as accessing a property, which is guaranteed to always exist, incurring no performance penalties and preventing the need to inject the FeatureService singleton everywhere. In fact, FeatureService is no longer a singleton, and instead just a pure "calculator" (we may change its name in a separate PR).

This allowed us to simplify and clean up a lot of related code, and also naturally removed the FeatureService dependency from block verification — which was the final goal of the PRs in this sequence up to this point, for the Multiprocess Verification project.

Another great earned benefit is, now that feature_states are always guaranteed to be calculated for all blocks, we can change the algorithm to use parent blocks to retrieve the previous state, instead of previous boundary blocks. This also allowed us to remove the get_ancestor_at_height() method.

Acceptance Criteria

  • Move the feature_states attribute from metadata to static metadata.
  • Move some FeatureService convenience methods to static metadata.
  • Remove FeatureService singleton as it's no longer needed and update usages.
  • Update FeatureService so it no longer relies on stateful metadata, and instead becomes a pure function
  • Optimize calculate_feature_state() by using parent blocks instead of previous boundary blocks.
  • Remove feature activation related methods from Block.
  • Update BitSignalingService to automatically enable support during MUST_SIGNAL via PubSub, instead of via FeatureService.
  • Implement a migration to convert metadata into static metadata.
  • Update tests accordingly.
  • ATTENTION: This refactor means we now store feature states for all blocks, and not just boundary blocks. I checked it and after this migration, the database went from 8.2 to 8.3 GB. Still, there are known ways we could optimize metadata storage in future PRs.

Checklist

  • If you are requesting a merge into master, confirm this code is production-ready and can be included in future releases as soon as it gets merged

@glevco glevco added the refactor label May 3, 2024
@glevco glevco self-assigned this May 3, 2024
@glevco glevco force-pushed the refactor/metadata/migrate-feature-states-5 branch from 9a24389 to 2765de7 Compare May 3, 2024 20:21
@glevco glevco force-pushed the refactor/metadata/clean-up-refactors-4 branch 2 times, most recently from d104751 to d009d7b Compare May 3, 2024 21:06
@glevco glevco force-pushed the refactor/metadata/migrate-feature-states-5 branch from 2765de7 to 099613d Compare May 3, 2024 21:16
@glevco glevco changed the title wip refactor(metadata): migrate feature states to static metadata May 3, 2024
@glevco glevco changed the title refactor(metadata): migrate feature states to static metadata refactor(metadata): migrate feature states to static metadata [part 5] May 3, 2024
@glevco glevco force-pushed the refactor/metadata/clean-up-refactors-4 branch from d009d7b to 4004509 Compare May 3, 2024 21:40
@glevco glevco force-pushed the refactor/metadata/migrate-feature-states-5 branch 4 times, most recently from e27a1dc to ee046ce Compare May 3, 2024 22:50
@glevco glevco marked this pull request as ready for review May 3, 2024 22:52
@glevco glevco requested review from jansegre and msbrogli as code owners May 3, 2024 22:52
@glevco glevco force-pushed the refactor/metadata/migrate-feature-states-5 branch 2 times, most recently from 70c231e to 34a9630 Compare May 6, 2024 00:50
@codecov
Copy link

codecov bot commented May 6, 2024

Codecov Report

Attention: Patch coverage is 85.71429% with 16 lines in your changes missing coverage. Please review.

Project coverage is 84.44%. Comparing base (335d050) to head (db94b36).

Files with missing lines Patch % Lines
...ction/storage/migrations/migrate_feature_states.py 50.00% 14 Missing ⚠️
...on/storage/migrations/remove_first_nop_features.py 0.00% 1 Missing ⚠️
...n/storage/migrations/remove_second_nop_features.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1018      +/-   ##
==========================================
+ Coverage   84.42%   84.44%   +0.01%     
==========================================
  Files         317      318       +1     
  Lines       24303    24237      -66     
  Branches     3698     3687      -11     
==========================================
- Hits        20519    20467      -52     
+ Misses       3070     3050      -20     
- Partials      714      720       +6     

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

@glevco glevco force-pushed the refactor/metadata/migrate-feature-states-5 branch 2 times, most recently from 67120ee to 1303641 Compare May 8, 2024 03:02
@glevco glevco force-pushed the refactor/metadata/clean-up-refactors-4 branch from 4004509 to 410212e Compare May 10, 2024 02:09
@glevco glevco force-pushed the refactor/metadata/migrate-feature-states-5 branch from 1303641 to 5fa770f Compare May 10, 2024 02:11
@glevco glevco force-pushed the refactor/metadata/clean-up-refactors-4 branch from 410212e to 1de39c3 Compare May 10, 2024 02:48
@glevco glevco force-pushed the refactor/metadata/migrate-feature-states-5 branch from 5fa770f to 052ae17 Compare May 10, 2024 02:50
@glevco glevco force-pushed the refactor/metadata/migrate-feature-states-5 branch from 41536ed to 1bb6e2d Compare August 16, 2024 15:24
@glevco glevco force-pushed the refactor/metadata/clean-up-refactors-4 branch from bd8f15b to c1f6ea6 Compare August 16, 2024 18:02
@glevco glevco force-pushed the refactor/metadata/migrate-feature-states-5 branch 2 times, most recently from ad55a26 to f3bf80d Compare August 16, 2024 20:21
@glevco glevco force-pushed the refactor/metadata/clean-up-refactors-4 branch from c1f6ea6 to 337c665 Compare August 19, 2024 16:29
@glevco glevco force-pushed the refactor/metadata/migrate-feature-states-5 branch 3 times, most recently from b2437ce to ca910a0 Compare August 19, 2024 19:05
@glevco glevco force-pushed the refactor/metadata/clean-up-refactors-4 branch from 337c665 to 30ad6a7 Compare August 19, 2024 19:09
@glevco glevco force-pushed the refactor/metadata/migrate-feature-states-5 branch from ca910a0 to a904b3f Compare August 19, 2024 19:10
@glevco glevco force-pushed the refactor/metadata/clean-up-refactors-4 branch from 30ad6a7 to a1b267e Compare August 19, 2024 19:46
@glevco glevco force-pushed the refactor/metadata/migrate-feature-states-5 branch from a904b3f to 3659269 Compare August 19, 2024 19:47
@glevco glevco force-pushed the refactor/metadata/clean-up-refactors-4 branch from a1b267e to 255b9e7 Compare August 22, 2024 17:39
@glevco glevco force-pushed the refactor/metadata/migrate-feature-states-5 branch from 3659269 to e05cca1 Compare August 22, 2024 17:40
@glevco glevco force-pushed the refactor/metadata/clean-up-refactors-4 branch from 255b9e7 to 6f86959 Compare August 23, 2024 21:39
@glevco glevco force-pushed the refactor/metadata/migrate-feature-states-5 branch from e05cca1 to 35e636b Compare August 23, 2024 21:39
@glevco glevco force-pushed the refactor/metadata/clean-up-refactors-4 branch from 6f86959 to 927d482 Compare August 27, 2024 14:25
Base automatically changed from refactor/metadata/clean-up-refactors-4 to master August 27, 2024 15:14
@glevco glevco force-pushed the refactor/metadata/migrate-feature-states-5 branch from 35e636b to 25f5e2d Compare August 27, 2024 15:16
@github-actions
Copy link

github-actions bot commented Aug 27, 2024

🐰 Bencher Report

Branchrefactor/metadata/migrate-feature-states-5
Testbedubuntu-22.04
Click to view all benchmark results
BenchmarkLatencyBenchmark Result
nanoseconds (ns)
(Result Δ%)
Lower Boundary
nanoseconds (ns)
(Limit %)
Upper Boundary
nanoseconds (ns)
(Limit %)
sync-v2 (up to 20000 blocks)📈 view plot
🚷 view threshold
101,462,798,848.64
(-0.48%)
91,760,403,941.47
(90.44%)
112,151,604,817.35
(90.47%)
🐰 View full continuous benchmarking report in Bencher

@glevco glevco force-pushed the refactor/metadata/migrate-feature-states-5 branch from 25f5e2d to cd6a4ac Compare September 5, 2024 15:02
@glevco glevco force-pushed the refactor/metadata/migrate-feature-states-5 branch from cd6a4ac to 7ceced8 Compare September 16, 2024 15:08
@glevco glevco force-pushed the refactor/metadata/migrate-feature-states-5 branch from 7ceced8 to 5d1e870 Compare October 3, 2024 21:09
@glevco
Copy link
Contributor Author

glevco commented Oct 7, 2024

I'm putting this PR on hold for now, and instead PR #1146 will revert related changes, keeping feature states in "normal" metadata, and not moving it to static metadata. See the new PR's description for motivation.

@msbrogli msbrogli force-pushed the master branch 2 times, most recently from eb416fa to 21d7909 Compare February 12, 2026 23:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

2 participants