Skip to content

chore(reserve): comments and rewording and final migration check#4795

Merged
istae merged 6 commits intomasterfrom
reserve-comments
Sep 2, 2024
Merged

chore(reserve): comments and rewording and final migration check#4795
istae merged 6 commits intomasterfrom
reserve-comments

Conversation

@istae
Copy link
Contributor

@istae istae commented Aug 29, 2024

Checklist

  • I have read the coding guide.
  • My change requires a documentation update, and I have done it.
  • I have added tests to cover my changes.
  • I have filled out the description and linked the related issues.

Description

Open API Spec Version Changes (if applicable)

Motivation and Context (Optional)

Related Issue (Optional)

Screenshots (if appropriate):

@istae istae changed the title chore: comments and rewording chore(reserve): comments and rewording Aug 29, 2024
@istae istae force-pushed the reserve-comments branch from 3a9503f to 1742597 Compare August 31, 2024 14:51
@istae istae changed the title chore(reserve): comments and rewording chore(reserve): comments and rewording and final migration check Aug 31, 2024
@istae istae marked this pull request as ready for review August 31, 2024 14:54
@istae istae requested review from acha-bill and martinconic August 31, 2024 14:54
Comment on lines +164 to +176
batchRadiusCnt, err := st.IndexStore().Count(&reserve.BatchRadiusItem{})
if err != nil {
return 0, 0, err
}

chunkBinCnt, err := st.IndexStore().Count(&reserve.ChunkBinItem{})
if err != nil {
return 0, 0, err
}

if batchRadiusCnt != chunkBinCnt {
return 0, 0, fmt.Errorf("index counts do not match, %d vs %d", batchRadiusCnt, chunkBinCnt)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be before the migration starts.
Then at the end, we compare that seenCnt == doneCnt == batchRadiusCnt == chunkBinCnt

@istae istae requested a review from acha-bill September 2, 2024 10:28
@ldeffenb
Copy link
Collaborator

ldeffenb commented Sep 2, 2024

So, if I've already migrated my sepolia testnet nodes without making a backup of the pre-2.2.0 database, should I db nuke before loading the next RC?

@istae
Copy link
Contributor Author

istae commented Sep 2, 2024

So, if I've already migrated my sepolia testnet nodes without making a backup of the pre-2.2.0 database, should I db nuke before loading the next RC?

We don't think that the issue is with the migration logic. So if your node completed the migration fine, then at this point, you do not need to do anything.

@ldeffenb
Copy link
Collaborator

ldeffenb commented Sep 2, 2024

We don't think that the issue is with the migration logic. So if your node completed the migration fine, then at this point, you do not need to do anything.

Thank you. When the team is ready, possibly after the next RC comes out correcting whatever is going on, I can load a new version of the OSM map into the sepolia testnet which should definitely stress the new chunk timestamp re-use/update logic.

@istae istae merged commit ce1a2a7 into master Sep 2, 2024
@istae istae deleted the reserve-comments branch September 2, 2024 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants