algolia / gatsby-plugin-algolia

A plugin to push to Algolia based on graphQl queries
https://yarn.pm/gatsby-plugin-algolia
Apache License 2.0
178 stars 45 forks source link

Partial updates #27

Closed u12206050 closed 4 years ago

u12206050 commented 5 years ago

Here it is. Works even better I believe than the hashCache version.

teamfphl commented 5 years ago

Looks great. Is there any ETA on when it could be merged? Thanks!

u12206050 commented 5 years ago

Suggestion changes implemented. Ready for merge.

Haroenv commented 5 years ago

@teamfphl I'd love for you to try out to verify it works in all kinds of installations, no need to wait until it's released

teamfphl commented 5 years ago

Tested and working :)

u12206050 commented 5 years ago

One month after a successful launch and daily building of the entire site I can confirm this works as it should, both updating only what has changed and removing entries that do not exist anymore.

Would be nice if it is merged into master.

Haroenv commented 5 years ago

Perfect, I'm putting it high on my todo list, thanks for your effort @u12206050

u12206050 commented 5 years ago

@Haroenv any reason why this isn't merged yet? I am starting on my third gatsby project with Algolia and I fined I still have to use my own version. Would you merge and publish asap? Otherwise I would have to publish my own version.

Haroenv commented 5 years ago

Because I have not had time to use the plugin in the last months, and don't want to merge something I haven't tested

fraserisland commented 5 years ago

Can also confirm i'm using this branch in production and have had no issues.

janosh commented 4 years ago

Very cool! Thanks @u12206050 for this contribution.

@Haroenv Seems like multiple users (1, 2, 3) confirmed it's working. Would be great if this was merged!

u12206050 commented 4 years ago

@janosh https://www.gatsbyjs.org/packages/gatsby-plugin-algolia-search/

ppcano commented 4 years ago

I received this message from algolia.

You are now incurring over-quota charges according to your plan rates:

To avoid over-quota charges, we recommend one or more of the following:

Reduce the number of indexing operations by syncing data less often and/or only re-indexing objects that need to be changed. Learn more about Indexing on Algolia Reduce the number of search operations Select another plan that better fits your needs

We have a very low number of search operations but we release (re-index) very frequently.

It looks this PR could help us fixing the over-quota issue by reducing the number of indexing operations as the first option suggests.

@Haroenv, It's a pity this PR hasn't been merged yet. The feature is really important to avoid algolia become expensive very quickly and the PR is relatively small.

Haroenv commented 4 years ago

The identified issues haven't been fixed, but I haven't had time myself yet, since I haven't used Gatsby in a while.

@u12206050 has published a fork which uses this behaviour though: https://yarnpkg.com/package/gatsby-plugin-algolia-search

ppcano commented 4 years ago

@Haroenv Thank you for the reply. I don't have a problem using that fork, but it gives more confidence to use the package maintained by the team behind the project.

Where do you think it is the best place for requesting the algolia team to work on this issue?

https://discourse.algolia.com/c/feedback

u12206050 commented 4 years ago

@ppcano We actively maintain the "fork", as we also maintain the same library for Gatsby and Gridsome. Read more: Gatsby plugin Algolia with partial updates

AnalogMemory commented 4 years ago

@u12206050

Tried your fork and get this error in console when building. Looks like the index.js in the example/src/layouts directory is getting picked up by gatsby

...
ERROR #85910  GRAPHQL

Multiple "root" queries found: "SiteTitleQuery" and "SiteTitleQuery".
Only the first ("SiteTitleQuery") will be registered.

Instead of:

   1 | query SiteTitleQuery {
   2 |   site {
   3 |     #...
   4 |   }
   5 | }
   6 |
   7 | query SiteTitleQuery {
   8 |   site {
   9 |     #...
  10 |   }
  11 | }

Do:

  1 | query siteTitleQueryAndSiteTitleQuery {
  2 |   site {
  3 |     #...
  4 |   }
  5 |   site {
  6 |     #...
  7 |   }
  8 | }

This can happen when you use two page/static queries in one file. Please combine those into one query.
If you're defining multiple components (each with a static query) in one file, you'll need to move each component to its own file.

File: node_modules/gatsby-plugin-algolia-search/example/src/layouts/index.js
...

"gatsby": "2.19.39"

Haroenv commented 4 years ago

I don't think that's related @AnalogMemory, the error message is talking about a query format in layout.js

AnalogMemory commented 4 years ago

@Haroenv It's a line in the examples folder, which is getting picked up by Gatsby. https://github.com/u12206050/gatsby-plugin-algolia/blob/d76e9c618912cab5164f085c070a905eab10a02f/example/src/layouts/index.js#L38

bloudermilk commented 4 years ago

It would be great to get this merged into the official package. Our index only has about ~1k records but we're blowing through our operations quota as our CI pipeline deploys our site many times per day.

It looks like most of the comments in the last PR review were questions or cosmetic suggestions. Maybe one other recent issue that @AnalogMemory reported?

If someone at Algolia can confirm that there is interest in merging I'm happy to pick up the torch.

janosh commented 4 years ago

If someone at Algolia can confirm that there is interest in merging I'm happy to pick up the torch.

That'd be great! There's certainly still interest from my side. 👍

ppcano commented 4 years ago

our CI pipeline deploys our site many times per day.

Same here.

I open an issue in discourse.algolia.com

AnalogMemory commented 4 years ago

@bloudermilk I'm using that fork in production and it really saved the day, operations dropped off immediately. The warning doesn't affect the build

Also this is Alex, back in the day when ya'll were at that spot in Venice 👋

Haroenv commented 4 years ago

There's interest in getting this merged, but the comments that are left now are to make sure this works in all use cases. I'd love to spend more time on this plugin, but unfortunately Gatsby hasn't been able to be my #1 priority due to some other big projects. I'll definitely review if someone else wants to build on top of this

u12206050 commented 4 years ago

I have updated my fork and released 0.5.7 without the example folder. If this is at all the case it shouldn't be a problem any more.

Algolia's api does allow for partial updating and that is what my fork makes use of. Note My fork is published on npm as gatsby-plugin-algolia-search so make sure you uninstall gatsby-plugin-algolia and then npm install --save gatsby-plugin-algolia-search

@AnalogMemory, @bloudermilk, @janosh, @ppcano

gburgett commented 4 years ago

Thanks @u12206050 ! Saved me a lot of work!

Haroen, if there's anything I can do to help push this along I'm happy to help out. We've just switched over to using this fork until it gets merged.

Haroenv commented 4 years ago

Thanks for suggesting @gburgett, The questions that still need to be answered are:

gburgett commented 4 years ago

Sure, happy to test locally with our workflow! We use a "digest" field on our algolia documents to detect if anything has changed. Our configuration looks like this:

gatsbyConfig.plugins.push({
    resolve: `gatsby-plugin-algolia-search`,
    options: {
      appId: process.env.GATSBY_ALGOLIA_APP_ID,
      apiKey: process.env.ALGOLIA_API_KEY,
      indexName: process.env.GATSBY_ALGOLIA_INDEX_NAME, // for all queries
      queries: algolia.queries,
      chunkSize: 10000, // default: 1000,
      enablePartialUpdates: true,
      matchFields: ['digest'],
    },
  })

Here's what happens when I run with partial updates on my ~1500 blog posts:

[Algolia Plugin] 1 queries to index - 0.013 ms
[Algolia Plugin] query 0: executing query - 0 ms
[Algolia Plugin] query 0: graphql resulted in 1468 records - 1.883 ms
[Algolia Plugin] query 0: starting Partial updates - 0 ms
[Algolia Plugin] query 0: found 1476 existing records - 0.502 ms
[Algolia Plugin] query 0: Partial updates – [insert/update: 70, total: 1468] - 0.001 ms
[Algolia Plugin] query 0: splitting in 1 jobs - 0.001 ms
[Algolia Plugin] deleting 8 object from papyrus_development_blogs index - 6.457 ms
[Algolia Plugin] Done in 14.739 s

This generates two batch updates

image image

If I set enablePartialUpdates: false and rebuild:

[Algolia Plugin] 1 queries to index - 0.011 ms
[Algolia Plugin] query 0: executing query - 0.384 ms
[Algolia Plugin] query 0: graphql resulted in 1468 records - 1.975 ms
[Algolia Plugin] query 0: splitting in 1 jobs - 0.001 ms
[Algolia Plugin] query 0: moving copied index to main index - 6.084 ms
[Algolia Plugin] Done in 9.159 s

And I see these lines in the Algolia logs:

image image

My index still has the correct 1,468 records

Haroenv commented 4 years ago

I'm sorry for being so slow on this pr, but this is now released on 0.8.0

janosh commented 4 years ago

@Haroenv No problem, just happy it got merged. 😄

bloudermilk commented 4 years ago

Huge thanks to everyone who stepped up to get this shipped.