chore: support TPCC benchmark #1699
Conversation
The signature of the public constructor for ProxyServer was accidentally changed in version 0.27.0. This re-instantes the constructor with the same signature.
…into support-tpcc-benchmark
…into support-tpcc-benchmark
…into support-tpcc-benchmark
…into support-tpcc-benchmark
There was a problem hiding this comment.
I have not looked into the actual TPC-C implementation, but I assume that it is still the same as the original. The main question that I have is 'Can we use java.sql.PreparedStatement (that works for both JDBC drivers) and remove the use of the simple query mode for the PostgreSQL JDBC driver.
benchmarks/tpcc/src/main/java/com/google/cloud/pgadapter/tpcc/AbstractBenchmarkRunner.java
Show resolved
Hide resolved
benchmarks/tpcc/src/main/java/com/google/cloud/pgadapter/tpcc/AbstractBenchmarkRunner.java
Outdated
Show resolved
Hide resolved
benchmarks/tpcc/src/main/java/com/google/cloud/pgadapter/tpcc/BenchmarkApplication.java
Outdated
Show resolved
Hide resolved
src/main/java/com/google/cloud/spanner/pgadapter/metadata/OptionsMetadata.java
Outdated
Show resolved
Hide resolved
src/main/java/com/google/cloud/spanner/pgadapter/statements/BackendConnection.java
Outdated
Show resolved
Hide resolved
| row = | ||
| paramQueryRow( | ||
| connection, | ||
| (tpccConfiguration.isLockScannedRanges() ? "/*@ lock_scanned_ranges=exclusive */" : "") |
There was a problem hiding this comment.
nit: we don't seem to add the lock_scanned_ranges hint to all queries. E.g. the query on line 368 does not add the hint. Is that intentional?
Would it otherwise make sense to move the logic for conditionally adding the hint to a query/DML statement into the paramQueryRow and similar methods, so it is easier to maintain and make sure all queries use the hint when the option has been enabled?
There was a problem hiding this comment.
What is the purpose of lock_scanned_ranges hint? I did not know we need to add it to all queries/DMLs.
I moved the logic to paramQueryRow, executeParamStatement, and also a few places using executeParamQuery.
There was a problem hiding this comment.
The hint is a potential optimization in a use-case like this. Whether it actually improves throughput or not, is currently not clear to me, but in theory it might. Adding this hint makes Spanner behave more like a 'traditional' database in case of lock conflicts; instead of optimistically only taking a read-lock, it will instruct Spanner to take an exclusive write lock.
And now that I think of it: I was the one who only added this in some places.... I added it where the standard implementation would use select ... for update. Which is where we would potentially want to add this. Sorry for the confusion. Let's just leave it as is for now, and optimize this in a follow-up PR (if necessary).
There was a problem hiding this comment.
Yeah, probably, it's not quite right to use exclusive lock to every query and DML. I will create a PR to revert this change.
benchmarks/tpcc/src/main/java/com/google/cloud/pgadapter/tpcc/AbstractBenchmarkRunner.java
Outdated
Show resolved
Hide resolved
benchmarks/tpcc/src/main/java/com/google/cloud/pgadapter/tpcc/BenchmarkApplication.java
Outdated
Show resolved
Hide resolved
olavloite
merged commit 6ea9343
into
GoogleCloudPlatform:postgresql-dialect
This adds the support to run TPCC benchmarking with the following runners:
In addition, we enabled the client library's metrics and added a OpenTelemetry metric in the benchmark runner to record roundtrip latencies.