Skip to content

#fix:1733 mutiple /run call fails on DBsession using SQL fix #2973

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+8 −8
Open

#fix:1733 mutiple /run call fails on DBsession using SQL fix #2973

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
91502b2
mutiple /run call fails on DBsession using SQL fix
akshatshaw Sep 18, 2025
352607e
Merge branch 'google:main' into main
akshatshaw Sep 18, 2025
da69f86
Merge branch 'main' into main
hangfei Sep 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions src/google/adk/sessions/database_session_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -607,14 +607,14 @@ async def append_event(self, session: Session, event: Event) -> Event:
StorageSession, (session.app_name, session.user_id, session.id)
)

if storage_session.update_timestamp_tz > session.last_update_time:
raise ValueError(
"The last_update_time provided in the session object"
f" {datetime.fromtimestamp(session.last_update_time):'%Y-%m-%d %H:%M:%S'} is"
" earlier than the update_time in the storage_session"
f" {datetime.fromtimestamp(storage_session.update_timestamp_tz):'%Y-%m-%d %H:%M:%S'}."
" Please check if it is a stale session."
)
# if storage_session.update_timestamp_tz > session.last_update_time:
# raise ValueError(
# "The last_update_time provided in the session object"
# f" {datetime.fromtimestamp(session.last_update_time):'%Y-%m-%d %H:%M:%S'} is"
# " earlier than the update_time in the storage_session"
# f" {datetime.fromtimestamp(storage_session.update_timestamp_tz):'%Y-%m-%d %H:%M:%S'}."
# " Please check if it is a stale session."
# )
Comment on lines +610 to +617
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Commenting out this optimistic locking check resolves the immediate issue with concurrent requests failing, but it introduces a risk of race conditions and silent data loss. When two requests operate on the same session concurrently, the last one to write will overwrite any state changes made by the other. This could lead to inconsistent session state.

If this check is being removed intentionally, the commented-out code should be deleted rather than left in the source. Leaving dead code can be confusing for future maintenance. It would also be beneficial to add a comment explaining the rationale for removing the check and the accepted concurrency model.

If data consistency is critical, consider alternative concurrency control mechanisms, such as pessimistic locking (e.g., using with_for_update() on the query that fetches storage_session).


# Fetch states from storage
storage_app_state = sql_session.get(StorageAppState, (session.app_name))
Expand Down