fix: actor events: read entries from event index in order of their insertion #11829
Conversation
aarshkshah1992
changed the title
aarshkshah1992
changed the title
chain/events/filter/index.go
Outdated
| // create struct for holding EventEntry with ordering | ||
| type eventEntryWithOrdering struct { | ||
| types.EventEntry | ||
| ordering uint64 |
There was a problem hiding this comment.
I don't think we need to select this field, we just need to order by it when querying.
There was a problem hiding this comment.
@Stebalien For some reason, if I add this to the ordering, we get way more records than we want and the tests start failing. I spent time debugging it before giving up.
I had changed the current ordering to s += "ORDER BY event.height DESC, event_entry.ordering ASC".
There was a problem hiding this comment.
Are you sure the issue wasn't the removal of the space? Unless I'm mistaken, that should have worked.
chain/events/filter/index.go
Outdated
| // sort the entries in the collected event by ordering | ||
| for _, ce := range ces { | ||
| ce := ce | ||
| sort.Slice(ce.entries, func(i, j int) bool { return ce.entries[i].ordering < ce.entries[j].ordering }) |
| @@ -672,8 +739,37 @@ func (ei *EventIndex) prefillFilter(ctx context.Context, f *eventFilter, exclude | |||
|
|
|||
| // collected event list is in inverted order since we selected only the most recent events | |||
| // sort it into height order | |||
| sort.Slice(ces, func(i, j int) bool { return ces[i].Height < ces[j].Height }) | |||
| f.setCollectedEvents(ces) | |||
| sort.Slice(ces, func(i, j int) bool { return ces[i].height < ces[j].height }) | |||
There was a problem hiding this comment.
I'm not sure why we bother with this at all...
There was a problem hiding this comment.
I agree. It was already here so didn't want to mess with it.
chain/events/filter/index.go
Outdated
| // rollback the transaction (a no-op if the transaction was already committed) | ||
| defer tx.Rollback() //nolint:errcheck | ||
|
|
||
| if _, err = tx.ExecContext(ctx, "ALTER TABLE event_entry ADD COLUMN ordering INTEGER NOT NULL DEFAULT 0"); err != nil { |
There was a problem hiding this comment.
I'd prefer to actually fill these (may be possible in a single statement?).
|
This should be closed once #11834 is merged. |
Related Issues
Closes #11823
Proposed Changes
Event entries should be read from the event index in the order of their insertion
Additional Info
Checklist
Before you mark the PR ready for review, please make sure that:
<PR type>: <area>: <change being made>fix: mempool: Introduce a cache for valid signaturesPR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, testarea, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps