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] Redundant read for create table with primary key #8024

Closed
m-iancu opened this issue Apr 13, 2021 · 2 comments
Closed

[YSQL] Redundant read for create table with primary key #8024

m-iancu opened this issue Apr 13, 2021 · 2 comments
Assignees
Labels
area/ysql Yugabyte SQL (YSQL)
Projects
Milestone

Comments

@m-iancu
Copy link
Contributor

m-iancu commented Apr 13, 2021

When creating a table with a primary key it looks like we issue a redundant read from the table itself.
This can be noticed by using the debug flag:

set yb_debug_log_docdb_requests = true;

and comparing the requests between a table without a pkey and one with a pkey.
For the latter, we see read requests to the main table (e.g. grep the postgresql log by the created table's id).
This may be related to an internal index backfill attempt, but for us we shouldn't need this for a primary key because the table is already primary key organized (so the pkey and main table point to the same data already).
We should confirm if this additional read is indeed not needed and, if so, remove it.

cc @ramsrivatsa @emhna @jaki

@m-iancu m-iancu added the area/ysql Yugabyte SQL (YSQL) label Apr 13, 2021
@m-iancu m-iancu added this to the 2.7.x milestone Apr 13, 2021
@m-iancu m-iancu added this to Backlog in YSQL via automation Apr 13, 2021
@yifanguan
Copy link
Contributor

Stacktrace:

    @        0x10f02ac13  yb::pggate::PgDocOp::SendRequestImpl()
    @        0x10f029fdf  yb::pggate::PgDocOp::SendRequest()
    @        0x10f02a29a  yb::pggate::PgDocOp::GetResult()
    @        0x10f00e1cf  yb::pggate::PgDml::FetchDataFromServer()
    @        0x10f00de14  yb::pggate::PgDml::Fetch()
    @        0x10efd0022  yb::pggate::PgApiImpl::DmlFetch()
    @        0x10efc00e8  YBCPgDmlFetch
    @        0x10e24026d  ybcFetchNextHeapTuple
    @        0x10e24018e  ybc_getnext_heaptuple
    @        0x10e240c36  ybc_heap_getnext
    @        0x10e1dc1f1  heap_getnext
    @        0x10e250c76  IndexBuildHeapRangeScanInternal
    @        0x10e25090f  IndexBuildHeapRangeScan
    @        0x10e2508d3  IndexBuildHeapScan
    @        0x10e241bfe  ybcinbuild
    @        0x10e24f646  index_build
    @        0x10e24e4d4  index_create
    @        0x10e2e4b99  DefineIndex
    @        0x10e4a1d50  ProcessUtilitySlow
    @        0x10e49fe85  standard_ProcessUtility
    @        0x10e49f3ef  YBProcessUtilityDefaultHook
    @        0x11a0310a2  pgss_ProcessUtility
    @        0x11a048f8d  ybpgm_ProcessUtility
    @        0x11a05c5de  pgaudit_NextProcessUtility_hook
    @        0x11a05b975  pgaudit_ProcessUtility_hook
    @        0x11a06d9be  pg_hint_plan_ProcessUtility
    @        0x10e5b0307  YBTxnDdlProcessUtility
    @        0x10e49f561  ProcessUtility
    @        0x10e4a12d4  ProcessUtilitySlow
    @        0x10e49fe85  standard_ProcessUtility
    @        0x10e49f3ef  YBProcessUtilityDefaultHook

The 'extra' PGSQL_READ RPC request is generated by IndexBuildHeapScan() when building a primary key index.
No matter buildstate.is_backfill is set to true or false, IndexBuildHeapScan() is always called() in ybcinbuild().
I'm doing further investigation and will edit this comment later.

@jaki
Copy link
Contributor

jaki commented Apr 20, 2021

build state is_backfill is for online schema change. CREATE TABLE with pkey doesn't use online schema change. is_backfill is probably irrelevant to the issue. When @m-iancu says "backfill", he probably means generally building an index on existing rows.

I think that, for pkey, index_build shouldn't be called in the first place.

yifanguan added a commit that referenced this issue May 7, 2021
Summary:
Compared with creating one table without a pkey, when creating a table with a pkey, we issue a redundant read from a table scan.
This revision fixes redundant PGSQL_READ requests when creating tables with a primary key.
The test: org.yb.pgsql.TestPgCacheConsistency#testNoDDLRetry is removed because
  - this test fails as expected due to the change in this revision.
  - this test will be deleted by [[ https://phabricator.dev.yugabyte.com/D10666 | D10666 ]], so it is better to delete it rather than try to change it.

Test Plan:
./yb_build.sh --scb --java-test org.yb.pgsql.TestPgDdl#testCreateTableWithPrimaryKey

seekCount and nextCount were different for with and without pkey before code changes. With pkey and without pkey counts become the same after code changes.

Reviewers: mihnea, rskannan, jason

Reviewed By: jason

Subscribers: zyu, jason, yql

Differential Revision: https://phabricator.dev.yugabyte.com/D11344
YSQL automation moved this from Backlog to Done May 7, 2021
YintongMa pushed a commit to YintongMa/yugabyte-db that referenced this issue May 26, 2021
Summary:
Compared with creating one table without a pkey, when creating a table with a pkey, we issue a redundant read from a table scan.
This revision fixes redundant PGSQL_READ requests when creating tables with a primary key.
The test: org.yb.pgsql.TestPgCacheConsistency#testNoDDLRetry is removed because
  - this test fails as expected due to the change in this revision.
  - this test will be deleted by [[ https://phabricator.dev.yugabyte.com/D10666 | D10666 ]], so it is better to delete it rather than try to change it.

Test Plan:
./yb_build.sh --scb --java-test org.yb.pgsql.TestPgDdl#testCreateTableWithPrimaryKey

seekCount and nextCount were different for with and without pkey before code changes. With pkey and without pkey counts become the same after code changes.

Reviewers: mihnea, rskannan, jason

Reviewed By: jason

Subscribers: zyu, jason, yql

Differential Revision: https://phabricator.dev.yugabyte.com/D11344
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ysql Yugabyte SQL (YSQL)
Projects
YSQL
  
Done
Development

No branches or pull requests

3 participants