feat: chain indexer todos [skip changelog] #12462
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,6 +60,11 @@ func (si *SqliteIndexer) ChainValidateIndex(ctx context.Context, epoch abi.Chain | |
| if !si.started { | ||
| return nil, xerrors.Errorf("ChainValidateIndex can only be called after the indexer has been started") | ||
| } | ||
|
|
||
| if si.isClosed() { | ||
| return nil, xerrors.Errorf("ChainValidateIndex can only be called before the indexer has been closed") | ||
| } | ||
|
|
||
| si.writerLk.Lock() | ||
| defer si.writerLk.Unlock() | ||
|
|
||
|
|
@@ -126,17 +131,107 @@ func (si *SqliteIndexer) ChainValidateIndex(ctx context.Context, epoch abi.Chain | |
| return nil, xerrors.Errorf("failed to cast tipset key cid: %w", err) | ||
| } | ||
|
|
||
| if !indexedTsKeyCid.Equals(expectedTsKeyCid) { | ||
| return nil, xerrors.Errorf("index corruption: non-reverted tipset at height %d has key %s, but canonical chain has %s", epoch, indexedTsKeyCid, expectedTsKeyCid) | ||
| } | ||
|
|
||
| // indexedTsKeyCid and expectedTsKeyCid are the same, so we can use `expectedTs` to fetch the indexed data | ||
| indexedData, err := si.getIndexedTipSetData(ctx, expectedTs) | ||
| if err != nil { | ||
| return nil, xerrors.Errorf("failed to get tipset message and event counts at height %d: %w", expectedTs.Height(), err) | ||
| } | ||
|
|
||
| if indexedData == nil { | ||
| return nil, xerrors.Errorf("invalid indexed data for tipset at height %d", expectedTs.Height()) | ||
| } | ||
|
|
||
| if err = si.verifyIndexedData(ctx, expectedTs, indexedData); err != nil { | ||
| return nil, xerrors.Errorf("failed to verify indexed data at height %d: %w", expectedTs.Height(), err) | ||
| } | ||
|
|
||
| return &types.IndexValidation{ | ||
| TipsetKey: expectedTs.Key().String(), | ||
| Height: uint64(expectedTs.Height()), | ||
| NonRevertedMessageCount: uint64(indexedData.nonRevertedMessageCount), | ||
| NonRevertedEventsCount: uint64(indexedData.nonRevertedEventCount), | ||
| }, nil | ||
| } | ||
|
|
||
| type indexedTipSetData struct { | ||
| nonRevertedMessageCount int | ||
| nonRevertedEventCount int | ||
| } | ||
|
|
||
| // getIndexedTipSetData fetches the indexed tipset data for a tipset | ||
| func (si *SqliteIndexer) getIndexedTipSetData(ctx context.Context, ts *types.TipSet) (*indexedTipSetData, error) { | ||
|
There was a problem hiding this comment. how about you just return a mostly-filled |
||
| tsKeyCidBytes, err := toTipsetKeyCidBytes(ts) | ||
| if err != nil { | ||
| return nil, xerrors.Errorf("failed to get tipset key cid: %w", err) | ||
| } | ||
|
|
||
| var data indexedTipSetData | ||
| err = withTx(ctx, si.db, func(tx *sql.Tx) error { | ||
| if err = tx.Stmt(si.stmts.getNonRevertedTipsetMessageCountStmt).QueryRowContext(ctx, tsKeyCidBytes).Scan(&data.nonRevertedMessageCount); err != nil { | ||
| return xerrors.Errorf("failed to query non reverted message count: %w", err) | ||
| } | ||
|
|
||
| if err = tx.Stmt(si.stmts.getNonRevertedTipsetEventCountStmt).QueryRowContext(ctx, tsKeyCidBytes).Scan(&data.nonRevertedEventCount); err != nil { | ||
| return xerrors.Errorf("failed to query non reverted event count: %w", err) | ||
| } | ||
|
|
||
| return nil | ||
| }) | ||
|
|
||
| return &data, err | ||
| } | ||
|
|
||
| // verifyIndexedData verifies that the indexed data for a tipset is correct | ||
| // by comparing the number of messages and events in the chainstore to the number of messages and events indexed. | ||
| // NOTE: Events are loaded from the executed messages of the tipset at the next epoch (ts.Height() + 1). | ||
| func (si *SqliteIndexer) verifyIndexedData(ctx context.Context, ts *types.TipSet, indexedData *indexedTipSetData) (err error) { | ||
|
There was a problem hiding this comment. ah I see why you want a struct for this, but it's still just 2 ints and it might even be clearer to provide them as named arguments here |
||
| tsKeyCid, err := ts.Key().Cid() | ||
| if err != nil { | ||
| return xerrors.Errorf("failed to get tipset key cid: %w", err) | ||
| } | ||
|
|
||
| // get the tipset where the messages of `ts` will be executed (deferred execution) | ||
| executionTs, err := si.cs.GetTipsetByHeight(ctx, ts.Height()+1, nil, false) | ||
|
There was a problem hiding this comment. see discussion in #12447 (comment), same problem here I think, particularly as you get close to the tip of the chain There was a problem hiding this comment. @rvagg Which is why we error out here if the parent of this tipset is not the original tipset. We can then ask the user to retry as that means that the chain has forked between the two tipsets. There was a problem hiding this comment. I don't think we want to re-execute the tipset/regenerate events here. |
||
| if err != nil { | ||
| return xerrors.Errorf("failed to get tipset by height: %w", err) | ||
| } | ||
|
|
||
| eParentTsKeyCid, err := executionTs.Parents().Cid() | ||
| if err != nil { | ||
| return xerrors.Errorf("failed to get execution tipset parent key cid: %w", err) | ||
| } | ||
|
|
||
| // the parent tipset of the execution tipset should be the same as the indexed tipset (`ts` should be the parent of `executionTs`) | ||
| if !eParentTsKeyCid.Equals(tsKeyCid) { | ||
| return xerrors.Errorf("execution tipset parent key mismatch: chainstore has %s, index has %s", eParentTsKeyCid, tsKeyCid) | ||
| } | ||
|
|
||
| executedMsgs, err := si.loadExecutedMessages(ctx, ts, executionTs) | ||
| if err != nil { | ||
| return xerrors.Errorf("failed to load executed messages: %w", err) | ||
| } | ||
|
|
||
| totalEventsCount := 0 | ||
| for _, emsg := range executedMsgs { | ||
| totalEventsCount += len(emsg.evs) | ||
| } | ||
|
|
||
| if totalEventsCount != indexedData.nonRevertedEventCount { | ||
| return xerrors.Errorf("tipset event count mismatch: chainstore has %d, index has %d", totalEventsCount, indexedData.nonRevertedEventCount) | ||
| } | ||
|
|
||
| totalExecutedMsgCount := len(executedMsgs) | ||
| if totalExecutedMsgCount != indexedData.nonRevertedMessageCount { | ||
| return xerrors.Errorf("tipset executed message count mismatch: chainstore has %d, index has %d", totalExecutedMsgCount, indexedData.nonRevertedMessageCount) | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (si *SqliteIndexer) backfillMissingTipset(ctx context.Context, ts *types.TipSet, backfill bool) (*types.IndexValidation, error) { | ||
| if !backfill { | ||
| return nil, xerrors.Errorf("missing tipset at height %d in the chain index, set backfill to true to backfill", ts.Height()) | ||
|
|
@@ -159,9 +254,16 @@ func (si *SqliteIndexer) backfillMissingTipset(ctx context.Context, ts *types.Ti | |
| return nil, xerrors.Errorf("error applying tipset: %w", err) | ||
| } | ||
|
|
||
| indexedData, err := si.getIndexedTipSetData(ctx, ts) | ||
| if err != nil { | ||
| return nil, xerrors.Errorf("failed to get tipset message and event counts at height %d: %w", ts.Height(), err) | ||
| } | ||
|
|
||
| return &types.IndexValidation{ | ||
| TipsetKey: ts.Key().String(), | ||
| Height: uint64(ts.Height()), | ||
| Backfilled: true, | ||
| NonRevertedMessageCount: uint64(indexedData.nonRevertedMessageCount), | ||
| NonRevertedEventsCount: uint64(indexedData.nonRevertedEventCount), | ||
| }, nil | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -82,5 +82,7 @@ func preparedStatementMapping(ps *preparedStatements) map[**sql.Stmt]string { | |
| &ps.hasNullRoundAtHeightStmt: "SELECT NOT EXISTS(SELECT 1 FROM tipset_message WHERE height = ?)", | ||
| &ps.getNonRevertedTipsetAtHeightStmt: "SELECT tipset_key_cid FROM tipset_message WHERE height = ? AND reverted = 0", | ||
| &ps.countTipsetsAtHeightStmt: "SELECT COUNT(CASE WHEN reverted = 1 THEN 1 END) AS reverted_count, COUNT(CASE WHEN reverted = 0 THEN 1 END) AS non_reverted_count FROM (SELECT tipset_key_cid, MAX(reverted) AS reverted FROM tipset_message WHERE height = ? GROUP BY tipset_key_cid) AS unique_tipsets", | ||
| &ps.getNonRevertedTipsetMessageCountStmt: "SELECT COUNT(*) FROM tipset_message WHERE tipset_key_cid = ? AND reverted = 0", | ||
| &ps.getNonRevertedTipsetEventCountStmt: "SELECT COUNT(*) FROM event WHERE message_id IN (SELECT message_id FROM tipset_message WHERE tipset_key_cid = ? AND reverted = 0)", | ||
|
There was a problem hiding this comment. @akaladarshi What if the tipset is NOT reverted but the events are reverted ? This query only ensures that the tipset is NOT reverted. Does it have any implications on the correctness of the code /API ? There was a problem hiding this comment. When you say There was a problem hiding this comment. would it be corruption if one were reverted and the other not? is that something we should also be checking for? |
||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
ensure
indexedDatais not nil before proceeding here.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.
Ideally, it will only be nil if error is there and if there is error and
indexedDatais not nil then that means issue is in DB, so we should return.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.
Yes but no harm in checking it and returning an error of it is nil here so user's node dosen't crash if there exists a bug that causes a nil value here.
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.
Done.