Skip to content

fix: strip tzinfo from datetime for MySQL in create_session#5084

Open
andreikravchenko-oviva wants to merge 6 commits intogoogle:mainfrom
andreikravchenko-oviva:fix/mysql-stale-session-marker
Open

fix: strip tzinfo from datetime for MySQL in create_session#5084
andreikravchenko-oviva wants to merge 6 commits intogoogle:mainfrom
andreikravchenko-oviva:fix/mysql-stale-session-marker

Conversation

@andreikravchenko-oviva
Copy link
Copy Markdown

@andreikravchenko-oviva andreikravchenko-oviva commented Mar 31, 2026

Link to Issue or Description of Change

Fixes #5085

When using DatabaseSessionService with MySQL, the very first append_event() after create_session() always fails with:

ValueError: The session has been modified in storage since it was loaded.
Please reload the session before appending more events.

Root cause: create_session() produces a timezone-aware _storage_update_marker (with +00:00), but MySQL DATETIME(fsp=6) doesn't store timezone info (MySQL docs). When append_event() reads update_time back, it gets a naive datetime — different marker string, strict comparison fails.

The code already strips tzinfo for SQLite and PostgreSQL but MySQL was missed:

# Before (line 464):
if is_sqlite or is_postgresql:        # MySQL not included
    now = now.replace(tzinfo=None)

# After:
is_mysql = self.db_engine.dialect.name == _MYSQL_DIALECT
if is_sqlite or is_postgresql or is_mysql:
    now = now.replace(tzinfo=None)

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

Added mysql to the existing parametrized dialect test. Removed the test that incorrectly asserted MySQL preserves timezone.

Manual End-to-End (E2E) Tests:

Verified marker values with Testcontainers MySQL 8.0:

create_session marker get_session marker Match?
Before fix ...T14:39:46.014977+00:00 ...T14:39:46.014977 No
After fix ...T14:44:49.388273 ...T14:44:49.388273 Yes

Confirmed that append_event() works immediately after create_session() without needing a re-fetch workaround.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

@adk-bot adk-bot added the services [Component] This issue is related to runtime services, e.g. sessions, memory, artifacts, etc label Mar 31, 2026
@andreikravchenko-oviva andreikravchenko-oviva marked this pull request as draft March 31, 2026 14:57
@andreikravchenko-oviva andreikravchenko-oviva marked this pull request as ready for review March 31, 2026 15:04
@rohityan rohityan self-assigned this Apr 1, 2026
@andreikravchenko-oviva andreikravchenko-oviva force-pushed the fix/mysql-stale-session-marker branch from 1490a3b to 62623ba Compare April 1, 2026 08:57
MySQL DATETIME(fsp=6) does not store timezone information, same as
SQLite and PostgreSQL. However, create_session only stripped tzinfo
for SQLite and PostgreSQL, leaving it intact for MySQL.

This caused a stale-writer false positive on the first append_event
after create_session: the storage_update_marker from create_session
included "+00:00" (timezone-aware), but get_session reads back a
naive datetime from MySQL, producing a marker without "+00:00".
The strict string comparison in append_event then raised ValueError.

Add MySQL to the dialect check that strips tzinfo before storing.
@andreikravchenko-oviva andreikravchenko-oviva force-pushed the fix/mysql-stale-session-marker branch from 62623ba to 9de6c13 Compare April 2, 2026 13:37
@andreikravchenko-oviva
Copy link
Copy Markdown
Author

@rohityan Hi
Could you please take a look at this PR?
I saw that you ran /gemini review for the duplicate one - #5209

@rohityan rohityan added the needs review [Status] The PR/issue is awaiting review from the maintainer label Apr 13, 2026
@rohityan
Copy link
Copy Markdown
Collaborator

Hi @andreikravchenko-oviva ,Thank you for your contribution! We appreciate you taking the time to submit this pull request. Your PR has been received by the team and is currently under review. We will provide feedback as soon as we have an update to share.

@rohityan
Copy link
Copy Markdown
Collaborator

Hi @DeanChensj , can you please review this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs review [Status] The PR/issue is awaiting review from the maintainer services [Component] This issue is related to runtime services, e.g. sessions, memory, artifacts, etc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MySQL: stale-writer false positive on first append_event after create_session

3 participants