2.7.1: using `get_post_meta()` re-triggers `pods_transient_set()` w/ 51kB storage payload on every page load

[lkraav]

(EDIT 2018-07-05 still the same with 2.7.6, PODS_API_CACHE false gets rid of them, but at what cost?)

I have an archive page where every entry accesses 3-4 pods fields via get_post_meta(). Every time I load this page, Query Monitor reports 56 transients set. It's like Pods keeps setting the transients for caching, but never manages to load the data from cache.

Here's an example Query Monitor traceback (all 50+ transient sets are exactly the same, other than the different transient name hash):

Transient name: pods_33bf92342012bb8c15c50bc755ee0c30

1. hybrid_get_content_template()
wp-content/themes/hybrid-core-4.0/inc/template.php:188
2. the_content()
wp-includes/post-template.php:240
3. apply_filters('the_content')
wp-includes/plugin.php:203
4. do_shortcode()
wp-includes/shortcodes.php:197
5. preg_replace_callback()
wp-includes/shortcodes.php:319
6. do_shortcode_tag()
wp-includes/shortcodes.php:319
7. ConversionXL\Institute\Plugin\Features\MyDashboard->ConversionXL\Institute\Plugin\Features\{closure}()
wp-content/plugins/conversionxl-institute/src/Plugin/Features/MyDashboard.php:67
8. ConversionXL\Institute\Plugin\Features\MyDashboard->render_dashboard_section_certificates()
wp-content/plugins/conversionxl-institute/src/Plugin/Features/MyDashboard.php:110
9. hybrid_get_content_template()
wp-content/themes/hybrid-core-4.0/inc/template.php:188
10. get_post_meta()
wp-includes/post.php:1778
11. get_metadata()
wp-includes/meta.php:485
12. apply_filters('get_post_metadata')
wp-includes/plugin.php:203
13. PodsMeta->get_post_meta()
wp-content/plugins/pods/classes/PodsMeta.php:2392
14. PodsMeta->get_meta()
wp-content/plugins/pods/classes/PodsMeta.php:2883
15. Pods->field()
wp-content/plugins/pods/classes/Pods.php:1217
16. PodsAPI->get_table_info()
wp-content/plugins/pods/classes/PodsAPI.php:8095
17. pods_transient_set()
wp-content/plugins/pods/includes/general.php:1548
18. pods_view_set()
wp-content/plugins/pods/includes/general.php:1441
19. PodsView::set()
wp-content/plugins/pods/classes/PodsView.php:333

What's going on here? Is the cache loading mechanism maybe building the transient name differently and missing existing entries + forcing re-caching on every page load?

[lkraav]

It's still happening w/ 2.7.6, which isn't really unexpected, since I don't think anyone is looking at it. But I am wondering about the logic of the problem - is this constant transient setting expected behavior and I'm misunderstanding something, or a bug or some object cache configuration side-effect?

https://github.com/tillkruss/redis-cache/ is our current object cache backend.

[quasel]

Sound's weird but that's maybe a question for @sc0ttkclark - did you try https://wordpress.org/plugins/pods-alternative-cache/ ? see FAQ for why and further Tips!

[lkraav]

@quasel it's indeed an architectural question, probably indeed @sc0ttkclark material but I know he's overloaded as is.

I'm not clear on why I would or would not have to add PAC to my stack. Default policy is always not to add anything to any stack without a specific reason.

[quasel]

Hi did you read the FAQ on the plugins page? To narrow it down I would start by deactivating any 3rd party caching plugin because (from the FAQ)

Hosts might have limits set on their object caching engine that are based on what they find optimal for their environment. Sometimes, plugins, themes, and even WordPress core can utilize object cache to the point where it gets too full. When that happens, certain caching engines like APC can remove objects from their cache and that can cause what appears to be random numbers of queries on each page load.

Does the number of created transients for pods always stays the same?

PODs API Caches pods configuration as it is (currently) stored in the DB - there are plans to allow code based configuration https://github.com/pods-framework/pods/issues?q=is%3Aopen+is%3Aissue+project%3Apods-framework%2Fpods%2F8

[lkraav]

Hosts might have limits set on their object caching engine that are based on what they find optimal for their environment.

My Redis cache has unlimited resources.

Something is definitely algorithmically off here (2.7.12). I'm going in w/ Xdebug.

[lkraav]

This if ( pods_api_cache() ) conditional to me is superweird https://github.com/pods-framework/pods/blob/2.x/classes/PodsAPI.php#L8680

        if ( pods_api_cache() ) {
            if ( ! did_action( 'init' ) ) {
                $transient .= '_pre_init';
            }
            pods_transient_set( $transient, $info );
        }

Even if we've successfully retrieve $_info = pods_transient_get( $transient ) value earlier in get_table_info() https://github.com/pods-framework/pods/blob/2.x/classes/PodsAPI.php#L8301

we then still unconditionally (does pods_api_cache() fail, ever?) proceed to re-set apparently this very same transient value. This happens for every Pods field get_post_meta() call, so that's how I'm getting tens and tens of unconditional transient sets on every Pods fields using page's load.

Shouldn't we track how we acquired $info, and skip pods_transient_set() if we got it via pods_transient_get() earlier?

Am I missing something here? cc @sc0ttkclark

[pglewis]

Even if we've successfully retrieve $_info = pods_transient_get( $transient ) value earlier in get_table_info() https://github.com/pods-framework/pods/blob/2.x/classes/PodsAPI.php#L8301

@lkraav: it feels a bit like duct tape but I think we could simply do a test against $_info before setting the transient? I would be open to better ideas if anyone has them but sussing out the intent of all the moving parts is always a fun game.

[lkraav]

I think we could simply do a test against $_info before setting the transient?

Yeah that's what I'm thinking, but what was the original intent behind coding it this way? To me, looks like logic got too complex, and it's an oversight.

Does this logic have test coverage? Not sure how to check.

[pglewis]

Yeah that's what I'm thinking, but what was the original intent behind coding it this way? To me, looks like logic got too complex, and it's an oversight.

Only The Scott knows.

[pglewis]

The duct tape fix adds to the long term problem and is how we arrive here in the first place, but absorbing what needs to be done for a proper refactor is likely to be resource intensive, including proper tests. I'm stuck picking my battles given the volume of other bugs on the pile, the time it takes to investigate these, and the pending push for 2.8. I'm not sure I could come up with a much better solution without spending days or weeks in there.

[lkraav]

Can you do a PR for me to run through staging and review?

Since we don't have beta/rc cycles, we could release it in the next minor, but revert if something unexpected actually is affected.

[pglewis]

Yeah, I'll see if I can get Scott's eyes on this... otherwise look for a PR link here soon

[pglewis]

Candidate fix: #5298

Let me know if you're able to test that @lkraav, I have not setup a local reproduction of the issue and am only doing eyeball debugging so far.