-
Notifications
You must be signed in to change notification settings - Fork 179
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
[Access] Add support for indexing execution data on Observers #5256
[Access] Add support for indexing execution data on Observers #5256
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5256 +/- ##
==========================================
- Coverage 55.84% 55.79% -0.06%
==========================================
Files 1030 1031 +1
Lines 100843 100860 +17
==========================================
- Hits 56320 56274 -46
- Misses 40186 40255 +69
+ Partials 4337 4331 -6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
…o AndriiDiachuk/access-add-support-for-indexing-execution-data
…://github.com/UlyanaAndrukhiv/flow-go into AndriiDiachuk/access-add-support-for-indexing-execution-data
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.
I didn't review all of the changes since I know some are duplicated from #5253.
Added a couple comments about where to place the collection handler, but this looks pretty close
…://github.com/UlyanaAndrukhiv/flow-go into AndriiDiachuk/access-add-support-for-indexing-execution-data
…-data' of https://github.com/AndriiDiachuk/flow-go into AndriiDiachuk/access-add-support-for-indexing-execution-data
…ing-execution-data
@peterargue Ready for a new round of review. |
…ing-execution-data
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.
added a couple small comments, but otherwise this looks ready to go
module/state_synchronization/indexer/collection_executed_metric.go
Outdated
Show resolved
Hide resolved
module/state_synchronization/indexer/collection_executed_metric.go
Outdated
Show resolved
Hide resolved
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.
looks good. can you add indexing to the default localnet config for observers here:
https://github.com/onflow/flow-go/blob/master/integration/localnet/builder/bootstrap.go#L473
"--execution-data-indexing-enabled=true",
"--execution-state-dir=/data/execution-state",
@peterargue It was added couple if lines above. But I moved all together now. |
…ing-execution-data
Thanks. Not sure how I missed that |
…support-for-indexing-execution-data [Access] Add support for indexing execution data on Observers
This PR includes implementations for the following issue:
Add execution state checkpoint and indexing support for the Observer node (#4848).
This branch was merged with #5253 as it needs some parts for the issue above.