Pulp

The urllib3 (as found in 1.15) PoolManager may already be doing a reasonable thing by comparing the cert/key paths so not convinced submitting an patch to urllib3 changing the HTTPSPoolKey to read the file and using the actual certificates when compared would be an improvement (but might be). Even if we did and they accepted, getting all the upstream patches on EL6/7 won't happen anytime soon. This would work for pulp3 assuming the PR for storing certificates on disk instead of DB and PR for download lib is merged because we would no longer be passing temporary file paths to python-requests.

A possible solution:

Create a SessionPool that caches sessions based on:

• scheme
• host
• port
• cert
• proxy settings

This seems cleaner and less brittle than patching things with an adapter subclass.

This would compensate for the current flaws in both python-requests and urllib3. And cert handling behavior in nectar and pulp. This would eliminate the need for the streamer/requests module which was an attempt to resolve this problem.

To address a separate but related problem, the pool could also store referenced certificates in /var/cache/streamer/certificates to ensure the sessions cached in the pool would not reference a deleted certificate. As would be the case with nectar writing and passing temporary files. Or, in pulp3 an importer being removed. The content of /var/cache/streamer/certificates would be cleared when by the SessionPool on streamer startup to prevent leaking certificate files.

Unused cached sessions should be purged after a specified about of time (expected cache behavior).

Even though, the question of python-requests and urllib3 being thread safe has not been settled, we could start by storing the cached sessions in a SessionPool instance attribute protected by an RLock. If thread safety becomes an issue or cannot be confirmed, this could be easily moved to a thread local. I do not think there would be any significant impact on performance if connections were not reused between threads. But we'd want to think though that. I have already confirmed that Twisted uses a thread pool.

This generally sounds good to me. Some quick thoughts:

We don't need anything fancy for evicting expired entries on a time period. We could just to an eviction sweep every time an item is being retrieved from the cache. If sessions sit around unused for a long time before they get noticed, that's no problem.

We identified some time ago when exploring this idea that it's ok for Session objects to simply fall out of scope and get garbage collected. We do that all the time already in other places, so there's no need to do anything to "close" or clean up a session. Just remove references to it.

I lean toward allowing Sessions to be shared by threads, since "requests" claims they are thread-safe. We can do further isolation if necessary later.

Applied in changeset pulp|0365f02192eed78693abb735460f96c7187914f5.