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

API data cache time #85

Closed
tremby opened this issue Jan 6, 2015 · 10 comments
Closed

API data cache time #85

tremby opened this issue Jan 6, 2015 · 10 comments

Comments

@tremby
Copy link
Contributor

tremby commented Jan 6, 2015

At https://github.com/prismicio/php-kit/blob/179a83363ef6c2fa120f5e2c90e5e24045eb776b/src/Prismic/Api.php#L330 the API data is cached for 5 units.

At https://github.com/prismicio/php-kit/blob/179a83363ef6c2fa120f5e2c90e5e24045eb776b/src/Prismic/Cache/CacheInterface.php#L43 the documentation for $ttl doesn't specify what the unit is.

At https://github.com/prismicio/php-kit/blob/179a83363ef6c2fa120f5e2c90e5e24045eb776b/src/Prismic/Cache/DefaultCache.php#L43 the $ttl variable is used as is and passed to apc_store, which interprets it as seconds.

  1. Please document in the interface that it means seconds, if that is indeed intended, or fix the logic if minutes were intended.
  2. Is caching the API data for as little as 5 seconds really intended? The request to get the API data is currently taking ~1 second for me on a test repository, and the thought of every five seconds every request until the value is recached taking 1 second longer than usual is unacceptable.
  3. Whether 5 seconds was intended or not, can you make that a default and have it configurable rather than hardcoded?

(edit: change links to point to source code as it was when ticket was originally opened)

@erwan
Copy link
Contributor

erwan commented Jan 6, 2015

Yes, it is seconds.

The API data is cached for 5 seconds only because a change in the writing room can happen at any moment, but we want to limit the number of calls as it would be useless to do several requests per seconds. Typically if you have several users accessing your site concurrently, they will all share the same cache so only one request every 5 seconds among all users will have the performance hit of the API call.

Now, 1 seconds sounds a bit long for the call to the API. We're usually below 200ms. Before making it configurable I'd rather make sure that the delay you're observing can not be improved.

Could you send a ticket to Intercom (from the Prismic UI) with your repository URL so we work that out? If the delay is caused by the latency, you can improve it on any production repository by activating the CDN. That should provide a low latency no matter where your server is located.

@erwan erwan closed this as completed Jan 6, 2015
@tremby
Copy link
Contributor Author

tremby commented Jan 6, 2015

I think you may misunderstand me. I want to increase the cache time, even make it indefinite for my particular application, since I can use a web hook to clear the cache when a change takes place in the writing room.

Point 1 above should still be addressed, for the sake of people like me writing cache implementations.
Point 2 -- fine.
Point 3 -- I'd still like to see this implemented. Users like me who do make use of web hooks to clear the cache would benefit from increasing the amount of time for which the API info is cached.

Typically if you have several users accessing your site concurrently, they will all share the same cache so only one request every 5 seconds among all users will have the performance hit of the API call.

Not true. If we have 10 requests per second and the API call takes 1 second, every 5 seconds when the cache expires all 10 of the requests in the next second will hit your API, each call taking 1 second. Only requests coming in after one of those API calls has finished (and so the cache is repopulated) will then skip the API call.

@tremby
Copy link
Contributor Author

tremby commented Jan 6, 2015

Could you send a ticket to Intercom (from the Prismic UI) with your repository URL so we work that out? If the delay is caused by the latency, you can improve it on any production repository by activating the CDN. That should provide a low latency no matter where your server is located.

I'm not sure that'll be necessary. Right now I'm testing on a clone of the pies repository, in open mode, without CDN switched on. The amount of latency isn't really the point of this, it's the fact that there's any latency at all -- I want the API cache to last a lot longer and eliminate it.

@tomjowitt
Copy link

The points made here by @tremby are exactly the same issues as we're facing. We've swapped out the APC cache and replaced it with a custom Redis backend and would like a lot more control over this without resorting to webhooks and lots of additional code.

@tremby
Copy link
Contributor Author

tremby commented Jun 14, 2015

I got tired of waiting a long while back and in my own implementation I actually just totally overrode the amount of cache time requested by the API calls. In other words, I ignore the $ttl value given in the CacheInterface#set call and decide for myself how long to cache.

@erwan
Copy link
Contributor

erwan commented Jun 15, 2015

tomjowitt: your problem is the 5 seconds TTL for the API object, or is there something else?

@erwan erwan reopened this Jun 15, 2015
@tomjowitt
Copy link

@erwan Yep, it'd be nice to have custom TTLs so our content is mostly fetched from Redis rather than expiring every 5 seconds.

@tremby
Copy link
Contributor Author

tremby commented Jun 15, 2015 via email

@erwan
Copy link
Contributor

erwan commented Jun 15, 2015

@tremby the other pages have a max-age corresponding to 1 year, which is basically the longest you can set until the browser consider the header as bogus and ignores it completely. Since the ref in included in the URL and the ref changes when any content changes, everything except the API object can basically be kept in cache forever.

I just updated the PHPdoc for the CacheInterface:
https://github.com/prismicio/php-kit/blob/master/src/Prismic/Cache/CacheInterface.php#L54

I'll probably make the "5 seconds" for the API object configurable, does an environment variable look like something that would make sense to PHP developers?

@tremby
Copy link
Contributor Author

tremby commented Jun 15, 2015

I just updated the PHPdoc for the CacheInterface

Thank you.

I'll probably make the "5 seconds" for the API object configurable, does an environment variable look like something that would make sense to PHP developers?

It wouldn't be unreasonable. I'd personally prefer a configuration hashmap passed to the API at initialization time, but understand that this would likely be a breaking change since other existing arguments configuring Prismic would be best put into the same hashmap. Or a method on the API object which can be called to change the configuration -- this wouldn't have to be breaking. Could alternatively do a combination of approaches, for example to offer an API method to configure, which would override the environment variable if present, but that might be overcomplicating it.

@erwan erwan closed this as completed Jun 16, 2015
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

No branches or pull requests

3 participants