-
Notifications
You must be signed in to change notification settings - Fork 37
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
Conversation
✅ Deploy Preview for jellyfishsdk ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov Report
@@ 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
... 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! |
Docker build preview for jellyfish/apps is ready! Built with commit e89e5e6
You can also get an immutable image with the commit hash
|
@lykalabrada -- 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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice 🤓 !! (lesser brain consuming)
i think need a log here tho...
|
What this PR does / why we need it:
Fixing random bug
!bestChain
reindex
to jump off the cleanup cycle ifNotFoundIndexError
!bestChain
Which issue(s) does this PR fixes?:
Fixes #
Additional comments?: