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.

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

Add skipBlock option to improve performance with event handlers #1968

Merged
merged 3 commits into from
Aug 28, 2023

Conversation

stwiname
Copy link
Collaborator

@stwiname stwiname commented Aug 23, 2023

Description

If a project only has event handlers there is not always a need to fetch all the block content. This can lead to ~10% performance increase in indexing speed.

skipBlock can be set in node runner options or in cli flags. It will automatically disable this if handlers other than event handlers are found.

I also found a bug where node runner options in project.yaml were broken because of default values in yargs. This will need to be fixed in other SDKs

Completes #1889 for Substrate

Type of change

Please delete options that are not relevant.

Checklist

@stwiname stwiname requested a review from jiqiang90 August 23, 2023 05:27
@stwiname stwiname force-pushed the project-upgrades branch 2 times, most recently from 998ac54 to f25d338 Compare August 27, 2023 22:30
@stwiname stwiname changed the base branch from project-upgrades to main August 28, 2023 03:04
return true;
}

export function isOnlyEventHandlers(project: SubqueryProject): boolean {
Copy link
Contributor

Choose a reason for hiding this comment

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

We could define event only handler in this way, but we could face another issue, we could not know whether user try to access block content from event which we have wrapped previously.

const wrappedEvents = wrapEvents(wrappedExtrinsics, events, wrappedBlock);

We can test this scenarios with starter, and we found error was thrown, but not clearly enough, could be very difficult to debug.

https://github.com/subquery/subql-starter/blob/96b09b6aa3a0c6506c423acf628f117cd47e9f83/Polkadot/Polkadot-starter/src/mappings/mappingHandlers.ts#L41

image

I think we could implement within wrapped event, if block content is accessed in handler, we could provide more friendly warning/error. Or even refetch the block content.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is why there is the skipBlock option, users have to opt into it and test it themselves. They should update their handler types accordingly

@stwiname stwiname merged commit f045e32 into main Aug 28, 2023
@stwiname stwiname deleted the skip-block branch October 18, 2023 03:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants