Launchpad itself

Merge lp:~stub/launchpad/bug-490239-session-tuning into lp:launchpad

  1. bug-490239-session-tuning
  2. Merge into devel
Proposed by Stuart Bishop
Status: Merged
Merge reported by: Stuart Bishop
Merged at revision: not available
Proposed branch: lp:~stub/launchpad/bug-490239-session-tuning
Merge into: lp:launchpad
Diff against target: 120 lines (+2/-80)
2 files modified
lib/canonical/launchpad/webapp/pgsession.py (+2/-19)
lib/canonical/launchpad/webapp/tests/test_pgsession.py (+0/-61)
To merge this branch: bzr merge lp:~stub/launchpad/bug-490239-session-tuning
Reviewer Review Type Date Requested Status
Guilherme Salgado (community) release-critical Approve
Graham Binns (community) code Approve
Review via email: mp+15403@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Stuart Bishop (stub) wrote :

Currently, our session machinery prunes old session information during a request. This is silly because it might take a while, and under high load, might contribute to a death spiral of the production systems and deletion requests time out and are retried by subsequent requests. Although this is still just a theory, removing this code still seems a good idea and batch processes to prune the old data are already in place.

Revision history for this message
Graham Binns (gmb) :
review: Approve (code)
Revision history for this message
Guilherme Salgado (salgado) :
review: Approve (release-critical)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/webapp/pgsession.py'
2--- lib/canonical/launchpad/webapp/pgsession.py 2009-06-25 05:30:52 +0000
3+++ lib/canonical/launchpad/webapp/pgsession.py 2009-11-30 09:55:24 +0000
4@@ -54,10 +54,11 @@
5 pickle bytea NOT NULL,
6 CONSTRAINT sessiondata_key UNIQUE (client_id, product_id, key)
7 );
8+
9+ Removing expired data needs to be done out of band.
10 """
11 implements(ISessionDataContainer)
12
13- timeout = 60 * DAYS
14 # If we have a low enough resolution, we can determine active users
15 # using the session data.
16 resolution = 9 * MINUTES
17@@ -67,7 +68,6 @@
18
19 def __getitem__(self, client_id):
20 """See zope.session.interfaces.ISessionDataContainer"""
21- self._sweep()
22 return PGSessionData(self, client_id)
23
24 def __setitem__(self, client_id, session_data):
25@@ -76,23 +76,6 @@
26 # themselves.
27 pass
28
29- _last_sweep = datetime.utcnow()
30- fuzz = 10 # Our sweeps may occur +- this many seconds to minimize races.
31-
32- def _sweep(self):
33- interval = timedelta(
34- seconds=self.resolution - self.fuzz + 2 * self.fuzz * random()
35- )
36- now = datetime.utcnow()
37- if self._last_sweep + interval > now:
38- return
39- self._last_sweep = now
40- query = """
41- DELETE FROM SessionData WHERE last_accessed
42- < CURRENT_TIMESTAMP AT TIME ZONE 'UTC' - '%d seconds'::interval
43- """ % self.timeout
44- self.store.execute(query, noresult=True)
45-
46
47 class PGSessionData(PGSessionBase):
48 implements(ISessionData)
49
50=== modified file 'lib/canonical/launchpad/webapp/tests/test_pgsession.py'
51--- lib/canonical/launchpad/webapp/tests/test_pgsession.py 2009-06-25 05:30:52 +0000
52+++ lib/canonical/launchpad/webapp/tests/test_pgsession.py 2009-11-30 09:55:24 +0000
53@@ -62,67 +62,6 @@
54 self.failUnless(isinstance(session_data, PGSessionData))
55 self.failUnless(ISessionData.providedBy(session_data))
56
57- def test_sweep(self):
58- product_id = 'Product Id'
59- client_id1 = 'Client Id #1'
60- client_id2 = 'Client Id #2'
61-
62- store = self.sdc.store
63-
64- # Create a session
65- session1 = self.sdc[client_id1]
66- session2 = self.sdc[client_id2]
67-
68- # Store some session data to ensure we can clean up sessions
69- # with data.
70- spd = self.sdc[client_id1][product_id]
71- spd['key'] = 'value'
72-
73- # Add and delete some data on the second client ID to ensure
74- # that it exists in the database.
75- session2._ensureClientId()
76-
77- # Do a quick sanity check. Nothing has been stored for the
78- # third client ID.
79- result = store.execute(
80- "SELECT client_id FROM SessionData ORDER BY client_id")
81- client_ids = [row[0] for row in result]
82- self.failUnlessEqual(client_ids, [client_id1, client_id2])
83- result = store.execute("SELECT COUNT(*) FROM SessionPkgData")
84- self.failUnlessEqual(result.get_one()[0], 1)
85-
86- # Push the session into the past. There is fuzzyness involved
87- # in when the sweeping actually happens (to minimize concurrency
88- # issues), so we just push it into the far past for testing.
89- store.execute("""
90- UPDATE SessionData
91- SET last_accessed = last_accessed - '1 year'::interval
92- WHERE client_id = ?
93- """, (client_id1,), noresult=True)
94-
95- # Make the SessionDataContainer think it hasn't swept in a while
96- self.sdc._last_sweep = self.sdc._last_sweep - timedelta(days=365)
97-
98- # Sweep happens automatically in __getitem__
99- ignored_result = self.sdc[client_id2][product_id]
100-
101- # So the client_id1 session should now have been removed.
102- result = store.execute(
103- "SELECT client_id FROM SessionData ORDER BY client_id")
104- client_ids = [row[0] for row in result]
105- self.failUnlessEqual(client_ids, [client_id2])
106-
107- # __getitem__ does not cause a sweep though if sweep has been
108- # done recently, to minimize database queries.
109- store.execute("""
110- UPDATE SessionData
111- SET last_accessed = last_accessed - '1 year'::interval
112- """, noresult=True)
113- session1._ensureClientId()
114- session1 = self.sdc[client_id1]
115- result = store.execute("SELECT COUNT(*) FROM SessionData")
116- self.failUnlessEqual(result.get_one()[0], 2)
117-
118 def test_storage(self):
119 client_id1 = 'Client Id #1'
120 client_id2 = 'Client Id #2'