Skip to content

store: Reduce number of queries to update subgraph_deployment#3290

Merged
lutter merged 1 commit intomasterfrom
lutter/transact
Mar 5, 2022
Merged

store: Reduce number of queries to update subgraph_deployment#3290
lutter merged 1 commit intomasterfrom
lutter/transact

Conversation

@lutter
Copy link
Collaborator

@lutter lutter commented Feb 26, 2022

When updating the metadata in subgraph_deployment, we currently do two to
three roundtrips to the database to

  • update the block pointer
  • update the entity count
  • update the firehose cursor (sometimes)

With this change, all three roundtrips are folded into one update
statement.

Copy link
Collaborator

@leoyvens leoyvens left a comment

Choose a reason for hiding this comment

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

This will now write the firehose cursor when its value is "", ping @maoueh to confirm that the != "" check was just an optimization and not necessary for correctness.

full_count_query = full_count_query,
count = count
)
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

This duplication with fn update_entity_count is unfortunate, would it be possible to not duplicate this code?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I factored that scary comment and string generation into a separate function.

@maoueh
Copy link
Contributor

maoueh commented Feb 28, 2022

@leoyvens @lutter It's all good to write the cursor if it's "", a Firehose cursor of value "" is semantically equal to no cursor at all. Ultimately, at the request level, we send "" to say to the service there is no cursor, so it's all good.

The thing is will change however is that it will effectively delete the firehose cursor it it existed. But that's good since upcoming PR #3174 was doing it already.

Either this PR or #3174 will need to be updated to remove the https://github.com/graphprotocol/graph-node/pull/3174/files#diff-ec9db820ff81f5904c7de003a414c9756dd8eb81b539122f0a70811752f2f313R125

@lutter lutter force-pushed the lutter/transact branch 2 times, most recently from 0f75178 to df2eda7 Compare February 28, 2022 18:23
@lutter
Copy link
Collaborator Author

lutter commented Feb 28, 2022

I also changed the logic of deployment::transact_block to set the firehose cursor to null when it is Some("") - even if it's all the same to the firehose, it's cleaner that way.

When updating the metadata in subgraph_deployment, we currently do two to
three roundtrips to the database to

  - update the block pointer
  - update the entity count
  - update the firehose cursor (sometimes)

With this change, all three roundtrips are folded into one update
statement.
@lutter lutter force-pushed the lutter/transact branch from df2eda7 to 2ac2c24 Compare March 5, 2022 01:48
@lutter lutter merged commit 2ac2c24 into master Mar 5, 2022
@lutter lutter deleted the lutter/transact branch March 5, 2022 01:51
maoueh added a commit that referenced this pull request Mar 7, 2022
…ndexing

Now that PR #3290 has been merged, which is always writing back the firehose cursor to the database, meaning it will clear any existing Firehose cursor for a subgraph that started from RPC but than switched to Firehose, it means it's not required to clear any existing cursor when starting a RPC subgraph.
maoueh added a commit that referenced this pull request Mar 11, 2022
…ndexing (#3331)

Now that PR #3290 has been merged, which is always writing back the firehose cursor to the database, meaning it will clear any existing Firehose cursor for a subgraph that started from RPC but than switched to Firehose, it means it's not required to clear any existing cursor when starting a RPC subgraph.

Co-authored-by: Matthieu Vachon <matt@streamingfast.io>
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.

3 participants