Skip to content

Commit 1ebb8e6

Browse files
committed
bug: fix undefined symbol in exception; limit session update time
Managed to get Redis session update pipeline to fail an update requiring retry. So exception code redis.Exceptions... got executed, except its supposed to be redis.exceptions. Improved error reporting in case where session update fails after three retries. updateTimestamp now only updates if old session timestamp was updated more the 60 seconds ago. Same as other backends. This should reduce write load on the backend server. Also spelling fixes and clarification in comments, flake8 fixes.
1 parent d2d707f commit 1ebb8e6

File tree

1 file changed

+26
-16
lines changed

1 file changed

+26
-16
lines changed

roundup/backends/sessions_redis.py

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818
"""
1919
__docformat__ = 'restructuredtext'
2020

21-
import marshal, redis, time
21+
import marshal
22+
import redis
23+
import time
2224

2325
from roundup.anypy.html import html_escape as escape
2426

@@ -177,22 +179,31 @@ def set(self, infoid, **newvalues):
177179
try:
178180
# assume this works or raises an WatchError
179181
# exception indicating I need to retry.
180-
# Since this is not a transaction, an error
182+
# Since this is not a real transaction, an error
181183
# in one step doesn't roll back other changes.
182184
# so I again ignore the return codes as it is not
183185
# clear that I can do the rollback myself.
184186
# Format and other errors (e.g. expireat('d', 'd'))
185-
# raise exceptions tht bubble up and result in mail
187+
# raise exceptions that bubble up and result in mail
186188
# to admin.
187189
transaction.execute()
188190
break
189-
except redis.Exceptions.WatchError:
191+
except redis.exceptions.WatchError:
190192
self.log_info(
191193
_('Key %(key)s changed in %(name)s db' %
192194
{"key": escape(infoid), "name": self.name})
193195
)
194196
else:
195-
raise Exception(_("Redis set failed afer 3 retries"))
197+
try:
198+
username = values['user']
199+
except KeyError:
200+
username = "Not Set"
201+
202+
raise Exception(
203+
_("Redis set failed after %(retries)d retries for "
204+
"user %(user)s with key %(key)s") % {
205+
"key": escape(infoid), "retries": _retry,
206+
"user": username})
196207

197208
def list(self):
198209
return list(self.redis.keys(self.makekey('*')))
@@ -214,18 +225,17 @@ def lifetime(self, key_lifetime=None):
214225
return time.time() + (key_lifetime or self.default_lifetime)
215226

216227
def updateTimestamp(self, sessid):
217-
''' Other backends update only if timestamp would change by more
218-
than 60 seconds. To do this in redis requires:
219-
get data _timestamp
220-
calculate if update needed
221-
if needed,
222-
set new timestamp
223-
why bother. Just set and forget.
228+
''' Even Redis can be overwhelmed by multiple updates, so
229+
only update if old session key is > 60 seconds old.
224230
'''
225-
# no need to do timestamp calculations
226-
lifetime = self.lifetime()
227-
# note set also updates the expireat on the key in redis
228-
self.set(sessid, __timestamp=lifetime)
231+
sess = self.get(sessid, '__timestamp', None)
232+
now = time.time()
233+
# unlike other session backends, __timestamp is not set to now
234+
# but now + lifetime.
235+
if sess is None or now > sess + 60 - self.default_lifetime:
236+
lifetime = self.lifetime()
237+
# note set also updates the expireat on the key in redis
238+
self.set(sessid, __timestamp=lifetime)
229239

230240
def clean(self):
231241
''' redis handles key expiration, so nothing to do here.

0 commit comments

Comments
 (0)