Skip to content
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

Open
1 task done
pao214 opened this issue Jun 11, 2024 · 0 comments
Open
1 task done

[YSQL] Set local limit in a multi page read #22821

pao214 opened this issue Jun 11, 2024 · 0 comments
Assignees
Labels
2024.1 Backport Required area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug priority/medium Medium priority issue

Comments

@pao214
Copy link
Contributor

pao214 commented Jun 11, 2024

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

  • I confirm this issue does not contain any sensitive information.
@pao214 pao214 added area/ysql Yugabyte SQL (YSQL) status/awaiting-triage Issue awaiting triage labels Jun 11, 2024
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue labels Jun 11, 2024
@pao214 pao214 self-assigned this Jun 11, 2024
@yugabyte-ci yugabyte-ci removed the status/awaiting-triage Issue awaiting triage label Aug 26, 2024
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
Projects
Status: Backporting
Development

No branches or pull requests

2 participants