Skip to content

find_shareable documents a FOR SHARE guarantee that the staging path does not get #1493

Open

Description

@KonstantinMirin

Problem

The docstring of ReportingRowStorageV1.find_shareable says it takes FOR SHARE on the row set "so retention cannot retire it before the caller's transaction commits". It has two callers. persist_revision_row_set calls it inside the ledger transaction for postgres bindings, where the lock does last until commit. stage_revision_rows calls it on a pooled connection with no enclosing transaction, so the row lock ends when that connection context exits, long before the commit. The commit path re-checks liveness with its own FOR SHARE, so there is no data loss. The documented guarantee is false for one of the two callers, and the re-check, which is the real guarantee, is not mentioned.

Evidence

All references are to main at 47084e83a.

  • src/adcp/reporting/ledger/row_storage.py:973-974, the docstring:

    Takes FOR SHARE on the row set so retention cannot retire it before the caller's transaction commits.

  • row_storage.py:983: " ORDER BY recorded_at DESC, row_set_id LIMIT 1 FOR SHARE".

  • src/adcp/reporting/ledger/row_write.py:196-197, in stage_revision_rows:

    async with store._pool.connection() as connection:
        shared = await storage.find_shareable(connection, prepared)

    No connection.transaction() encloses the call; psycopg ends the implicit transaction when the with block exits.

  • row_write.py:316-320: persist_revision_row_set calls find_shareable on the ledger transaction's connection for postgres bindings.

  • row_write.py:327-339: the commit re-check for object bindings, SELECT 1 FROM adcp_reporting_row_sets WHERE row_set_id = %s AND rows_state = 'live' FOR SHARE, with its own comment "FOR SHARE keeps retention from retiring it before the transaction ends."

Suggested fix

Either drop FOR SHARE from find_shareable and document that the commit re-check is the guarantee for both callers, or keep it and make the docstring precise: the lock lasts as long as the caller's transaction, stage_revision_rows calls it outside one, and persist_revision_row_set re-checks liveness at commit for that case.

Introduced in #1467.

Activity

  1. added
    documentationImprovements or additions to documentation
    pythonPull requests that update python code
    on Oct 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    documentationImprovements or additions to documentationpythonPull requests that update python code

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions