--- Comment #3 from Neil Kandalgaonkar <> 2010-12-07 
20:01:50 UTC ---
Okay, there's a number of things here...

Somewhat irrelevant to the main thrust here -- making the "local" cache of the
file in UploadStash->files upon storage, rather than a memoized retrieval --
this seems wrong to me as you only get a chance to do this in the same process
as the one that created the record. Any subsequent access is slow. If the
argument is that the caches are fast anyway then the whole optimization of
having UploadStash->files should be removed.

Okay, the main thing you did is to:

- make entries directly in cache. I verified that there isn't any case in
MediaWiki where we have absolutely no cache available, so that seems okay.

- but now we're relying on an unspecified number of forms of cache, that will
change in the future, to do intraserver communication. Unlike $_SESSION which
has to do that even for a website to work, other forms of caching may not. This
will happen to work in the MediaWiki cluster since we'll get memcache, and on a
typical standalone MediaWiki you'll get a *DBM, so maybe it's okay, but it
bothers me a little.

- Since you give each upload their own key, they are truly isolated so that
probably will fix the concurrency issue. This makes all the uploads truly
isolated, which is nice, but also breaks with the convention of storing all of
them under a particular key (this is what everyone else does, such as the
UploadByUrl, Firefogg uploads) and it also makes it difficult to find all the
uploads later if we wanted to. There is no case in the application where we
have to do this, but I anticipate we will want to do this later.

You might have a really simple solution here but I want to think about it a bit

Some other ideas to fix it:

- It might be as simple as eliminating the line that creates an array in
$_SESSION[UploadBase::SESSION_NAME]. This is PHP, multi-dimensional hashes
spring into existence on assignment. Depending on how this is implemented maybe
that solves it.

- Figuring out a locking system to obtain control over
$_SESSION[UploadBase::SESSION_KEYNAME]. Of course that means we are starting to
implement a database. ;(

- Actually use the database -- least desirable from a standpoint of doing a
quick fix, but probably the best long term solution

Configure bugmail:
------- You are receiving this mail because: -------
You are on the CC list for the bug.

Wikibugs-l mailing list

Reply via email to