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.

Sign up for GitHub

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

Jump to bottom

fix: events index: record processed epochs and tipsets for events and eth_get_log blocks till requested tipset has been indexed #12080

Merged
merged 20 commits into from
Jun 20, 2024
Merged

fix: events index: record processed epochs and tipsets for events and eth_get_log blocks till requested tipset has been indexed #12080

merged 20 commits into from
Jun 20, 2024

Conversation

aarshkshah1992
Copy link
Contributor

@aarshkshah1992 aarshkshah1992 commented Jun 12, 2024 •
edited
Loading

Closes #11640 and #12078

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 12, 2024 •
edited
Loading

All checks have passed

Stebalien
Stebalien reviewed Jun 12, 2024
View reviewed changes
Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

I have some opinions but this really needs a look by @rvagg (who has done more thinking on this than I have at this point).

chain/events/filter/index.go Outdated Show resolved Hide resolved
chain/events/filter/index.go Outdated Show resolved Hide resolved
chain/events/filter/index.go Outdated Show resolved Hide resolved
@Stebalien Stebalien requested a review from rvagg June 12, 2024 18:19
@aarshkshah1992 aarshkshah1992 changed the title fix: events index: record processed epochs and tipsets for events fix: events index: record processed epochs and tipsets for events and eth_get_log blocks till requested tipset has been indexed Jun 14, 2024
rvagg
rvagg reviewed Jun 17, 2024
View reviewed changes
chain/events/filter/index.go Outdated Show resolved Hide resolved
rvagg
rvagg reviewed Jun 17, 2024
View reviewed changes
chain/events/filter/index.go Outdated Show resolved Hide resolved
rvagg
rvagg reviewed Jun 17, 2024
View reviewed changes
chain/events/filter/index.go Outdated Show resolved Hide resolved
rvagg
rvagg reviewed Jun 17, 2024
View reviewed changes
chain/events/filter/index.go Outdated Show resolved Hide resolved
rvagg
rvagg reviewed Jun 17, 2024
View reviewed changes
chain/events/filter/index.go Outdated Show resolved Hide resolved
rvagg
rvagg reviewed Jun 17, 2024
View reviewed changes
chain/events/filter/index.go Outdated Show resolved Hide resolved
rvagg
rvagg reviewed Jun 17, 2024
View reviewed changes
chain/events/filter/index.go Outdated Show resolved Hide resolved
rvagg
rvagg reviewed Jun 17, 2024
View reviewed changes
node/impl/full/eth.go Outdated Show resolved Hide resolved
rvagg
rvagg reviewed Jun 17, 2024
View reviewed changes
node/impl/full/eth.go Outdated Show resolved Hide resolved
@rvagg rvagg mentioned this pull request Jun 17, 2024
Merged
rvagg
rvagg approved these changes Jun 17, 2024
View reviewed changes
Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

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

I think this looks ok, but it's slightly risky so I'd be more comfortable having someone else look too, @Stebalien if available would be good

@aarshkshah1992
Copy link
Contributor Author

@rvagg Would love a review from @Stebalien too but which part do you find risky ?

Stebalien
Stebalien reviewed Jun 18, 2024
View reviewed changes
chain/events/filter/index.go Outdated Show resolved Hide resolved
chain/events/filter/index.go Show resolved Hide resolved
chain/events/filter/index.go Outdated Show resolved Hide resolved
chain/events/filter/index.go Outdated Show resolved Hide resolved
chain/events/filter/index.go Outdated Show resolved Hide resolved
node/impl/full/eth.go Show resolved Hide resolved
node/impl/full/eth.go Show resolved Hide resolved
node/impl/full/eth.go Show resolved Hide resolved
@Stebalien
Copy link
Member

FYI, #12111

@aarshkshah1992
Copy link
Contributor Author

@Stebalien I have addressed all your review comments.

Except for #12080 (comment) which I would like to address as part of the refactor in #12116 because that really fixes this in a clean way rather than patching the current broken dual write/dual read mechanism we have for (ChainNotify writes to filter buffer and index DB -> event APIs read from both filter buffer and index DB).

@aarshkshah1992
Copy link
Contributor Author

@rvagg @Stebalien I'd like to merge this. Based on an offline conversation with @Stebalien -> he agrees this is good to go in it's current form as it does fix the missing event issue withGetLogs.

We will prioritise #12116 next.

rvagg
rvagg reviewed Jun 20, 2024
View reviewed changes
chain/events/filter/index.go Outdated Show resolved Hide resolved
rvagg
rvagg reviewed Jun 20, 2024
View reviewed changes
node/impl/full/eth.go Outdated Show resolved Hide resolved
rvagg
rvagg reviewed Jun 20, 2024
View reviewed changes
node/impl/full/eth.go Outdated Show resolved Hide resolved
rvagg
rvagg reviewed Jun 20, 2024
View reviewed changes
chain/events/filter/index.go Outdated Show resolved Hide resolved
rvagg
rvagg reviewed Jun 20, 2024
View reviewed changes
node/impl/full/eth.go Outdated Show resolved Hide resolved
rvagg
rvagg reviewed Jun 20, 2024
View reviewed changes
node/impl/full/eth.go Outdated Show resolved Hide resolved
aarshkshah1992 and others added 2 commits June 20, 2024 10:53
@aarshkshah1992 @rvagg
Apply suggestions from code review
4869218 
Co-authored-by: Rod Vagg <rod@vagg.org>
@aarshkshah1992
reduce duplication
f42a5f4
@aarshkshah1992 aarshkshah1992 merged commit c87e2f2 into master Jun 20, 2024
77 checks passed
@aarshkshah1992 aarshkshah1992 deleted the feat/record-seen-event-epochs branch June 20, 2024 08:00
@aarshkshah1992 aarshkshah1992 mentioned this pull request Jun 20, 2024
Closed
7 tasks
@rvagg rvagg mentioned this pull request Jun 27, 2024
Closed
18 tasks
rvagg added a commit that referenced this pull request Jun 27, 2024
@aarshkshah1992 @rvagg
fix: events index: record processed epochs and tipsets for events and…
b544ebb 
… eth_get_log blocks till requested tipset has been indexed (#12080)

* record seen event epochs

* create correct index

* migrate to version 6

* fix typo

* test both conditions

* changes as per review

* record reverted tipsets

* see if tipsets has events and has not been reverted

* sub/unsub tipset updates from the index

* eth_get_logs should wait for events

* fix naming

* changes as per review

* solve issue with missing events

* use correct var

* changes as per review

* add unique constraint

* fix test wait

* check for events at min_height as well

* Apply suggestions from code review

Co-authored-by: Rod Vagg <rod@vagg.org>

* reduce duplication

---------

Co-authored-by: Rod Vagg <rod@vagg.org>
@rvagg rvagg mentioned this pull request Jun 27, 2024
Merged
rvagg added a commit that referenced this pull request Jul 10, 2024
@aarshkshah1992 @rvagg
fix: events index: record processed epochs and tipsets for events and…
c98a572 
… eth_get_log blocks till requested tipset has been indexed (#12080)

* record seen event epochs

* create correct index

* migrate to version 6

* fix typo

* test both conditions

* changes as per review

* record reverted tipsets

* see if tipsets has events and has not been reverted

* sub/unsub tipset updates from the index

* eth_get_logs should wait for events

* fix naming

* changes as per review

* solve issue with missing events

* use correct var

* changes as per review

* add unique constraint

* fix test wait

* check for events at min_height as well

* Apply suggestions from code review

Co-authored-by: Rod Vagg <rod@vagg.org>

* reduce duplication

---------

Co-authored-by: Rod Vagg <rod@vagg.org>
rvagg added a commit that referenced this pull request Jul 10, 2024
@aarshkshah1992 @rvagg
fix: events index: record processed epochs and tipsets for events and…
4ace8c9 
… eth_get_log blocks till requested tipset has been indexed (#12080)

* record seen event epochs

* create correct index

* migrate to version 6

* fix typo

* test both conditions

* changes as per review

* record reverted tipsets

* see if tipsets has events and has not been reverted

* sub/unsub tipset updates from the index

* eth_get_logs should wait for events

* fix naming

* changes as per review

* solve issue with missing events

* use correct var

* changes as per review

* add unique constraint

* fix test wait

* check for events at min_height as well

* Apply suggestions from code review

Co-authored-by: Rod Vagg <rod@vagg.org>

* reduce duplication

---------

Co-authored-by: Rod Vagg <rod@vagg.org>
@rvagg rvagg mentioned this pull request Aug 2, 2024
Closed
@BigLep BigLep mentioned this pull request Aug 6, 2024
Closed
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Stebalien Stebalien Stebalien left review comments

@rvagg rvagg rvagg approved these changes

Assignees

@aarshkshah1992 aarshkshah1992

Labels
None yet
Projects
FilOz
Status: ☑️ Done (Archive)
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Record processed epochs / tipsets in events index
3 participants
@aarshkshah1992 @Stebalien @rvagg