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

Create a new read method or isolation level for read request with FOR KEY SHARE FOR SHARE row locks #2496

Closed
hectorgcr opened this issue Oct 2, 2019 · 0 comments
Assignees
Labels
kind/enhancement This is an enhancement of an existing feature

Comments

@hectorgcr
Copy link
Contributor

Currently, whenever we receive a read request with a FOR KEY SHARE or FOR SHARE row lock, we set the isolation level of that request to SERIALIZABLE, so we can grab the right locks internally. It's better to have a different mechanism that is unique to these two locks so that we make it clear that we are not running a SERIALIZABLE transaction.

This is related to #1199

@hectorgcr hectorgcr self-assigned this Oct 2, 2019
@hectorgcr hectorgcr added the kind/enhancement This is an enhancement of an existing feature label Oct 2, 2019
@jaki jaki changed the title Crate a new read method or isolation level for read request with FOR KEY SHARE FOR SHARE row locks Create a new read method or isolation level for read request with FOR KEY SHARE FOR SHARE row locks Oct 3, 2019
@jaki jaki assigned jaki and unassigned hectorgcr Nov 12, 2019
jaki pushed a commit that referenced this issue Nov 20, 2019
Summary:
Enable `FOR UPDATE` and `FOR NO KEY UPDATE` row locking statements with
the caveat that `FOR NO KEY UPDATE` behaves like `FOR UPDATE` (see
follow-up issue #2922).  Also, fix conflict detection such that `FOR
SHARE` locks do not conflict with each other.

Implement the fix as follows:

1. Pass down `FOR UPDATE` and `FOR NO KEY UPDATE` row locks through
   Protobufs.

   1. Pass them through `PgsqlReadRequestPB.row_mark_type`.  (Also,
      change the field from `repeated` to `optional`.)
   1. Pass them through `KeyValueWriteBatchPB.row_mark_type`.  (Also,
      change the field from `repeated` to `optional`.)

1. Add a row lock parameter to `docdb::GetStrongIntentTypeSet`, and
   permeate row lock information throughout the affected areas (fix
   issue #2842).  Remove the isolation level hack in
   `tablet::Tablet::PrepareTransactionWriteBatch` in favor of using row
   lock information (fix issue #2496).
1. Create a new value type `kRowLock` to be placed in the value of
   intents for row locking.  Handle this value type in
   `docdb::(anonymous namespace)::DecodeStrongWriteIntent` (for
   in-transaction reads) and `docdb::IntentToWriteRequest` (for
   transaction commits).
1. Create tests, specified in the test plan.

Also, do the following:

* Create new files defining helper functions related to row locks (row
  marks).

  * `src/yb/common/row_mark.cc`
  * `src/yb/common/row_mark.h`

* Prevent `ROW_MARK_REFERENCE` and `ROW_MARK_COPY` from getting passed
  down as the `rowmark` execution parameter.
* Update regress test `yb_pg_privileges` (java test
  `TestPgRegressAuthorization`) to uncomment `FOR UPDATE` row locking
  statements.

Test Plan:
* Jenkins
* `./yb_build.sh`

  * `--cxx-test pgwrapper_pg_mini-test --gtest_filter
    PgMiniTest.RowLockConflictMatrix`
  * `--cxx-test pgwrapper_pg_mini-test --gtest_filter
    PgMiniTest.SerializableDeleteForNoKeyUpdate`
  * `--cxx-test pgwrapper_pg_mini-test --gtest_filter
    PgMiniTest.SerializableDeleteForUpdate`
  * `--cxx-test pgwrapper_pg_mini-test --gtest_filter
    PgMiniTest.SerializableInsertForNoKeyUpdate`
  * `--cxx-test pgwrapper_pg_mini-test --gtest_filter
    PgMiniTest.SerializableInsertForUpdate`
  * `--cxx-test pgwrapper_pg_mini-test --gtest_filter
    PgMiniTest.SnapshotDeleteForNoKeyUpdate`
  * `--cxx-test pgwrapper_pg_mini-test --gtest_filter
    PgMiniTest.SnapshotDeleteForUpdate`
  * `--cxx-test pgwrapper_pg_mini-test --gtest_filter
    PgMiniTest.SnapshotInsertForNoKeyUpdate`
  * `--cxx-test pgwrapper_pg_mini-test --gtest_filter
    PgMiniTest.SnapshotInsertForUpdate`

Reviewers: hector, sergei, mikhail

Reviewed By: mikhail

Subscribers: yql, bogdan

Differential Revision: https://phabricator.dev.yugabyte.com/D7453
@jaki jaki closed this as completed Nov 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement This is an enhancement of an existing feature
Projects
None yet
Development

No branches or pull requests

2 participants