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

fix(whale-api): add reindex for failed to indexed block #2169

Merged
merged 5 commits into from
Nov 10, 2023

Conversation

lykalabrada
Copy link
Contributor

@lykalabrada lykalabrada commented Nov 7, 2023

What this PR does / why we need it:

Fixing random bug !bestChain

  • introduce status reindex to jump off the cleanup cycle if NotFoundIndexError
  • logged on random bug!bestChain

Which issue(s) does this PR fixes?:

Fixes #

Additional comments?:

Copy link

netlify bot commented Nov 7, 2023

Deploy Preview for jellyfishsdk ready!

Name Link
🔨 Latest commit 3883065
🔍 Latest deploy log https://app.netlify.com/sites/jellyfishsdk/deploys/654dc942565c2b00094c8bda
😎 Deploy Preview https://deploy-preview-2169--jellyfishsdk.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented Nov 7, 2023

Codecov Report

Merging #2169 (3883065) into main (6f312b9) will increase coverage by 1.12%.
The diff coverage is 60.00%.

@@            Coverage Diff             @@
##             main    #2169      +/-   ##
==========================================
+ Coverage   90.86%   91.99%   +1.12%     
==========================================
  Files         366      371       +5     
  Lines       11069    11243     +174     
  Branches     1448     1474      +26     
==========================================
+ Hits        10058    10343     +285     
+ Misses        970      864     -106     
+ Partials       41       36       -5     
Files Coverage Δ
apps/whale-api/src/module.indexer/status.ts 100.00% <100.00%> (ø)
...whale-api/src/module.indexer/rpc.block.provider.ts 86.66% <50.00%> (-2.62%) ⬇️

... and 89 files with indirect coverage changes

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

Copy link

github-actions bot commented Nov 7, 2023

Docker build preview for jellyfish/apps is ready!

Built with commit e89e5e6

  • ghcr.io/birthdayresearch/legacy-api:pr-2169
  • ghcr.io/birthdayresearch/playground-api:pr-2169
  • ghcr.io/birthdayresearch/status-api:pr-2169
  • ghcr.io/birthdayresearch/whale-api:pr-2169

You can also get an immutable image with the commit hash

  • ghcr.io/birthdayresearch/legacy-api:e89e5e6e97c0f63e88a4acfad4573530c26503c6
  • ghcr.io/birthdayresearch/playground-api:e89e5e6e97c0f63e88a4acfad4573530c26503c6
  • ghcr.io/birthdayresearch/status-api:e89e5e6e97c0f63e88a4acfad4573530c26503c6
  • ghcr.io/birthdayresearch/whale-api:e89e5e6e97c0f63e88a4acfad4573530c26503c6

@canonbrother
Copy link
Contributor

canonbrother commented Nov 10, 2023

@lykalabrada
suggestion:

-- await this.statusMapper.put(hash, height, Status.ERROR)
++ if (err instanceof NotFoundIndexerError) {
++         await this.statusMapper.put(hash, height, Status.REINDEX)
++ } else {
++         await this.statusMapper.put(hash, height, Status.ERROR)
++ }
// cleanup
...
switch (status.status) {
           case Status.INVALIDATED:
           case Status.INDEXED:
++       case Status.REINDEX:        
        return
    }

it will solve the cleanup lock and reindex based on bestchain checker.. at the end.. we'd finally see the "issue" log

await this.invalidate(indexed.hash, indexed.height)
this.logger.log(`[Synchronize - not the best chain] Indexing prev block ${indexed.height}...`)
// Retry indexing the previous block before invalidating it
const MAX_RETRIES = 3
Copy link
Contributor

@canonbrother canonbrother Nov 10, 2023

Choose a reason for hiding this comment

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

we do not need this ady after REINDEX..
all reindex/pre-index will be auto handling in synchronize
one retry pattern is sufficient

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, then we will not be reindexing the previous block, just the current block thats failing. Also fine with me, we just have to test on any stucked ocean, to see if fix still works the same :)
cc: @pierregee

@@ -100,7 +133,7 @@ export class RPCBlockProvider {
* @param {defid.Block<Transaction>} nextBlock to check previous block hash
*/
private static async isBestChain (indexed: Block, nextBlock: defid.Block<defid.Transaction>): Promise<boolean> {
return nextBlock.previousblockhash === indexed.hash
return indexed.hash === nextBlock.previousblockhash
Copy link
Contributor

@canonbrother canonbrother Nov 10, 2023

Choose a reason for hiding this comment

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

nice 🤓 !! (lesser brain consuming)

@canonbrother
Copy link
Contributor

canonbrother commented Nov 10, 2023

i think need a log here tho...

} else {
await this.invalidate(indexed.hash, indexed.height)

eg:

this.logger.error(`indexed{n: ${indexed.height}, h: ${indexed.hash}}, nextBlock{n: ${nextBlock.height}, prevH: ${nextBlock.previousblockhash}}`)

@pierregee pierregee marked this pull request as ready for review November 10, 2023 07:20
@lykalabrada lykalabrada changed the title chore(whale-api): add a retry for missing previous block txns chore(whale-api): add reindex for failed to indexed block Nov 10, 2023
@canonbrother canonbrother changed the title chore(whale-api): add reindex for failed to indexed block Bug(whale-api): add reindex for failed to indexed block Nov 10, 2023
@github-actions github-actions bot removed the kind/chore Non feature change label Nov 10, 2023
@canonbrother canonbrother changed the title Bug(whale-api): add reindex for failed to indexed block bug(whale-api): add reindex for failed to indexed block Nov 10, 2023
@canonbrother canonbrother changed the title bug(whale-api): add reindex for failed to indexed block fix(whale-api): add reindex for failed to indexed block Nov 10, 2023
@github-actions github-actions bot added the kind/fix Fix a bug label Nov 10, 2023
@fuxingloh fuxingloh merged commit dd01b74 into main Nov 10, 2023
39 of 40 checks passed
@fuxingloh fuxingloh deleted the lyka/retry-prevblock-indexer branch November 10, 2023 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants