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] Release locks acquired during aborted subtransaction #10039

Closed
robertsami opened this issue Sep 17, 2021 · 0 comments
Closed

[docdb] Release locks acquired during aborted subtransaction #10039

robertsami opened this issue Sep 17, 2021 · 0 comments
Labels
area/docdb YugabyteDB core features kind/bug This issue is a bug priority/medium Medium priority issue

Comments

@robertsami
Copy link
Contributor

robertsami commented Sep 17, 2021

Jira Link: DB-524
During a normal (no explicit locks) serializable read, we transparently write an "implicit" kStrongRead intent lock. These locks always belong to the top-level transaction (see: #9587)

However, in all other cases, including serializable reads with explicit locks, we should treat these locks as part of the subtransaction, and therefore they should be ignored by or invisible to other transactions if they are aborted (see: https://github.com/postgres/postgres/blob/master/src/backend/access/heap/README.tuplock#L99).

As a first cut, perhaps we can do this in a "best effort" async manner.

However, in order to fully match the behavior of Postgres, we should release these locks synchronously. Not doing so could lead to the following behavior, which does not match Postgres:

process 1

-- client 1

-- client 2

client 1 - begin;
	 - savepoint a;
	 - select * from t where a = 1 for update;

client 2 - begin;

client 1 - rollback to a;

client 2 - update t set b = 2 where a = 1 for update;  # if above rollback was not effectively synchronous, we may still conflict here

We could make this operation effectively synchronous by making ROLLBACK TO {savepoint} synchronously talk to the transaction coordinator and making some other modifications to our write path when determining if a lock belonging to a subtransaction_id > 1 is still valid.

@robertsami robertsami added the area/docdb YugabyteDB core features label Sep 17, 2021
@robertsami robertsami self-assigned this Sep 17, 2021
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue labels May 24, 2022
robertsami added a commit that referenced this issue Jun 1, 2022
…ther transactions during conflict resolution

Summary:
Before this commit, any intents of other transactions discovered during conflict resolution would have the same lifetime as the other transaction, even if those intents were part of an aborted subtransaction. This commit modifies the behavior to ignore all aborted intents of other transactions during conflict resolution, in a best-effort manner.

The changes we make to support this new behavior are as follows:
1. Send aborted subtransaction metadata to the transaction coordinator during transaction heartbeat
2. Send subtransaction metadata to transaction participant in GetTransactionStatus RPCs
3. Do not conflict with intents from aborted subtransactions in conflict resolution
4. Track whether any intents discovered in a given subtransaction are modification intents. If this is not the case for any subtransactions which are not aborted for a given transaction, and that transaction is committed, ignore it.

One important caveat is this now introduces a divergence from postgres behavior which was masked by the behavior we had before this commit. In postgres serializable isolation, any rows which are read implicitly acquire a read lock. Even if that read happened within a subtransaction which is later rolled back, the implicit read lock is tied to the scope of the outer transaction. We are tracking this here: #9587

Test Plan: ybd --java-test 'org.yb.pgsql.TestPgSavepoints'

Reviewers: sergei, pjain

Reviewed By: sergei, pjain

Subscribers: mbautin, zyu, ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D17177
robertsami added a commit that referenced this issue Jun 8, 2022
…ansactions of other transactions during conflict resolution

Summary:
Before this commit, any intents of other transactions discovered during conflict resolution would have the same lifetime as the other transaction, even if those intents were part of an aborted subtransaction. This commit modifies the behavior to ignore all aborted intents of other transactions during conflict resolution, in a best-effort manner.

The changes we make to support this new behavior are as follows:
1. Send aborted subtransaction metadata to the transaction coordinator during transaction heartbeat
2. Send subtransaction metadata to transaction participant in GetTransactionStatus RPCs
3. Do not conflict with intents from aborted subtransactions in conflict resolution
4. Track whether any intents discovered in a given subtransaction are modification intents. If this is not the case for any subtransactions which are not aborted for a given transaction, and that transaction is committed, ignore it.

One important caveat is this now introduces a divergence from postgres behavior which was masked by the behavior we had before this commit. In postgres serializable isolation, any rows which are read implicitly acquire a read lock. Even if that read happened within a subtransaction which is later rolled back, the implicit read lock is tied to the scope of the outer transaction. We are tracking this here: #9587

Original commit: D17177 / 4fb167

Test Plan: ybd --java-test 'org.yb.pgsql.TestPgSavepoints'

Reviewers: sergei, pjain

Reviewed By: pjain

Subscribers: ybase, zyu, mbautin

Differential Revision: https://phabricator.dev.yugabyte.com/D17383
pkj415 pushed a commit that referenced this issue Jun 19, 2022
…ansactions of other transactions during conflict resolution

Summary:
Before this commit, any intents of other transactions discovered during conflict resolution would have the same lifetime as the other transaction, even if those intents were part of an aborted subtransaction. This commit modifies the behavior to ignore all aborted intents of other transactions during conflict resolution, in a best-effort manner.

The changes we make to support this new behavior are as follows:
1. Send aborted subtransaction metadata to the transaction coordinator during transaction heartbeat
2. Send subtransaction metadata to transaction participant in GetTransactionStatus RPCs
3. Do not conflict with intents from aborted subtransactions in conflict resolution
4. Track whether any intents discovered in a given subtransaction are modification intents. If this is not the case for any subtransactions which are not aborted for a given transaction, and that transaction is committed, ignore it.

One important caveat is this now introduces a divergence from postgres behavior which was masked by the behavior we had before this commit. In postgres serializable isolation, any rows which are read implicitly acquire a read lock. Even if that read happened within a subtransaction which is later rolled back, the implicit read lock is tied to the scope of the outer transaction. We are tracking this here: #9587

Original commit: 4fb1676 / D17177

Test Plan: ybd --java-test 'org.yb.pgsql.TestPgSavepoints'

Reviewers: rsami

Reviewed By: rsami

Subscribers: ybase, zyu, mbautin

Differential Revision: https://phabricator.dev.yugabyte.com/D17708
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/bug This issue is a bug priority/medium Medium priority issue
Projects
None yet
Development

No branches or pull requests

3 participants