Cache is not safe for concurrent use (by processes or threads)

[James Westby]

[Impact]
Concurrency issues in lazr.restfulclient are affecting some processes used by various teams; including Foundations, PS integration, etc.
The most affected users are those who make heavy use of scheduled tasks around launchpadlib (see bug 513116); where different calls to various parts of launchpadlib (or the bzr launchpad plugin) will concurrently try to access cache, step on each others' cache files and mangle the files causing errors for further calls using lazr.restfulclient / launchpadlib.

[Test case]
1) Update package.
2) Run the following command:
for i in `seq 1 50` ; do python -c 'from launchpadlib.launchpad import Launchpad; lp = Launchpad.login_anonymously("test-cache", "production"); lp.distributions["ubuntu"]' & ; done

After a few runs, on an affected system the spawned processes will write out backtraces due to mangled files, such as the following error:
cElementTree.ParseError: not well-formed (invalid token): line 40599, column 18

On a patched system, all spawned processes will terminate successfully.

[Regression Potential]
Due to the changes in caching method, it may be possible that lazr.restfulclient fails to write cache files because of path size (which would likely increase due to the change), or otherwise fails to properly cache data increasing delays in retrieving information that may have already been retrieved.

----

httplib2 says:

class FileCache(object):
    """Uses a local directory as a store for cached files.
    Not really safe to use if multiple threads or processes are going to.
    be running on the same cache.
    """

launchpadlib sub-classes that, but doesn't change that fact.

This means that with multiple processes or threads you will
suddenly get strange error messages (bug 404204).

This is made worse by the fact that creating an object uses a
global cache, and login_with does this with no way to override it.

The best way to fix this is to just fix the cache implementation
rather than everyone learning to use a private cache and
fixing launchpadlib to make this possible with the convenience
methods.

This is a pre-requisite for fixing bug 513116, which is about the fact that launchpadlib is not thread-safe.

[Martin Pool (mbp)]

Does the cache actually save significant time at the moment? When I last looked, it would never avoid round trips.

[Leonard Richardson (leonardr)]

We only serve the Cache-Control header for the service root (WADL and JSON), so those are the only resources for which we currently avoid round trips. But here are three reasons to like the cache anyway:

1. Those service root representations are pretty big fish to have caught. They're requested every time a Launchpad instance is created, and the WADL representation in particular is rather large. Taking launchpadlib down to one request per week for these resources was a big performance win.

2. The cache is where we store the ETags that let us make conditional GET and PATCH requests, even for resources who don't have Cache-Control headers.

3. At any time we can decide to be less conservative, and start sending Cache-Control for other documents. Then the client-side cache will be more useful. We just haven't made that decision yet.

[Colin Watson (cjwatson)]

Is there any chance this could be released at some point? We keep running into this on precise-based infrastructure, so it'd be great to have this fix in quantal and SRUed into precise.

[Mathieu Trudel-Lapierre (cyphermox)]

Let's SRU this to Quantal and Precise; they're both 0.12.0, so just applying the particular diff should be good.

[Mathieu Trudel-Lapierre (cyphermox)]

Uploaded SRU package to quantal-proposed...

[Dave Walker (davewalker)]

Hello James, or anyone else affected,

Accepted lazr.restfulclient into quantal-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/lazr.restfulclient/0.12.0-2ubuntu0.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Steve Langasek (vorlon)]

Hello James, or anyone else affected,

Accepted lazr.restfulclient into precise-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/lazr.restfulclient/0.12.0-1ubuntu1.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Ubuntu Foundations Team Bug Bot (crichton)]

The fix for this bug has been awaiting testing feedback in the -proposed repository for precise for more than 90 days. Please test this fix and update the bug appropriately with the results. In the event that the fix for this bug is still not verified 15 days from now, the package will be removed from the -proposed repository.

[Brian Murray (brian-murray)]

I tried creating this bug on Precise using the test case provided and was unsuccessful.

[Brian Murray (brian-murray)]

I was able to recreate this on Quantal, and verify the fix, but I had to create a bash script with the test case and run it 2 or 3 times in a row.

[Brian Murray (brian-murray)]

I also got the test case working and the verification done in Precise.

[Launchpad Janitor (janitor)]

This bug was fixed in the package lazr.restfulclient - 0.12.0-1ubuntu1.1

---------------
lazr.restfulclient (0.12.0-1ubuntu1.1) precise-proposed; urgency=low

  * debian/patches/concurrency_fixes_rev_122.patch: backport rev 122: Creates
    AtomicFileCache as a parent class for MultipleRepresentationCache to
    handle concurrent use issues. (LP: #459418)
 -- Mathieu Trudel-Lapierre <email address hidden> Fri, 07 Jun 2013 08:48:59 -0400

[Scott Kitterman (kitterman)]

The verification of this Stable Release Update has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regresssions.

[Launchpad Janitor (janitor)]

This bug was fixed in the package lazr.restfulclient - 0.12.0-2ubuntu0.1

---------------
lazr.restfulclient (0.12.0-2ubuntu0.1) quantal-proposed; urgency=low

  * debian/patches/concurrency_fixes_rev_122.patch: backport rev 122: Creates
    AtomicFileCache as a parent class for MultipleRepresentationCache to
    handle concurrent use issues. (LP: #459418)
 -- Mathieu Trudel-Lapierre <email address hidden> Fri, 22 Feb 2013 15:01:50 +0000