Jump to content

Confused by caching.


artfulrobot
 Share

Recommended Posts

According to the docs at https://processwire.com/api/ref/wire-cache/get/ for $cache->get($key, $expiry), $expiry means:

Quote

 

Optionally specify max age (in seconds) OR oldest date string.

  • If cache exists and is older, then blank returned. You may omit this to divert to whatever expiration was specified at save() time. ...

 

I thought that meant:

  1. at 9am store something with an hour's expiry (so 10am) using $cache->save($k, $v, 60*60);
  2. At 9:01am, ask for it, if the value is younger than 30 mins, using $cache->get($k, 30*60);

But it doesn't. And it seems to me it can't do as it's documented to do because the date the thing was stored (and therefore how old it is) is not stored in the db table.

In fact the SQL that's run is: SELECT * FROM caches WHERE name=$k AND expires <= 9:31am

So even though our data is 1 minute old, it's not considered younger than 30 mins old and is not returned.

The only way the cache could honour the docs would be if the date the thing was stored in the db was saved. Then we could do whenSaved < 9:31am (though I'd suggest we also specify that it hasn't expired too, to honour the save() contract). But we don't have that field.

Just wanted to sanity check these findings with the community. And if I'm right, how do we go about suggesting clarifications for the docs?

If I'm just a confused noob, please explain the designed use-case for the documented get/expiry behaviour.

  • Like 1
Link to comment
Share on other sites

As far as I can tell, you're correct. Based on the way the documentation example is written, and how the methods process a numeric expire value in the get request, it doesn't:

get cache only if it’s less than or equal to 1 hour old (3600 seconds)

Instead, it...

get cache only if it expires within the next 1 hour (3600 seconds)

As for how best to report/request a change like this we might need to understand how the method was intended... In this case I'm not entirely sure if @ryan intended it to function the way the documentation is written, or if the code example in the documentation is incorrect...

  • Like 1
Link to comment
Share on other sites

If I'm reading this correctly, the confusion is all about what "age of cache" means, right?

Currently "age of cache" refers to the expiration time of the cache, not to the time it was stored. Meanwhile you expected it to mean the time the cache was stored. I've always assumed age to refer to the expiration time, but you are correct — it is not unreasonable at all to expect age to refer to the time of storage 🙂

In terms of documentation, something along the lines of "Optionally specify expiration time" and "If cache exists but has expired, then blank is returned" would perhaps be more in line with actual behaviour, but generally speaking the rule of thumb is that in this context "age" is about "expiration time" rather than "storage time".

To be clear I'm quite certain that the way the function works and the way it calculates expiration is exactly how it was meant to work — and even if it wasn't, changing it at this point would be a major breaking change in terms of behaviour for existing setups.

  • Like 5
Link to comment
Share on other sites

Thanks both.

I think the confusion arises because age is used to mean 2 different things in the cache context. In code, it's $expires in both contexts.

  1. For save() and also for get() when a function is also provided $expires means when the data should expire.
  2. For get() when a function is NOT provided $expires means "get if the thing expires before this new $expires, oh but also if it never expires (because of that 2010 date that's used to mean never expire...)".

The 2nd case is my beef!

  • Typically, when wanting a cached value, you care to fetch it unless it's too old for you to rely on. I can see the use case that you cache something with a long expiry, but that one user might only want to use a value if it's not that old.
  • But that's not what we have. We have sort of the opposite: get something if it's going to expire before the given date. I'd love it if someone could give a scenario in which this was useful. Perhaps the use-case is cache warming? Like you might want to get stuff that's going to expire soon, and update the cache now. We could then use get($name, $expiry) and then recalculate the value and save() with a new expiry? This is the only thing I can think of?
  • The fact that $expiry means 2 completely different things in the same get() api call is a trip hazard that needs flagging in the docs. I spent ages wondering why caching wasn't working and eventually stepped through to look at the SQL to get here.

 

 

  • Like 2
Link to comment
Share on other sites

For testing reasons, I added the following code after this line to make sure get() is never called in two-argument form.

			if($func === null)
				throw new WireExeption('Illegal use of $expiry argument in WireCache::get without a function argument');

So far, I haven't seen any exceptions, even when running maintenance. I'm going to run a lot of tests anyway over the next week as I'm in the process of finalizing my Redis WireCache module. Unless something unexpected pops up there, the docs should probably be amended to read:

/**
 ...
	 * @param int|string|null|false $expire Optionally specify max lifetime (in seconds) OR expiration time as a date string, or false to ignore.
	 * - Do not pass in $expire when calling get() without a $func argument, since the results are likely not what you expect
	 * - If using a $func, the behavior of $expire is the same as that of save().
...
*/

 

  • Like 2
  • Thanks 1
Link to comment
Share on other sites

@BitPoet

Are you doing this locally or are you preparing  a PR for the project? If so you're brave! I bet someone out there is relying on the existing behaviour. e.g. for cache warming or some other purpose for which my imagination was too limited.

There's specific code written to implement the existing behaviour, so that ought to be removed if you're going to throw an exception. Clearly someone, @ryan maybe, had something in mind, although I stand by my opinion that it's super confusing 😆

Link to comment
Share on other sites

29 minutes ago, artfulrobot said:

Are you doing this locally or are you preparing  a PR for the project?

Just locally, to make sure I'm not missing any cache queries I may not have expected, since I can't just map the database cache's logic to that of a key value store 1:1 while keeping its performance benefits.

Link to comment
Share on other sites

11 hours ago, teppo said:

If I'm reading this correctly, the confusion is all about what "age of cache" means, right?

My confusion was from the comment in the code example within the documentation that discusses the use of the $expire parameter using get() (the one with a value of 3600 being passed). But, overall, properly defining its meaning is far more beneficial overall. I tend to agree with @artfulrobot that there are times when knowing when a cache was created (ex: to clear an improperly stored value in a recently created cache) could be more useful than knowing when it expires, though both could potentially be useful in certain circumstances. It seems we only have the one option, as far as I can tell. I'm not well-versed in PW's caching (yet).

Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
 Share

  • Recently Browsing   0 members

    • No registered users viewing this page.
×
×
  • Create New...