feat: lotus-shed tooling for chain indexer #12474
Conversation
|
Should I keep it that way or remove it because |
|
@akaladarshi Let's remove it because |
Ok sure. @aarshkshah1992 |
|
@akaladarshi Yeah we can add data validation as well in a follow-up PR. That would be great 👍 However, please note that that should be done in the API, not in the |
|
@akaladarshi But let's do that once we have this |
akaladarshi
force-pushed
the
akaladarshi/lotus-shed-chainindexer-tooling
branch
from
fa127ed
to
0dc4dba
Compare
cmd/lotus-shed/indexes.go
Outdated
| @@ -34,6 +35,15 @@ const ( | |||
| insertEntry = `INSERT OR IGNORE INTO event_entry(event_id, indexed, flags, key, codec, value) VALUES(?, ?, ?, ?, ?, ?)` | |||
| upsertEventsSeen = `INSERT INTO events_seen(height, tipset_key_cid, reverted) VALUES(?, ?, false) ON CONFLICT(height, tipset_key_cid) DO UPDATE SET reverted=false` | |||
| tipsetSeen = `SELECT height,reverted FROM events_seen WHERE tipset_key_cid=?` | |||
|
|
|||
| getEthTxHashCountForTipset = `SELECT COUNT(*) FROM eth_tx_hash WHERE message_cid IN (SELECT message_cid FROM tipset_message WHERE tipset_key_cid = ? AND reverted = 0)` | |||
There was a problem hiding this comment.
should we return data related to eth_tx_hash from ChainValidateIndex API in IndexValidation as well ?.
There was a problem hiding this comment.
@akaladarshi After thinking this through, I think we should return data for ETH TX hashes, however for now, let's just continue validating counts like we are currently doing. We can validate the actual contents as well as part of a future work stream but I think what we have is good for now..
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Please update the PR title to match https://github.com/filecoin-project/lotus/blob/master/CONTRIBUTING.md#pr-title-conventions
There was a problem hiding this comment.
Please update the PR title to match https://github.com/filecoin-project/lotus/blob/master/CONTRIBUTING.md#pr-title-conventions
akaladarshi
changed the title
github-actions
bot
dismissed
their stale review
PR title now matches the required format.
| }, | ||
| &cli.BoolFlag{ | ||
| Name: "backfill", | ||
| Usage: "Backfill missing index entries while validating the chain index. When enabled, the command will perform backfilling for any missing indexes (default: true)", |
There was a problem hiding this comment.
"will perform backfilling for any missing epochs in the index"
| if fromEpoch == 0 { | ||
| curTs, err := api.ChainHead(ctx) | ||
| if err != nil { | ||
| return err |
There was a problem hiding this comment.
need an informative error message
| } | ||
| fromEpoch = int(curTs.Height()) - 1 | ||
| } else { | ||
| fromEpoch = fromEpoch - 1 |
There was a problem hiding this comment.
why do we need this ?
There was a problem hiding this comment.
Only do this is fromEpoch >= head.Height()
There was a problem hiding this comment.
This was done because ChainValidateIndex fetches event of ts from ts+1, So in case of fromEpoch == head.height(), we can't get ts.Height() + 1.
So due to nature of ChainValidateIndex by default fromEpoch will not be included in backfilling, and to maintain consistency I did fromEpoch -1.
|
|
||
| toEpoch := cctx.Int("to") | ||
| if toEpoch > fromEpoch { | ||
| return fmt.Errorf("to epoch must be less than from epoch") |
| fromEpoch, toEpoch) | ||
|
|
||
| // starting from the FromEpoch-1 and going down to the ToEpoch | ||
| // this is because `ChainValidateIndex` fetches the tipset.height()+1, which might not be available in case of chain head |
There was a problem hiding this comment.
this comment is not clear. Wdym by "ChainValidateIndex fetches the tipset.height()+1" ?
| if failfast { | ||
| return fmt.Errorf("failed to validate index for epoch %d: %w", epoch, err) | ||
| } | ||
| log.Warnf("Error validating index for epoch %d: %v", epoch, err) |
| continue | ||
| } | ||
|
|
||
| if !indexValidateResp.TipSetKey.IsEmpty() || indexValidateResp.Height != uint64(epoch) && indexValidateResp.Backfilled == backfill { |
There was a problem hiding this comment.
I dont think we need this validation block here at all. The ChainIndexValidationAPI guruantees that it will return a valid indexValidateResp if error is nil.
|
@akaladarshi This looks mostly correct. Thanks for all the great work here 👍 Let me merge this, fill in the missing gaps and test it out e2e on a calibnet node. I will tag you for review on the changes I make. |
aarshkshah1992
merged commit e7506cc
into
filecoin-project:feat/implement-index-validation-api
This PR is a part of: #12450
PR changes:
lotus-shed indexes validate-chainindexto inspect and backfill the chain indexer throughChainValidateIndexAPI.