Make WordPress Core

Opened 12 years ago

Closed 11 years ago

#22844 closed defect (bug) (wontfix)

random_seed transient is autoloaded with no reason

Reported by: mark-k's profile mark-k Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.4.2
Component: Security Keywords:
Focuses: performance Cc:

Description

As the only use is in password/secret generation which never happens on front end this should not be auto loaded.

IMO Either set an expiry time to avoid auto loading it, or make it an not auto loading option instead of transient.

Change History (8)

#1 @mark-k
12 years ago

  • Cc mark@… added

#2 @nacin
12 years ago

A non-expiring transient is indeed autoloaded, but I'm not sure it matters. We're talking a single option holding not a whole lot of data.

#3 @mark-k
12 years ago

@nacin, Sure, was thinking about marking it trivial, but it is a low risk change to add expiry, and it will bring some DB response and memory usage improvement to sites which don't use caching.

#4 follow-up: @nacin
12 years ago

some DB response and memory usage improvement

It would be good to see numbers on this. I would think this would be minimal. I doubt the query would be any faster (down to thousandths of a second) because it has to do a full table scan for all options anyway, and memory savings would be in the order of bytes.

I think we'd rather avoid storing something like this with an expiration. Semantically, it is not something that needs to expire after a period of time.

#5 in reply to: ↑ 4 @mark-k
12 years ago

Replying to nacin:

I think we'd rather avoid storing something like this with an expiration. Semantically, it is not something that needs to expire after a period of time.

But transients have an implicit expiration when object cache is used. AFAIK with APC object caching, the transient will expire every time appache is reset or when it is purged from memory due to low use. Therefor it makes more sense to have it as a not auto loading option.

And I agree about the minimal impact on performance my suggestion has, but then where do you draw the line? Taken to extreme it can be said on almost every option that its memory and speed impact is negligible, so why don't we auto load all the options table.

#6 @nacin
12 years ago

When transients were implemented in 2.8, the argument (as far as I know — I was not quite yet involved) was that a transient without an expiration was likely going to be fairly frequently needed, thus we should autoload it and avoid the query. Yes, it was transient data, but it's still data we need regularly until it decides to go away. Transients with expirations didn't really fit that mold.

Based on where to draw the line, should *any* transients be auto-loaded? The other transients that are autoloaded (having no expiration are) are doing_cron, update_core, update_plugins, and update_themes. These transients can be needed at pretty much any time, whenever WordPress is executing. Other transients, like API results (popular importers, browse happy, credits, themes feature list, plugins tag list), are only needed in limited, targeted situations. Which shoe fits random_seed?

It is worth noting that random_seed used to be an option, but was converted to a transient. Generally, options that became transients did not have an expiration, because they were "transient" but not necessarily something that should ever need to expire.

We've spoken about implicit expirations for DB transients too — #20316.

#7 @mark-k
12 years ago

<meta>
#20316 was one of the things that actually drove me into this subject. My main motivation is to have a plugin which will profile the use of options and recommend which ones should be autoloaded and which don't, based on the actual usage pattern for the specific site.

After reading the documentation and relevant code of the transients API I understand the motivation, but I don't think the result is consistent or easier to use then using options for the same purpose. For example, for better front end performance RSS feed should be fetched and cached in cron process where there is no advantage for using transients. Use of transiet API as it is for RSS fetching leads to slower front end from time to time when you use an RSS widget.

But back to the actual issue
</meta>

I think it is hard to say when an option/transient should be auto-loaded, but most of the time it is easy to say when it should not. Based on the usage pattern of random_seed it should not. My criteria would be "whatever transient/option is not used in a function called from the usage of public WP API intended to be used by themes should not be auto-loaded by default", and with this criteria non of the site transients you mentioned should auto-load.

I'm not a native english speaker but it seems to me that by the definition of the word in english http://www.merriam-webster.com/dictionary/transient it should not be used for objects which are expected to live indefinitely ;)

#8 @nacin
11 years ago

  • Component changed from Performance to Security
  • Focuses performance added
  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

Closing as I think random_seed should remain autoloaded.

Note: See TracTickets for help on using tickets.