Gervase Markham [:gerv] Reporter
	Description • 23 years ago

Following a discussion with Dave:

We need to think about GetVersionTable(), which gets called by most CGIs even if
they don't use all the info it provides. 

Currently, it uses the versioncache file - has it been proved that this is more
effective than some finer-grained request system which accesses the DB every
time for just the data needed?

Should the info returned be security-filtered by default? Currently we are doing
this filtering, to remove hidden products and components, in a lot of different
scripts, which opens up the possibility of bugs and differences. We could have a
param which said "ignore this user's security - give me the whole lot".

Gerv

	Dave Miller [:justdave]
	Comment 1 • 22 years ago

This bug hasn't been touched in over a year.  Given our current state of
rearrangement within Bugzilla, anyone have any additional thoughts on this?

	Bradley Baetz (:bbaetz)
	Comment 2 • 22 years ago

I've been putting this off. Its going to have to get done, though. I'm just
hoping to cludge it for show_bug, and fix stuff up properly later.

	Max Kanat-Alexander Assignee
	Comment 3 • 20 years ago

Well, the code does say "This needs to go away" over GenerateVersionTable (which
is the bulk of what GetVersionTable is doing). So let's change the summary. :-)

Wow, I don't even *understand* GenerateVersionTable. Could somebody either
explain it in full here, or add some doc comments to it?

	Gervase Markham [:gerv] Reporter
	Comment 4 • 20 years ago

We cache a load of rarely-changing stuff from the DB on disk in the file
data/versioncache. GenerateVersionTable() creates that file; it's run once an
hour, I believe.

Gerv

	Max Kanat-Alexander Assignee
	Comment 5 • 20 years ago

OK. So GetVersionTable isn't slow (because it just sets a cached variable). And
GenerateVersionTable, which might take a second or less, is called once an hour.

With MySQL being able to use only one index per query, I think that versioncache
is still a good solution.

Another option is to start using $dbh->prepare_cached inside of mod_perl, but
that's far away.

At the least, though, GetVersionTable should actually return the version table
as a local variable, instead of re-setting a global. :-)

What are actual problems with it that people have run into, when using it?

	Max Kanat-Alexander Assignee
	Comment 6 • 20 years ago

OK, I know how to do this.

We can replace this with a Bugzilla::DataCache object. It will be a singleton if
at all possible (which means "one instance globally throughout the entire
application").

	Max Kanat-Alexander Assignee
	Comment 7 • 20 years ago

It will work basically like Bugzilla::Config does now, where it imports the data
from a file.

Bugzilla::DataCache will EXPORT all of the data in versioncache.
Bugzilla::DataCache::flush() -- same as unlinking data/versioncache now.
Bugzilla::DataCache::_read_cache() will read-in the data.
Bugzilla::DataCache::_write_cache() will write-out the data.

I would put it in Bugzilla::Config, but it just doesn't seem to have the same
purpose as that module.

Eventually, we can refactor it to be accessed perhaps through
Bugzilla->datacache or something like that, and it could be more "lazy."

The *ideal* method of accomplishing what DataCache does would be with the
memcached daemon.

	Max Kanat-Alexander Assignee
	Comment 8 • 20 years ago

OK, I worked on this for several hours yesterday. I entirely re-factored
versioncache into a module called Bugzilla::DataCache. I moved all the variables
normally held in versioncache out of the global scope, and into the package.

Here's what I think:

(1) In most of the places where we use versioncache variables, it's pointless.
We're only using one or two variables.

(2) Do we have any evidence that at any time versioncache actually improved
performance enough that it's worth all the code mess it causes? Bonsai says that
GenerateVersionTable was written by terry, and it's part of the original checkin
for Bugzilla.

So, I say, "versioncache considered harmful." I think we should remove it, then
see if there are *actually* performance problems without it, and then fix those
performance problems where they arise.

Instead, we should do this:

  Move all usage of all current versioncache variables to instead call a
subroutine in some .pm file. For example, @legal_products would be a sub called
"legal_products" inside Product.pm (which doesn't exist yet) or BugClass.pm
(from bug 280122). These subs can do once-per-page caching, if they want, just
like Bugzilla::User::groups does, now.

	Frédéric Buclin
	Comment 9 • 19 years ago

The trunk is now frozen to prepare Bugzilla 2.22. Enhancement bugs are retargetted to 2.24.

	Max Kanat-Alexander Assignee
	Comment 10 • 19 years ago

justdave mentioned that for Zippy they made versioncache regenerate on every page hit and there was no noticable performance issue.

So we're just going to eliminate it. If there are any performance problems exposed, we can fix them in a more architecturally-correct way.

	Frédéric Buclin
	Comment 11 • 18 years ago

versioncache is dead!! YAY!