Skip to content

Characterize SQL cluster metadata pagination repeating its first page - #12017

Draft
taylan-oai wants to merge 1 commit into
temporalio:mainfrom
taylan-oai:dev/taylan/scope-cluster-refresh-pagination
Draft

taylan-oai wants to merge 1 commit into
temporalio:mainfrom
taylan-oai:dev/taylan/scope-cluster-refresh-pagination

Conversation

@taylan-oai

Copy link
Copy Markdown

Summary

Document and reproduce SQL cluster metadata pagination repeating its first page so a follow-up fix can restore forward progress without changing cluster lifecycle semantics.

Problem

The MySQL, PostgreSQL, and SQLite list queries select data, data_encoding, and version, but omit cluster_name. ListClusterMetadata builds the continuation token from the returned row's ClusterName, which remains empty. The serialized empty string is a nonempty token; the next request decodes it and chooses the initial query again.

With two unchanged clusters and page size 1, every page returns the first cluster. A client following these tokens cannot reach later clusters or finish its traversal. The background cluster cache refresher and startup metadata loader use page size 100, so a full first page can prevent their scans from completing while reads continue to succeed. Errors or context cancellation can interrupt the scan.

The existing shared persistence suite checks that both pages contain one row, but does not check their identities or progress. This SQL defect does not require concurrent writes or cluster rename.

Approach

Add a comment where the SQL store produces the cursor and a bounded native SQLite characterization. The test persists two distinct valid cluster records, verifies the second by point read, and demonstrates that the continuation token decodes to an empty cursor and returns the first cluster repeatedly. Its passing assertions document the current defect; a repair should replace them with forward-progress and termination assertions.

Two contained follow-up options are:

  1. Project the physical key in all three list queries. Include cluster_name in the list-specific SELECT projection so the current store can encode the last row's actual ordering key. This is the smaller option and preserves the existing token format and point-read behavior.
  2. Derive the cursor from the last persisted payload. Decode that row's cluster metadata before token construction and encode its nonempty cluster name. This preserves the current projections, but couples pagination to payload decoding and relies on the payload name matching the physical key. That invariant and malformed-payload behavior would need explicit validation.

Neither option is implemented or validated here.

Validation

  • Native in-memory SQLite characterization: passed.
  • Existing SQLite cluster metadata persistence suite plus characterization: passed, including all seven existing suite cases.
  • git diff --check: passed.
  • Native changed-package lint reported no issues.

MySQL and PostgreSQL share the source-level projection defect; they were not exercised against running databases. The test uses bounded page reads and does not start an indefinitely repeating loader or refresher.

Risks, rollout, and scope

This changes comments and tests only. The characterization intentionally asserts the current broken outcome and must change alongside a repair. Cluster mutation consistency, Cassandra pagination, cache eviction confirmation, and namespace rename are outside this candidate.

References

  • #2331 fixed token forwarding by the cluster refresher.
  • #5608 fixed token forwarding by a startup caller.
  • #12012 concerns namespaces renamed across a page cursor and does not cover this SQL projection defect.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants