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

[DocDB] Skip acquiring weak in-memory lock on host/primary table for locks of colocated tables #18790

Closed
1 task done
basavaraj29 opened this issue Aug 21, 2023 · 0 comments
Closed
1 task done
Assignees
Labels
area/docdb YugabyteDB core features kind/enhancement This is an enhancement of an existing feature priority/medium Medium priority issue

Comments

@basavaraj29
Copy link
Contributor

basavaraj29 commented Aug 21, 2023

Jira Link: DB-7672

Description

While determining locks that need to be obtained for a transaction in ::DetermineKeysToLock (docdb.cc), we obtain a weak lock on the empty key for all the write operations.

For transactions trying to acquire write locks on a colocated table, the above leads to a lock entry with an empty doc key (without the colocation id) which corresponds to the primary tablet. On finding conflict(s), a waiter transaction ends up in the wait-queue with lock entries formed from ::DetermineKeysToLock. In the existing implementation, a waiter transaction that executes UPDATE <table> SET ... WHERE .. enters the wait-queue with the following records

SubDocKey(DocKey([], []), []) parsed_intent.types=[kWeakRead, kWeakWrite]
SubDocKey(DocKey(ColocationId=990615573, [], []), []) parsed_intent.types=[kWeakRead, kWeakWrite]
SubDocKey(DocKey(ColocationId=990615573, [], [1]), []) parsed_intent.types=[kWeakRead, kWeakWrite]
SubDocKey(DocKey(ColocationId=990615573, [], [1]), [SystemColumnId(0)]) parsed_intent.types=[kStrongRead, kStrongWrite]

Since the colocation id is missing in the first lock entry, the table id isn't populated by the downstream code at the lock level. Also since the table id doesn't get populated at the tablet level (since this is a colocated tablet), pg_locks errors out with message Response must contain exactly one among LockInfoPB::table_id or TabletLockInfoPB::table_id.

Skip acquiring a weak lock on the empty key for write ops having a colocation id/cotable id set, as a weak lock on the colocated table is obtained anyways as part of obtaining weak locks on the other non-empty prefixes.

Warning: Please confirm that this issue does not contain any sensitive information

  • I confirm this issue does not contain any sensitive information.
@basavaraj29 basavaraj29 added area/docdb YugabyteDB core features status/awaiting-triage Issue awaiting triage labels Aug 21, 2023
@basavaraj29 basavaraj29 self-assigned this Aug 21, 2023
@yugabyte-ci yugabyte-ci added kind/enhancement This is an enhancement of an existing feature priority/medium Medium priority issue and removed status/awaiting-triage Issue awaiting triage labels Aug 21, 2023
basavaraj29 added a commit that referenced this issue Aug 23, 2023
… locks of colocated tables

Summary:
While determining locks that need to be obtained for a transaction in `::DetermineKeysToLock` (`docdb.cc`), we obtain a weak lock on the empty key for all the write operations.

For transactions trying to acquire write locks on a colocated table, the above leads to a lock entry with an empty doc key (without the colocation id) which corresponds to the primary tablet. On finding conflict(s), a waiter transaction ends up in the wait-queue with lock entries formed from `::DetermineKeysToLock`. In the existing implementation, a waiter transaction that executes `UPDATE <table> SET ... WHERE ..` enters the wait-queue with the following records
```
SubDocKey(DocKey([], []), []) parsed_intent.types=[kWeakRead, kWeakWrite]
SubDocKey(DocKey(ColocationId=990615573, [], []), []) parsed_intent.types=[kWeakRead, kWeakWrite]
SubDocKey(DocKey(ColocationId=990615573, [], [1]), []) parsed_intent.types=[kWeakRead, kWeakWrite]
SubDocKey(DocKey(ColocationId=990615573, [], [1]), [SystemColumnId(0)]) parsed_intent.types=[kStrongRead, kStrongWrite]
```

For a transaction with granted locks executing the same above statement, the empty key intent doesn't get written to intentsdb. we explicitly check for presence of cotable id/colocation id [[ https://github.com/yugabyte/yugabyte-db/blob/master/src/yb/dockv/intent.cc#L288 | here ]], and if exists, all the records that make it to intentsdb have this prefix. We should do the same in `::DetermineKeysToLock`, which is what the diff does. A weak lock on the colocated table is anyways obtained as part of obtaining weak locks on the other non-empty prefixes.

Failure that exposed this issue:
Since the colocation id is missing in the first lock entry, the table id isn't populated by the downstream code at the lock level. Also since the table id doesn't get populated at the tablet level (since this is a colocated tablet), `pg_locks` errors out with message `Response must contain exactly one among LockInfoPB::table_id or TabletLockInfoPB::table_id`.

Added a `pg_locks` test case that kind of validates the fix.
Jira: DB-7672, DB-7571

Test Plan:
Jenkins
./yb_build.sh --cxx-test pgwrapper_pg_get_lock_status-test --gtest_filter PgGetLockStatusTest.TestColocatedWaiterWriteLock

Reviewers: rsami, sergei

Reviewed By: rsami

Subscribers: rthallam, ybase, mlillibridge

Differential Revision: https://phorge.dev.yugabyte.com/D27882
basavaraj29 added a commit that referenced this issue Aug 23, 2023
…primary table for locks of colocated tables

Summary:
Original commit: 1e2dfef / D27882
While determining locks that need to be obtained for a transaction in `::DetermineKeysToLock` (`docdb.cc`), we obtain a weak lock on the empty key for all the write operations.

For transactions trying to acquire write locks on a colocated table, the above leads to a lock entry with an empty doc key (without the colocation id) which corresponds to the primary tablet. On finding conflict(s), a waiter transaction ends up in the wait-queue with lock entries formed from `::DetermineKeysToLock`. In the existing implementation, a waiter transaction that executes `UPDATE <table> SET ... WHERE ..` enters the wait-queue with the following records
```
SubDocKey(DocKey([], []), []) parsed_intent.types=[kWeakRead, kWeakWrite]
SubDocKey(DocKey(ColocationId=990615573, [], []), []) parsed_intent.types=[kWeakRead, kWeakWrite]
SubDocKey(DocKey(ColocationId=990615573, [], [1]), []) parsed_intent.types=[kWeakRead, kWeakWrite]
SubDocKey(DocKey(ColocationId=990615573, [], [1]), [SystemColumnId(0)]) parsed_intent.types=[kStrongRead, kStrongWrite]
```

For a transaction with granted locks executing the same above statement, the empty key intent doesn't get written to intentsdb. we explicitly check for presence of cotable id/colocation id [[ https://github.com/yugabyte/yugabyte-db/blob/master/src/yb/dockv/intent.cc#L288 | here ]], and if exists, all the records that make it to intentsdb have this prefix. We should do the same in `::DetermineKeysToLock`, which is what the diff does. A weak lock on the colocated table is anyways obtained as part of obtaining weak locks on the other non-empty prefixes.

Failure that exposed this issue:
Since the colocation id is missing in the first lock entry, the table id isn't populated by the downstream code at the lock level. Also since the table id doesn't get populated at the tablet level (since this is a colocated tablet), `pg_locks` errors out with message `Response must contain exactly one among LockInfoPB::table_id or TabletLockInfoPB::table_id`.

Added a `pg_locks` test case that kind of validates the fix.
Jira: DB-7672, DB-7571

Test Plan:
Jenkins
./yb_build.sh --cxx-test pgwrapper_pg_get_lock_status-test --gtest_filter PgGetLockStatusTest.TestColocatedWaiterWriteLock

Reviewers: rsami, sergei

Reviewed By: rsami

Subscribers: mlillibridge, ybase, rthallam

Differential Revision: https://phorge.dev.yugabyte.com/D28007
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docdb YugabyteDB core features kind/enhancement This is an enhancement of an existing feature priority/medium Medium priority issue
Projects
Status: Done
Development

No branches or pull requests

2 participants