-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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.
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
[YSQL] Set local limit in a multi page read #22821
Labels
2024.1 Backport Required
area/ysql
Yugabyte SQL (YSQL)
kind/bug
This issue is a bug
priority/medium
Medium priority issue
Comments
pao214
added
area/ysql
Yugabyte SQL (YSQL)
status/awaiting-triage
Issue awaiting triage
labels
Jun 11, 2024
yugabyte-ci
added
kind/bug
This issue is a bug
priority/medium
Medium priority issue
labels
Jun 11, 2024
9 tasks
1 task
pao214
added a commit
that referenced
this issue
Sep 4, 2024
Summary: ### Issue We set the read time explicitly in pg_op.cc for subsequent pages. This sets the read time on consistent read point. Consistent read point thinks this is a new read point and clears out all the local limit values stored. This will cause the local limit to advance, leading to read restart errors. ### Fix Clear out paging read time field before passing the response back to the pg layer from tserver's pg client session. Except for catalog sessions since they do not follow used_read_time logic. ### History 1c2e37d was introduced to use the same read time across all pages of a read. This was done as follows: docdb sets a read_time in the paging state and passes back to Pg. Pg would then copy paste the same paging state in the next RPC to docdb. Docdb would then pick the read time from the paging state to ensure that the same snapshot is used. b97a881 was a follow-up fix after 1c2e37d. After this fix, PG, sets the read time in the subsequent rpc requests using the read time from the paging state, instead of relying on docdb to use the read time from the copy pasted paging state. Currently, we do not need the paging state read time (sans upgrade reasons) for 1. Plain sessions have used_read_time logic that sets the read time of subsequent RPCs. 2. DDL sessions use distributed transactions from the start and the used_read_time for that is also handled. Catalog sessions require paging read time at the moment. We do not clear paging read time for catalog requests when sending back the response to pggate. **Upgrade/Rollback safety:** The used read time logic is present in local proxy. Since the local proxy is upgraded along with the Pg layer, no upgrade issues are expected. Jira: DB-11718 Test Plan: Jenkins ``` ./yb_build.sh --cxx-test pg_read_visibility-test --gtest-filter MultiPageScan ./yb_build.sh --cxx-test pgwrapper_pg_single_tserver-test --gtest_filter PgSingleTServerTest.TestPagingInSerializableIsolation ./yb_build.sh --gtest_filter PgLibPqTest.PagingReadRestart ``` Reviewers: pjain, sergei, dmitry Reviewed By: pjain, sergei, dmitry Subscribers: svc_phabricator, yql, ybase Differential Revision: https://phorge.dev.yugabyte.com/D35750
jasonyb
pushed a commit
that referenced
this issue
Sep 6, 2024
Summary: c587efd [docs] minor edit (#23796) 31e09f3 [PLAT-15029] yba installer split data and software directory setup f5ba17d [PLAT-15039]: Fix bootstrap on bi-directional xCluster config creation 578248a [#23770] YSQL: Deterministically populate catalog cache in tests with Connection Manager enabled 7b1f22a [#23799] test: Fixed PgTableSizeTest.SharedTableSize test for pg15 788434a [#18771, #21352] docdb: Fix LightweightMessage max size when parsing 02ced43 [#22821] YSQL: Preserve local limit in a multi-page read 50ff737 [#23741] docdb: Fix cloning of colocated databases with only parent table Excluded: 9889df7 [#23706] YSQL: Add table-level catcache Prometheus metrics c770d79 [#23747] MetaCache: Callback should not be called while holding the lock Excluded: 40689bc [#22150] YSQL, QueryDiagnostics: EXPLAIN (ANALYZE, DIST) support for queryDiagnostics 1655e69 [PLAT-15148]: Set XCluster Table Status to DroppedFromTarget if table in replication is dropped from target only 16262f7 [#22519] YSQL: Simplify API of the ExplicitRowLockBuffer class 6614afb [PLAT-14958][PLAT-14959] Make ssh fields optional if skipProvisioning is true 1153b56 [PLAT-14867] Make sure restart alerts don't trigger for small time updates during NTP sync bf1c7bc [PLAT-12226] Add connection pooling status to universe health check 38d8ae8 [PLAT-14805]Support adding EAR configs a180bef [#19134] YSQL, ASH: Setting ASH circular buffer size based on the number of cores 7d8fc76 Adjust heading link (#23807) 4c6cf5a [PLAT-4899]Basic validation of certificates f24eb10 [#23787] YSQL: Avoid executing conn mgr guc variables hooks for parallel workers ee18df8 [PLAT-13921] [K8] [UI] Universe action tasks are disabled after a failed shrink rr node task Excluded: dcf1821 [#23797] YSQL: Modify some tests to run in single connection mode with Connection Manager 0ac22cd [Docs] Remove Drift chat bot (#23802) 0e91003 [#22825] DocDB: Vector Index General Read Path with DummyANN e8f09b5 [PLAT-15175] Make runtime conf for skipping cluster consistency check public ee479ee Versionwarning (#23781) a05c6a3 [DB-12681] yugabyted-ui: Add Voyager commands to different Voyager phases in the UI. cc80d59 [#23777] yugabyted: updating the pg parity testcase to reflect the new gflags enabled for the pg parity feature. Test Plan: Jenkins: rebase: pg15-cherrypicks Reviewers: jason, tfoucher Subscribers: yql Differential Revision: https://phorge.dev.yugabyte.com/D37822
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
2024.1 Backport Required
area/ysql
Yugabyte SQL (YSQL)
kind/bug
This issue is a bug
priority/medium
Medium priority issue
Jira Link: DB-11718
Description
Issue
We lose the local limit in a multi-page read.
Issue Type
kind/bug
Warning: Please confirm that this issue does not contain any sensitive information
The text was updated successfully, but these errors were encountered: