Skip to content

Commit

Permalink
perf: Don't update session in cache after every request (#26308) (#26362
Browse files Browse the repository at this point in the history
)

It's just reading and writing same information except last_update inside
data which is never read back from this.

(cherry picked from commit 7fe9cc4)

Co-authored-by: Ankush Menat <ankush@frappe.io>
  • Loading branch information
mergify[bot] and ankush committed May 9, 2024
1 parent 2fdd0a0 commit c0ba12c
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 9 deletions.
1 change: 0 additions & 1 deletion frappe/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,6 @@ def sync_database(rollback: bool) -> bool:
# update session
if session := getattr(frappe.local, "session_obj", None):
if session.update():
frappe.db.commit()
rollback = False

return rollback
Expand Down
2 changes: 1 addition & 1 deletion frappe/desk/desktop.py
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ def get_workspace_sidebar_items():
has_access = "Workspace Manager" in frappe.get_roles()

# don't get domain restricted pages
blocked_modules = frappe.get_doc("User", frappe.session.user).get_blocked_modules()
blocked_modules = frappe.get_cached_doc("User", frappe.session.user).get_blocked_modules()
blocked_modules.append("Dummy Module")

# adding None to allowed_domains to include pages without domain restriction
Expand Down
15 changes: 8 additions & 7 deletions frappe/sessions.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ def generate_csrf_token():


class Session:
__slots__ = ("user", "user_type", "full_name", "data", "time_diff", "sid")
__slots__ = ("user", "user_type", "full_name", "data", "time_diff", "sid", "_update_in_cache")

def __init__(self, user, resume=False, full_name=None, user_type=None):
self.sid = cstr(frappe.form_dict.get("sid") or unquote(frappe.request.cookies.get("sid", "Guest")))
Expand All @@ -203,6 +203,7 @@ def __init__(self, user, resume=False, full_name=None, user_type=None):
self.full_name = full_name
self.data = frappe._dict({"data": frappe._dict({})})
self.time_diff = None
self._update_in_cache = False

# set local session
frappe.local.session = self.data
Expand Down Expand Up @@ -313,6 +314,7 @@ def get_session_data(self):

data = self.get_session_data_from_cache()
if not data:
self._update_in_cache = True
data = self.get_session_data_from_db()
return data

Expand Down Expand Up @@ -363,23 +365,23 @@ def start_as_guest(self):

def update(self, force=False):
"""extend session expiry"""

if frappe.session.user == "Guest":
return

now = frappe.utils.now()

Sessions = frappe.qb.DocType("Sessions")

self.data["data"]["last_updated"] = now
self.data["data"]["lang"] = str(frappe.lang)

# update session in db
last_updated = frappe.cache.hget("last_db_session_update", self.sid)
time_diff = frappe.utils.time_diff_in_seconds(now, last_updated) if last_updated else None

# database persistence is secondary, don't update it too often
updated_in_db = False
if (force or (time_diff is None) or (time_diff > 600)) and not frappe.flags.read_only:
self.data.data.last_updated = now
self.data.data.lang = str(frappe.lang)
# update sessions table
(
frappe.qb.update(Sessions)
Expand All @@ -391,11 +393,10 @@ def update(self, force=False):
frappe.db.set_value("User", frappe.session.user, "last_active", now, update_modified=False)

frappe.db.commit()
frappe.cache.hset("last_db_session_update", self.sid, now)

updated_in_db = True

frappe.cache.hset("session", self.sid, self.data)
frappe.cache.hset("last_db_session_update", self.sid, now)
frappe.cache.hset("session", self.sid, self.data)

return updated_in_db

Expand Down

0 comments on commit c0ba12c

Please sign in to comment.