enable static filters for SGs with many dynamic datasource #4008
Conversation
| @@ -230,6 +230,9 @@ impl<S: SubgraphStore> SubgraphInstanceManager<S> { | |||
| (manifest, manifest_idx_and_name) | |||
| }; | |||
|
|
|||
| let static_filters = self.static_filters | |||
| || (manifest.network_name() == "bsc" && manifest.templates.len() > 10000); | |||
There was a problem hiding this comment.
I think it would make sense to do this for any network.
There was a problem hiding this comment.
@leoyvens thoughts about this?
| @@ -230,6 +230,9 @@ impl<S: SubgraphStore> SubgraphInstanceManager<S> { | |||
| (manifest, manifest_idx_and_name) | |||
| }; | |||
|
|
|||
| let static_filters = self.static_filters | |||
| || (manifest.network_name() == "bsc" && manifest.templates.len() > 10000); | |||
There was a problem hiding this comment.
Isn't manifest.templates.len() just the number of templates in the manifest? We want to count actual dynamic data sources, so want to look at data_sources.len() ?
There was a problem hiding this comment.
Yeah I was trying to avoid including the data_sources but you're right, it's prolly a few in 10000 so that should be fine, I'll change this
There was a problem hiding this comment.
actually this also uses the previously stored data sources, so I need to trim those as well
|
Looking at how static filters are currently implemented, this might not have the intended effect. Because it currently will add the filters from the templates but it doesn't prevent the filters from the dynamic data sources from being added. When static filters are enabled, we need the final filter to be static data sources + templates, ignoring the stored dynamic data sources. Edit: I see @mangas just said the same thing in the above thread. |
mangas
force-pushed
the
enable-bsc-static-filters
branch
from
491fef9
to
bd6ad09
Compare
| @@ -218,18 +218,28 @@ impl<S: SubgraphStore> SubgraphInstanceManager<S> { | |||
|
|
|||
| info!(logger, "Successfully resolved subgraph files using IPFS"); | |||
|
|
|||
| // Add dynamic data sources to the subgraph | |||
| manifest.data_sources.extend(data_sources); | |||
|
|
|||
| info!( | |||
| logger, | |||
| "Data source count at start: {}", | |||
| manifest.data_sources.len() | |||
| ); | |||
There was a problem hiding this comment.
We do want this log to include the count of dynamic data sources.
| let dds_count = manifest | ||
| .data_sources | ||
| .len() | ||
| .abs_diff(static_data_sources.len()); |
There was a problem hiding this comment.
I wouldn't bother doing the diff and would just use the total len but this is also fine.
| let onchain_data_sources = static_data_sources | ||
| .iter() | ||
| .filter_map(|d| d.as_onchain().cloned()) | ||
| .collect::<Vec<_>>(); |
There was a problem hiding this comment.
No need to collect if iter() will be called right after.
| // if static_filters is enabled, build a minimal filter with the static data sources and | ||
| // add the necessary filters based on templates. | ||
| // if not enabled we just stick to the filter based on all the data sources. | ||
| // This changes specifically removes dynamic data sources based filters because these can be derived |
There was a problem hiding this comment.
| // This changes specifically removes dynamic data sources based filters because these can be derived | |
| // This specifically removes dynamic data sources based filters because these can be derived |
mangas
force-pushed
the
enable-bsc-static-filters
branch
from
bd6ad09
to
bb47b12
Compare
mangas
force-pushed
the
enable-bsc-static-filters
branch
from
bb47b12
to
cfc88b0
Compare
mangas
force-pushed
the
enable-bsc-static-filters
branch
from
31ab1d3
to
fde0d55
Compare
mangas
changed the title
Add a subgraph start check for number of dynamic data sources. If above the defined threshold, force static filters.