BoldGrid / w3-total-cache

GNU General Public License v2.0
152 stars 85 forks source link

W3-Total-Cache fragment caching incorrectly disregards transients' expirations #40

Closed archon810 closed 4 months ago

archon810 commented 5 years ago

I'm debugging some slowdowns of the AMP validator with the AMP team right now, and right now we've arrived at this weird behavior coming from W3TC, which seems to have been in place (and possibly broken as a result) for years https://wordpress.stackexchange.com/questions/108287/trouble-with-transient-api-when-w3tc-is-activated.

If we don't want to use object caching, we disable it in W3TC settings. However, even when it's disabled, W3TC insists on copying object-cache.php to wp-content. Once that file is in place, $_wp_using_ext_object_cache is set to true by WordPress, and transients no longer work.

Why does W3TC insist on having object-cache.php with object caching disabled in its settings? Is this a bug?

archon810 commented 5 years ago

I've debugged this further, and it looks like what's happening here is for some reason Fragment Caching (which is enabled) kicks in instead of object caching, even though object caching is disabled, and requires object-cache.php.

That'd be understandable if it only used object caching for caching fragments (i.e. stuff surrounded by <!-- mfunc mycode -->, but it actually takes over object caching entirely and caches all calls to set_transient(), get_transient(), etc.

Is this a bug or an expected feature of fragment caching for some reason? Because with object caching disabled, one would expect the transients to go to the db for persistent storage (as opposed to memcached which is what fragment caching is set to in our setup).

archon810 commented 5 years ago

This ticket is evolving, and I now know exactly where the buggy behavior we're trying to fix is: Fragment Caching.

The gist is: given that fragment caching takes over caching all trasients as discussed above, no matter if you have object caching on or off, it has a nasty surprise: it disregards set_transient's own expiration and forces the expiration value on all transients to be what "Default lifetime of cached fragments" is set to in the settings.

So if you set a transient to expire in a day, but "Default lifetime of cached fragments" is set to 3 minutes, all transients will expire in 3 minutes.

This is bad!

I also tested enabling object caching and fragment caching together. Turns out, fragment caching takes priority and continues taking over transients, exhibiting the same buggy behavior.

The only way to make transients work properly and not disappear is by using object cache without fragment caching - then transients' own expirations are honored, no matter what "Default lifetime of cache objects" is set to.

Is this enough to reproduce and fix this issue?

Thank you.

maxicus commented 5 years ago

Fragment Cache is W3TC PRO functionality, while this repository is about community plugin version. Please use PRO support.

archon810 commented 5 years ago

The community version has the code for fragment caching inside. I'm currently testing Pro and enabled it temporarily.

I've spent hours on debugging, and presented a serious bug to you, why would you just throw it away and ignore it, especially if fixing it would improve the paid version? Are you saying I need to pay W3EDGE just to report a bug in the Pro version?

gidomanders commented 5 years ago

Hi @archon810,

As Max said, this repository is for the community edition of the plugin. For Pro we have another system to track bugs, so we would like to handle this request in there. We're not dismissing the topic, it's just not relevant to users of the free version of the plugin ;-)

archon810 commented 5 years ago

@gidomanders Alright, but I'm not a Pro user, so can we just discuss it here, or where should we move the discussion to?

BTW I just discovered that Fragment Caching and Page Fragment Caching are different things based on your FAQ, and for dynamic code chunks, the Fragment Caching extension isn't even needed, so I can just disable it. The naming and requirements are quite confusing and ambiguous, especially since the extension mentions "page" in its description.

bwmarkle commented 5 years ago

Hi @archon810, I just wanted to say thanks for posting this issue. I really appreciate that you've taken the time to do so. We're having a bit of internal discussion on this matter. I don't have any answers for you this moment, but we're reviewing things and figuring out a course of action.

archon810 commented 5 years ago

Great, thanks for the update. Do you want to reopen this ticket in the meantime?

bwmarkle commented 5 years ago

Hi @archon810 After some discussion, our plan is to focus one of our next releases on improved purging (based on several of the GitHub issues you've created). It sounds like the process does have some hurdles as well as a few different implementation options we need to consider.

I just want to make sure that you know your feedback isn't falling on deaf ears :) It's triggering quite some discussion within the team, and I'm excited to see what we come up with for a future release.

I can't say we'll have something for you in just a few days, but we'll keep you updated. It'll take a little bit of time to figure out the best solution and get it all coded out.

bwmarkle commented 5 years ago

Hi @archon810,

We did some testing on this end, and it doesn't appear that W3TC is actually affecting the transients in the way you're describing. We could have missed something, but right now we're thinking it may be something else interfering.

One of the new things we're working on now to help troubleshoot issues like this is a new Cache Purge Stack Trace. I'm not sure if that's what we'll call the new feature, but basically what we'd like to do is update the feedback provided in debug mode to show basically the stack trace of what caused the last cache purge. If it was caused by W3TC we would be able to see that, or if it was caused by 3rd party code, we'd be able to see that too.

I'll keep you updated. If you're willing to test this new feature when we release it soon, that would be very cool.

Thanks,

bwmarkle commented 4 years ago

Hi @archon810,

I just wanted to let you know that the new Purge Cache Log feature has been released. If you wanted to learn more, here is an article about it: https://www.boldgrid.com/support/w3-total-cache/purge-cache-log/

Thanks, - Brad

archon810 commented 4 years ago

Hi @bwmarkle,

This is a great tool, awesome addition. However, based on this comment https://github.com/W3EDGE/w3-total-cache/issues/40#issuecomment-530742709 and my discovery that Fragment Cache (the W3TC extension) and Page Fragment Caching functionality are different things, and I only need Page Fragment Caching, I don't believe this is an issue we'll hit anymore, unless we re-enable the Fragment Cache extension for some reason.

So I'm sorry, but I don't think I can dedicate the time to debug a case that we won't run into anymore, but I hope you end up reproducing it with Fragment Cache W3TC extension enabled.

cssjoe commented 4 months ago

This issue appears to be outdated. We have implemented changes to the Fragment Cache feature. As for the transient expiration and storage in the database, expiration is set using the lifetime setting and not what is specified when setting each transient. The database is used as well only if the setting is enabled. Please let us know if there is still an issue.