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

perf: Graphql CDN using Stellate #7623

Merged
merged 4 commits into from
Nov 2, 2023
Merged

perf: Graphql CDN using Stellate #7623

merged 4 commits into from
Nov 2, 2023

Conversation

preschian
Copy link
Member

@preschian preschian commented Oct 12, 2023

Hi @kodadot/internal-dev. Would you like to try Stellate for our GraphQL endpoint? In a simple comparison, it appears to be faster. This could help us if we get a large number of visitors at the same time. Perhaps if the number of visitors is not too high, the effect might not be too noticeable. For now, I’ve set the cache on this endpoint to last for 60 seconds.

Before After
Screenshot 2023-10-12 152306 Screenshot 2023-10-12 152243
https://canary.kodadot.xyz/ahk/collection/150 https://deploy-preview-7623--koda-canary.netlify.app/ahk/collection/150

https://stellate.co/docs/graphql-edge-cache/cache-rules

// cache rules

rules: [
  {
    types: [
      'NFTEntity',
      'NFTEntitiesConnection',
      'CollectionEntity',
      'CollectionEntitiesConnection',
      'Event',
      'EventEntity',
      'LastEventEntity'
    ],
    maxAge: 60,
    swr: 600,
    description: 'Cache some types',
  },
],

PR Type

  • Bugfix
  • Feature
  • Refactoring

Context

Had issue bounty label?

Copilot Summary

🤖 Generated by Copilot at 45af347

This pull request adds support for the new Stellate indexers, which are a different kind of Subsquid indexer for the NFT gallery. It updates the indexers.ts and constants.ts files to handle both Stellate and Squid indexers and use the appropriate one for each network.

🤖 Generated by Copilot at 45af347

SquidEndpoint changes
Stellate indexers join in
Autumn of networks

@netlify
Copy link

netlify bot commented Oct 12, 2023

Deploy Preview for koda-canary ready!

Name Link
🔨 Latest commit e792f52
🔍 Latest deploy log https://app.netlify.com/sites/koda-canary/deploys/653f7c8ba1486f0008270c02
😎 Deploy Preview https://deploy-preview-7623--koda-canary.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@preschian preschian changed the title pref: stellate graphql perf: stellate graphql Oct 12, 2023
@reviewpad
Copy link
Contributor

reviewpad bot commented Oct 12, 2023

AI-Generated Summary: This pull request includes changes to two files: indexers.ts and constants.ts. The indexer endpoint for 'ahk' in indexers.ts and 'stick' in constants.ts have been updated to 'https://query-stick.stellate.sh'. The addition of this new URL has also been included in the SquidEndpoint type in indexers.ts.

@reviewpad reviewpad bot added the small Pull request is small label Oct 12, 2023
@reviewpad
Copy link
Contributor

reviewpad bot commented Oct 12, 2023

Reviewpad Report

⚠️ Warnings

  • Please link an issue to the pull request

@preschian preschian marked this pull request as ready for review October 12, 2023 08:45
@preschian preschian requested a review from a team as a code owner October 12, 2023 08:45
@preschian preschian requested review from roiLeo and Jarsen136 and removed request for a team October 12, 2023 08:45
@preschian preschian marked this pull request as draft October 12, 2023 08:47
Copy link
Contributor

@roiLeo roiLeo left a comment

Choose a reason for hiding this comment

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

How does it work? does it only cache data?
Will we need it for all indexers? wouldn't we have a problem on the billing?
Do you have acces at some sort of backend dashboard?

@preschian
Copy link
Member Author

How does it work? does it only cache data?

It wraps the subsquid endpoint with stellate

before:
user -> https://squid.subsquid.io/stick/graphql

after:
user -> https://query-stick.stellate.sh/ -> cache miss -> https://squid.subsquid.io/stick/graphql
user -> https://query-stick.stellate.sh/ -> cache hit -> return the cache response

Will we need it for all indexers? wouldn't we have a problem on the billing?

for testing maybe we can test with one endpoint first? they have a free plan with unlimited requests. the limitation is only working on 2 endpoints, and need to put their badge

Do you have acces at some sort of backend dashboard?

I just created the account by myself. what we need only subsquid endpoint to use stellate

@Jarsen136
Copy link
Contributor

Jarsen136 commented Oct 12, 2023

Caching graphql is a good idea, but I also have some concerns.

  • I wonder if it supports bypassing the cache. For example, if a user buy a nft we need to fetch the latest information as fast as possible instead of cache.

  • Is it compatible with the current real-time subscription feature useSubscriptionGraphql? I'm afraid the cache would affect it.

@preschian
Copy link
Member Author

preschian commented Oct 12, 2023

Caching graphql is a good idea, but I also have some concerns.

  • I wonder if it supports bypassing the cache. For example, if a user buy a nft we need to fetch the latest information as fast as possible instead of cache.

we can use custom header to bypass the cache https://stellate.co/docs/graphql-edge-cache/bypass-the-cache

or, not putting all Types in the cache. currently in the rules I put

[
  'NFTEntity',
  'NFTEntitiesConnection',
  'CollectionEntity',
  'CollectionEntitiesConnection',
  'Event',
  'EventEntity',
  'LastEventEntity',
]
  • Is it compatible with the current real-time subscription feature useSubscriptionGraphql? I'm afraid the cache would affect it.

interesting, do we need wss endpoint for subscription call? seems like Stellate can't support that. ref: https://feedback.stellate.co/feature-requests/p/support-websocket-subscriptions

ws subscription error

image

unfortunately, we need to adjust the subscription over HTTP instead of WS

@yangwao
Copy link
Member

yangwao commented Oct 12, 2023

Ah GraphCDN => Stellate?

We've been considering this one in the past.

…ss://squid.subsquid.io/stick/graphql' as the WebSocket URL
@preschian
Copy link
Member Author

For the subscription query, what if we fallback to the subsquid endpoint? like this c01503c

Ah GraphCDN => Stellate?

We've been considering this one in the past.

yes, cdn for graphql

@preschian preschian changed the title perf: stellate graphql perf: graphql cdn using stellate Oct 12, 2023
@preschian preschian changed the title perf: graphql cdn using stellate perf: Graphql CDN using Stellate Oct 12, 2023
@vikiival
Copy link
Member

So looking forward to hear opinion from @kodadot/internal-dev about using stellate.

unfortunately, we need to adjust the subscription over HTTP instead of WS

We can fallback to the sqd endpoint.
the point why we use WebSocket on some parts of the app is the reason that we need fresh data.

But I would be happy to try it on AssetHubs (2 endpoints) and if we would see potential benefit we can scale from there.

@roiLeo
Copy link
Contributor

roiLeo commented Oct 26, 2023

So looking forward to hear opinion from @kodadot/internal-dev about using stellate.

meh.. I have no opinion

But I would be happy to try it on AssetHubs (2 endpoints) and if we would see potential benefit we can scale from there.

yes we could try that first

* fix(TheFooter.vue): add missing import for useTheme
* refactor(TheFooter.vue): remove unused import for TranslateResult
@sonarcloud
Copy link

sonarcloud bot commented Oct 30, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@codeclimate
Copy link

codeclimate bot commented Oct 30, 2023

Code Climate has analyzed commit e792f52 and detected 0 issues on this pull request.

View more on Code Climate.

@preschian
Copy link
Member Author

lets try it then? @vikiival
added Stellate badge in the footer

image

@preschian preschian marked this pull request as ready for review October 30, 2023 09:52
@vikiival
Copy link
Member

added Stellate badge in the footer

Shouldn't be the badge in the readme?

@yangwao
Copy link
Member

yangwao commented Nov 2, 2023

Thanks!
pay 50 usd

@yangwao
Copy link
Member

yangwao commented Nov 2, 2023

😍 Perfect, I’ve sent the payout
💵 $50 @ 4.58 USD/DOT ~ 10.917 $DOT
🧗 1xjvRADwdJcnmUCLWerEHR4iGCT5EBTm4D4fzLLg4LcAC9p
🔗 0x3a9f96740a41ae5e790748c4f0b85d17e381cf7562d08bb4aa24e3880fd72967

🪅 Let’s grab another issue and get rewarded!
🪄 github.com/kodadot/nft-gallery/issues

@yangwao yangwao added the paid pull-request has been paid label Nov 2, 2023
@yangwao
Copy link
Member

yangwao commented Nov 2, 2023

looking for improvement.
300ms cut seems fair, might improve the lighthouse?

@yangwao yangwao merged commit 6f7638f into main Nov 2, 2023
13 of 14 checks passed
@yangwao yangwao deleted the perf/stellate-graphql branch November 2, 2023 16:34
@preschian
Copy link
Member Author

might improve the lighthouse?

It is probably not significant since I only set the cache to 60 seconds
but if the LCP in some page rely on graphql request, we can test to set a longer cache on that

@vikiival
Copy link
Member

vikiival commented Nov 3, 2023

but if the LCP in some page rely on graphql request,

90% of landing 🥹

@preschian
Copy link
Member Author

90% of landing 🥹

aahh, I see. let me check

@preschian
Copy link
Member Author

After some checking, our FCP is currently a bit high. Currently, adjustment on LCP will not give significant results. We need to reduce the javascript size on the client

image

SSR or reducing unused JS in the pages could be helpful in this case, I guess

image

@roiLeo
Copy link
Contributor

roiLeo commented Nov 3, 2023

SSR or reducing unused JS in the pages could be helpful in this case, I guess

feel free to test on #7725

@preschian
Copy link
Member Author

SSR or reducing unused JS in the pages could be helpful in this case, I guess

feel free to test on #7725

checking 👍

@preschian
Copy link
Member Author

confirmed, with SSR from #7725 looks better

FCP from 9xx ms -> 6xx ms
Unused JS 3.2 MB -> 2.5 MB

this will help us to increase the scores

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Open to discussion paid pull-request has been paid small Pull request is small waiting-for-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants