Skip to content

feat(indexes): remove sync-v1 indexes#1200

Merged
jansegre merged 1 commit intomasterfrom
feat/remove-sync-v1-indexes
Dec 11, 2025
Merged

feat(indexes): remove sync-v1 indexes#1200
jansegre merged 1 commit intomasterfrom
feat/remove-sync-v1-indexes

Conversation

@jansegre
Copy link
Member

@jansegre jansegre commented Jan 8, 2025

Motivation

After removing sync-v1 (#1197) and removing block ties (#673) there is no need to have the sync-v1 indexes (tx-tips, block-tips and all-tips). There should be a slight runtime benefit (no measurement has been made), but more noticeably, there will be a substantial improvement in startup time.

Acceptance Criteria

  • Remove tx_tips, block_tips and all_tips indexes
  • Refactor relevant methods
  • Refactor tests

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

@jansegre jansegre requested a review from msbrogli as a code owner January 8, 2025 16:38
@jansegre jansegre self-assigned this Jan 8, 2025
@github-actions
Copy link

github-actions bot commented Jan 8, 2025

🐰 Bencher Report

Branchfeat/remove-sync-v1-indexes
Testbedubuntu-22.04
Click to view all benchmark results
BenchmarkLatencyBenchmark Result
minutes (m)
(Result Δ%)
Lower Boundary
minutes (m)
(Limit %)
Upper Boundary
minutes (m)
(Limit %)
sync-v2 (up to 20000 blocks)📈 view plot
🚷 view threshold
1.68 m
(-3.01%)Baseline: 1.73 m
1.56 m
(92.79%)
2.08 m
(80.82%)
🐰 View full continuous benchmarking report in Bencher

@jansegre jansegre changed the base branch from master to feat/consensus-change January 24, 2025 16:15
@jansegre jansegre force-pushed the feat/remove-sync-v1-indexes branch from b6bdf57 to 0e6013a Compare January 24, 2025 16:19
@codecov
Copy link

codecov bot commented Jan 24, 2025

Codecov Report

❌ Patch coverage is 97.36842% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.23%. Comparing base (5e3d848) to head (0f8cb8d).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
hathor/transaction/resources/mempool.py 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1200      +/-   ##
==========================================
+ Coverage   86.19%   86.23%   +0.04%     
==========================================
  Files         440      437       -3     
  Lines       34056    33725     -331     
  Branches     5327     5265      -62     
==========================================
- Hits        29353    29082     -271     
+ Misses       3677     3628      -49     
+ Partials     1026     1015      -11     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jansegre jansegre force-pushed the feat/remove-sync-v1-indexes branch from 0e6013a to 752b56a Compare January 24, 2025 16:38
@jansegre jansegre force-pushed the feat/consensus-change branch from 0a38b06 to ea7d72c Compare January 24, 2025 18:45
@jansegre jansegre force-pushed the feat/remove-sync-v1-indexes branch from 752b56a to cbfd6f1 Compare January 24, 2025 18:46
@jansegre jansegre requested a review from glevco January 27, 2025 18:55
@jansegre jansegre force-pushed the feat/remove-sync-v1-indexes branch from cbfd6f1 to 105ee8d Compare February 26, 2025 15:16
@glevco
Copy link
Contributor

glevco commented Mar 20, 2025

This PR greatly reduces memory usage, see https://github.com/HathorNetwork/on-call-incidents/issues/196.

@jansegre jansegre force-pushed the feat/consensus-change branch 2 times, most recently from eb49f63 to f32a3f9 Compare April 1, 2025 10:05
@jansegre jansegre force-pushed the feat/remove-sync-v1-indexes branch from 105ee8d to 11d5dd1 Compare April 1, 2025 10:05
@jansegre jansegre force-pushed the feat/consensus-change branch from f32a3f9 to e11dfd3 Compare July 1, 2025 16:21
@jansegre jansegre force-pushed the feat/remove-sync-v1-indexes branch from 11d5dd1 to 796f9a2 Compare July 1, 2025 16:33
@jansegre jansegre force-pushed the feat/consensus-change branch from e11dfd3 to 4b3736f Compare August 7, 2025 17:39
@jansegre jansegre force-pushed the feat/consensus-change branch from 4b3736f to 5e88bb8 Compare August 21, 2025 22:31
@jansegre jansegre force-pushed the feat/remove-sync-v1-indexes branch from 796f9a2 to 9692db5 Compare August 22, 2025 17:29
@jansegre jansegre force-pushed the feat/consensus-change branch from 5e88bb8 to b65c060 Compare September 11, 2025 15:34
@jansegre jansegre force-pushed the feat/remove-sync-v1-indexes branch from 9692db5 to 86d519d Compare September 11, 2025 15:41
@jansegre jansegre force-pushed the feat/consensus-change branch from b65c060 to 299608f Compare September 24, 2025 18:07
@jansegre jansegre force-pushed the feat/remove-sync-v1-indexes branch from 86d519d to 221618e Compare September 24, 2025 19:16
@msbrogli msbrogli force-pushed the feat/consensus-change branch from 299608f to dd0b9fe Compare September 30, 2025 16:54
@msbrogli msbrogli force-pushed the feat/consensus-change branch 3 times, most recently from 9341e49 to dda0697 Compare December 9, 2025 12:24
@github-project-automation github-project-automation bot moved this from In Progress (Done) to In Review (WIP) in Hathor Network Dec 11, 2025
@msbrogli msbrogli force-pushed the feat/consensus-change branch from 248b99e to 08a48da Compare December 11, 2025 04:04
@jansegre jansegre force-pushed the feat/remove-sync-v1-indexes branch from 4e7c26c to 941485d Compare December 11, 2025 12:05
msbrogli
msbrogli previously approved these changes Dec 11, 2025
@msbrogli msbrogli force-pushed the feat/consensus-change branch 2 times, most recently from b06697d to 2942f66 Compare December 11, 2025 14:57
@jansegre jansegre force-pushed the feat/remove-sync-v1-indexes branch from 941485d to 52e64ef Compare December 11, 2025 16:42
@msbrogli msbrogli force-pushed the feat/consensus-change branch from 2942f66 to 5e3d848 Compare December 11, 2025 17:12
@jansegre jansegre force-pushed the feat/remove-sync-v1-indexes branch from 52e64ef to 342762e Compare December 11, 2025 18:21
@jansegre jansegre changed the base branch from feat/consensus-change to master December 11, 2025 18:28
@jansegre jansegre dismissed msbrogli’s stale review December 11, 2025 18:28

The base branch was changed.

@jansegre jansegre force-pushed the feat/remove-sync-v1-indexes branch from 342762e to a6245b8 Compare December 11, 2025 18:33
msbrogli
msbrogli previously approved these changes Dec 11, 2025
# We need to check r1 as well to make sure we don't count twice the transactions/blocks that are
# just changing from voided to executed or vice-versa
if r1 and r3:
if r1:
Copy link
Contributor

Choose a reason for hiding this comment

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

Before, this was r1 and r3 which was equivalent to r2 and r4 which is equivalent to self.sorted_all.add_tx(tx) and self.sorted_blocks.add_tx(tx) (in the case of blocks, and analogous for txs).

Then new code is just equivalent to the sorted_all parts and is missing the sorted_blocks parts, which would mean r1 and r2 now. Is this correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, true!

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

Comment on lines +158 to +159
tips1 = {tx.hash for tx in state1.protocol.node.tx_storage.iter_mempool()}
tips2 = {tx.hash for tx in state2.protocol.node.tx_storage.iter_mempool()}
Copy link
Contributor

Choose a reason for hiding this comment

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

iter_mempool_tips instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

@jansegre jansegre moved this from In Review (WIP) to In Review (Done) in Hathor Network Dec 11, 2025
@jansegre jansegre merged commit a579e7f into master Dec 11, 2025
5 of 7 checks passed
@jansegre jansegre deleted the feat/remove-sync-v1-indexes branch December 11, 2025 19:59
@github-project-automation github-project-automation bot moved this from In Review (Done) to Waiting to be deployed in Hathor Network Dec 11, 2025
@jansegre jansegre mentioned this pull request Dec 11, 2025
2 tasks
@jansegre jansegre moved this from Waiting to be deployed to Done in Hathor Network Dec 12, 2025
@jansegre jansegre mentioned this pull request Dec 12, 2025
2 tasks
@glevco glevco mentioned this pull request Jan 9, 2026
1 task
@jansegre jansegre mentioned this pull request Jan 14, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants