test: add a default benchmark for measuring baselines. #2739

arpan14 · 2023-11-23T05:31:29Z

Adding a benchmark (along with usual considerations) for measuring the latencies of some Java Client APIs. For any new feature, we usually first define the baseline by running benchmarks (without the feature).
Adding few utility methods to measure the p99/p95/p50 latencies.
Added benchmarking best-practices to avoid hotspot (by randomising keys), avoid cold start, etc.

…od while retrying exceptions in unit tests. * For details on issue see - googleapis#2206

fix: prevent illegal negative timeout values into thread sleep() method while retrying exceptions in unit tests.

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

google-cloud-spanner/src/test/java/com/google/cloud/spanner/BenchmarkingDataLoaderUtility.java

google-cloud-spanner/src/test/java/com/google/cloud/spanner/DefaultBenchmark.java

google-cloud-spanner/src/test/java/com/google/cloud/spanner/BenchmarkingUtilityTest.java

olavloite · 2024-02-29T11:25:21Z

google-cloud-spanner/src/test/java/com/google/cloud/spanner/BenchmarkingUtilityTest.java

+public class BenchmarkingUtilityTest {
+
+  @ClassRule
+  public static IntegrationTestEnv env = new IntegrationTestEnv();


Is this used anywhere?

olavloite · 2024-02-29T11:30:35Z

google-cloud-spanner/src/test/java/com/google/cloud/spanner/DefaultBenchmark.java

+   * <p>Some read requests will be long-running and will cause session leaks. Such sessions will be
+   * removed by the session maintenance background task if SessionPool Option
+   * ActionOnInactiveTransaction is set as WARN_AND_CLOSE.
+   *
+   * <p>Some write requests will be long-running. The test asserts that no sessions are removed by
+   * the session maintenance background task with SessionPool Option ActionOnInactiveTransaction set
+   * as WARN_AND_CLOSE. This is because PDML writes are expected to be long-running.


copy-paste error?

Yes, removed.

olavloite · 2024-02-29T11:32:17Z

google-cloud-spanner/src/test/java/com/google/cloud/spanner/DefaultBenchmark.java

+      final BenchmarkState server, int numberOfOperations) {
+    List<Duration> results = new ArrayList<>(numberOfOperations);
+    // Execute one update to make sure everything has been warmed up.
+    executeUpdate(server);


Shouldn't this also call executeWarmup, so the number of warmup queries is respected for updates as well?

Yes, added this.

olavloite · 2024-02-29T11:34:40Z

google-cloud-spanner/src/test/java/com/google/cloud/spanner/BenchmarkingUtilityTest.java

+   * <p>Table schema used here: CREATE TABLE FOO ( id INT64 NOT NULL, BAZ INT64, BAR INT64, )
+   * PRIMARY KEY(id);
+   */
+  @Test


Do we want this as an actual test that is executed during (nightly?) builds? If so, it should verify that the test actually succeeds by adding at least one assertion.

Or would it make more sense to move this method to the actual benchmark class, so it can more easily be executed together with a benchmark?

I had annotated BenchmarkingUtilityTest with @Category(SlowTest.class). This will make sure its excluded from our pre-submits and nightly tests. Note that the data loading utility is a one time activity and does not need to be executed repeatedly along with the benchmarks. Hence, I think it will be better to leave this as separate standalone class.

I have now moved the latency reporting utility methods (printResults, percentile, etc.) to a new class AbstractLatencyBenchmark. Any benchmark that aims to report latencies can extend AbstractLatencyBenchmark and get this methods.

I am renaming this class to BenchmarkingUtilityScripts and it now only has the data loading functionality. Avoiding Test in the class name will ensure it does not qualify for any continuous tests.

…nchmarkingUtilityTest.java Co-authored-by: Knut Olav Løite <koloite@gmail.com>

olavloite

LGTM

olavloite · 2024-03-01T18:45:23Z

google-cloud-spanner/src/test/java/com/google/cloud/spanner/DefaultBenchmark.java

 *
 * <p>CREATE TABLE FOO ( id INT64 NOT NULL, BAZ INT64, BAR INT64, ) PRIMARY KEY(id);
 *
 * <p>Below are a few considerations here: 1. We use all default options for this test because that


super-nit (feel free to leave as is): Use <ol> and <li> tags to create an actual ordered (numbered) list.

arpan14 and others added 12 commits February 8, 2023 12:22

fix: prevent illegal negative timeout values into thread sleep() meth…

edc5bbf

…od while retrying exceptions in unit tests. * For details on issue see - googleapis#2206

Merge pull request #1 from arpan14/retryerror

49a85df

fix: prevent illegal negative timeout values into thread sleep() method while retrying exceptions in unit tests.

Fixing lint issues.

4cd497b

Merge branch 'googleapis:main' into main

4a6aa8e

Merge branch 'googleapis:main' into main

b2aa09d

Merge branch 'googleapis:main' into main

8d6d71e

Merge branch 'googleapis:main' into main

77e6e7d

Merge branch 'googleapis:main' into main

e8b7fad

Merge branch 'googleapis:main' into main

8aa84e1

Merge branch 'googleapis:main' into main

57fd405

Merge branch 'googleapis:main' into main

1253563

test: add a default benchmark for measuring baselines.

cd6c20c

arpan14 requested a review from a team as a code owner November 23, 2023 05:31

product-auto-label bot added size: l Pull request size is large. api: spanner Issues related to the googleapis/java-spanner API. labels Nov 23, 2023

arpan14 added the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 23, 2023

gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 23, 2023

🦉 Updates from OwlBot post-processor

972d466

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

gcf-owl-bot bot requested a review from a team as a code owner November 23, 2023 07:38

olavloite reviewed Nov 24, 2023

View reviewed changes

arpan14 added 2 commits February 26, 2024 16:56

chore: address PR comments.

8de3b20

chore: address comments.

1ce06fe

arpan14 requested a review from olavloite February 27, 2024 10:07

Merge branch 'main' into latency-benchmark-utils

7d67e62

olavloite reviewed Feb 29, 2024

View reviewed changes

arpan14 and others added 2 commits March 1, 2024 23:21

Update google-cloud-spanner/src/test/java/com/google/cloud/spanner/Be…

e3548bd

…nchmarkingUtilityTest.java Co-authored-by: Knut Olav Løite <koloite@gmail.com>

chore: resolve PR comments.

ebda762

arpan14 requested a review from olavloite March 1, 2024 18:14

olavloite approved these changes Mar 1, 2024

View reviewed changes

arpan14 added the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 2, 2024

gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 2, 2024

arpan14 added the automerge Merge the pull request once unit tests and other checks pass. label Mar 2, 2024

gcf-merge-on-green bot merged commit 9a8cce8 into googleapis:main Mar 2, 2024

gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Mar 2, 2024

arpan14 deleted the latency-benchmark-utils branch March 2, 2024 15:36

test: add a default benchmark for measuring baselines. #2739

test: add a default benchmark for measuring baselines. #2739

Uh oh!

Conversation

arpan14 commented Nov 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arpan14 Mar 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

olavloite left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

arpan14 commented Nov 23, 2023 •

edited

Loading

arpan14 Mar 1, 2024 •

edited

Loading