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

feat: savepoints #2278

Merged
merged 13 commits into from Apr 12, 2023
Merged

feat: savepoints #2278

merged 13 commits into from Apr 12, 2023

Conversation

olavloite
Copy link
Collaborator

@olavloite olavloite commented Feb 17, 2023

Adds support for savepoints to the Connection API. This will be surfaced to users through the JDBC driver and PGAdapter.

Savepoints use the internal retry mechanism for read/write transactions to emulate actual savepoints:

  • Setting a savepoint is supported in both read/write and read-only transactions. Savepoints in a read-only transaction are no-ops and are guaranteed to always work. They are supported purely for compatibility reasons. That is; if a framework automatically uses savepoints, this feature ensures that the framework does not fail if it uses savepoints in combination with a read-only transaction.
  • Releasing a savepoint removes it from the transaction. This is an in-mem operation only that is guaranteed to always work, assuming that the application uses an existing savepoint name.
  • Rolling back to a savepoint is supported for both read-only and read/write transactions. For read-only transactions it is a no-op and is guaranteed to always work. Rolling back to a savepoint in a read/write transaction is also always guaranteed to work. However, resuming the transaction after rolling back to a savepoint in a read/write transaction can fail if the underlying data has been modified, or if one or more of the statements before the savepoint that was rolled back to returns non-deterministic data. Note that rolling back to a savepoint does not remove the savepoint from the transaction. It is therefore possible to roll back to the same savepoint multiple times.

Adds support for savepoints to the Connection API. Savepoints use the internal retry
mechanism for read/write transactions to emulate actual savepoints. Rolling back to
a savepoint is guaranteed to always work, but resuming the transaction after rolling
back can fail if the underlying data has been modified, or if one or more of the
statements before the savepoint that was rolled back to returns non-deterministic
data.
@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. api: spanner Issues related to the googleapis/java-spanner API. labels Feb 17, 2023
@olavloite olavloite marked this pull request as ready for review February 18, 2023 09:36
@olavloite olavloite requested review from a team as code owners February 18, 2023 09:36
@olavloite olavloite changed the title feat: savepoint feat: savepoints Feb 18, 2023
@olavloite olavloite added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 28, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 28, 2023
}
}

private final LinkedList<Savepoint> savepoints = new LinkedList<>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I imagine you already picked the best data structure here, but wondering if you considered any other like Deque for stack like popping or a TreeMap for fast retrieval of elements.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered a couple of different options, but landed on LinkedList both for simplicity, but also because it aligns with how SAVEPOINT works. They are strictly ordered by an index (i.e. a map structure makes less sense, as there is no mapping between a key and a value other than a simple index key), and removing a savepoint from the list always means removing everything after it as well. That means that cutting one node in the middle of the list is a cheap operation.
That being said; a transaction is unlikely to ever have more than a handful of savepoints, meaning that the chosen data structure won't have any practical effect on the performance.

  • Deque was indeed one that I considered, but I dropped it as we need to be able to peek multiple steps back, which is not supported by a pure Deque implementation.
  • TreeMap or similar would not really be a good fit, as the key in this case is just an index. The savepoint names also do not need to be unique (in PostgreSQL).


/**
* Rolls back to the given savepoint. Rolling back to a savepoint undoes all changes and releases
* all locks that have been taken after the savepoint. Rolling back to a savepoint does not remove
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you mean internal locks, as we don't allow for "SQL" locks in Cloud Spanner?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

}
assertEquals(RANDOM_RESULT_SET_ROW_COUNT, count);
}
connection.commit();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if I commit after a rollbackToSavepoint? (I assume it would potentially work with the statements up to the given savepoint, if nothing has changed, but just verifying my understanding)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your understanding is correct. The idea is that a savepoint will allow you to reset the transaction to a valid point and then either continue or commit, instead of having to abort the entire transaction.

connection.executeUpdate(INSERT_STATEMENT);
connection.executeUpdate(INSERT_STATEMENT);
// Change the result of the initial query.
mockSpanner.putStatementResult(StatementResult.query(statement, generator.generate()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice test

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks :-)

@olavloite olavloite added kokoro:force-run Add this label to force Kokoro to re-run the tests. automerge Merge the pull request once unit tests and other checks pass. labels Apr 8, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 8, 2023
@gcf-merge-on-green
Copy link
Contributor

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Apr 8, 2023
@olavloite olavloite merged commit b02f584 into main Apr 12, 2023
23 checks passed
@olavloite olavloite deleted the savepoint branch April 12, 2023 05:09
gcf-merge-on-green bot pushed a commit that referenced this pull request Apr 14, 2023
🤖 I have created a release *beep* *boop*
---


## [6.40.0](https://togithub.com/googleapis/java-spanner/compare/v6.39.0...v6.40.0) (2023-04-14)


### Features

* Savepoints ([#2278](https://togithub.com/googleapis/java-spanner/issues/2278)) ([b02f584](https://togithub.com/googleapis/java-spanner/commit/b02f58435b97346cc8e08a96635affe8383981bb))


### Performance Improvements

* Remove custom transport executor ([#2366](https://togithub.com/googleapis/java-spanner/issues/2366)) ([e27dbe5](https://togithub.com/googleapis/java-spanner/commit/e27dbe5f58229dab208eeeed44d53e741700c814))


### Dependencies

* Update dependency com.google.cloud:google-cloud-shared-dependencies to v3.7.0 ([#2377](https://togithub.com/googleapis/java-spanner/issues/2377)) ([40402af](https://togithub.com/googleapis/java-spanner/commit/40402af54f94f16619d018e252181db29ae6855e))
* Update dependency org.graalvm.buildtools:junit-platform-native to v0.9.21 ([#2379](https://togithub.com/googleapis/java-spanner/issues/2379)) ([ae7262d](https://togithub.com/googleapis/java-spanner/commit/ae7262d37391c0ec2fee1dcbb24899e4fa16ae17))
* Update dependency org.graalvm.buildtools:native-maven-plugin to v0.9.21 ([#2380](https://togithub.com/googleapis/java-spanner/issues/2380)) ([0cb159e](https://togithub.com/googleapis/java-spanner/commit/0cb159efc97f02b42f064244e3812a0fd3d82db6))

---
This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/java-spanner API. size: xl Pull request size is extra large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants