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!: add support for CommitStats #544

Merged
merged 25 commits into from Feb 17, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
3872199
feat!: add support for CommitStats
olavloite Oct 24, 2020
def8768
fix: add clirr ignored differences
olavloite Oct 24, 2020
5372dae
Merge branch 'master' into commit-stats2
olavloite Oct 31, 2020
34fbda6
fix: error message should start with getCommitResponse
olavloite Oct 31, 2020
8f61c2a
Merge branch 'master' into commit-stats2
olavloite Nov 6, 2020
041b34d
Merge branch 'master' into commit-stats2
olavloite Dec 5, 2020
8299054
fix: remove overload delay
olavloite Dec 10, 2020
9b710a7
Merge branch 'master' into commit-stats2
olavloite Dec 10, 2020
8c15641
Merge branch 'master' into commit-stats2
olavloite Jan 23, 2021
919cf02
chore: cleanup after merge
olavloite Jan 23, 2021
97ec917
fix: update copyright years of new files
olavloite Jan 23, 2021
afeb0fd
test: fix flaky test
olavloite Jan 23, 2021
8524526
test: skip commit stats tests on emulator
olavloite Jan 23, 2021
06b3e22
test: missed one commit stats tests against emulator
olavloite Jan 23, 2021
1c17d16
test: skip another emulator test
olavloite Jan 23, 2021
664b87d
test: add missing test cases
olavloite Jan 23, 2021
6573a0f
fix: address review comments
olavloite Jan 30, 2021
0b448c3
Merge branch 'master' into commit-stats2
olavloite Feb 1, 2021
83039fb
Merge branch 'master' into commit-stats2
olavloite Feb 3, 2021
57ea714
chore: use junit assertion instead of truth
olavloite Feb 3, 2021
66c5f88
chore: replace truth asserts with junit asserts
olavloite Feb 4, 2021
1d17973
chore: replace truth assertions with junit
olavloite Feb 5, 2021
48ef21b
chore: cleanup test and variable names
olavloite Feb 8, 2021
1f33c42
fix: rename test method and variables
olavloite Feb 9, 2021
7d45ace
fix: address review comments
olavloite Feb 16, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .kokoro/release/publish_javadoc.sh
Expand Up @@ -62,7 +62,7 @@ popd
# V2
mvn clean site -B -q -Ddevsite.template="${KOKORO_GFILE_DIR}/java/"

pushd target/devsite
pushd target/devsite/reference

# create metadata
python3 -m docuploader create-metadata \
Expand Down
7 changes: 4 additions & 3 deletions CODE_OF_CONDUCT.md
@@ -1,3 +1,4 @@
<!-- # Generated by synthtool. DO NOT EDIT! !-->
# Code of Conduct

## Our Pledge
Expand Down Expand Up @@ -69,12 +70,12 @@ dispute. If you are unable to resolve the matter for any reason, or if the
behavior is threatening or harassing, report it. We are dedicated to providing
an environment where participants feel welcome and safe.

Reports should be directed to *[PROJECT STEWARD NAME(s) AND EMAIL(s)]*, the
Project Steward(s) for *[PROJECT NAME]*. It is the Project Steward’s duty to
Reports should be directed to *googleapis-stewards@google.com*, the
Project Steward(s) for *Google Cloud Client Libraries*. It is the Project Steward’s duty to
receive and address reported violations of the code of conduct. They will then
work with a committee consisting of representatives from the Open Source
Programs Office and the Google Open Source Strategy team. If for any reason you
are uncomfortable reaching out the Project Steward, please email
are uncomfortable reaching out to the Project Steward, please email
opensource@google.com.

We will investigate every complaint, but you may not receive a direct response.
Expand Down
47 changes: 47 additions & 0 deletions google-cloud-spanner/clirr-ignored-differences.xml
Expand Up @@ -406,4 +406,51 @@
<className>com/google/cloud/spanner/AbstractLazyInitializer</className>
<method>java.lang.Object initialize()</method>
</difference>

<!-- Support for CommitStats added -->
Copy link
Contributor

Choose a reason for hiding this comment

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

another one where a major version bump is required

<difference>
<differenceType>7012</differenceType>
<className>com/google/cloud/spanner/AsyncTransactionManager</className>
<method>com.google.api.core.ApiFuture getCommitResponse()</method>
</difference>
<difference>
<differenceType>7012</differenceType>
<className>com/google/cloud/spanner/AsyncRunner</className>
<method>com.google.api.core.ApiFuture getCommitResponse()</method>
</difference>
<difference>
<differenceType>7012</differenceType>
<className>com/google/cloud/spanner/TransactionManager</className>
<method>com.google.cloud.spanner.CommitResponse getCommitResponse()</method>
</difference>
<difference>
<differenceType>7012</differenceType>
<className>com/google/cloud/spanner/TransactionRunner</className>
<method>com.google.cloud.spanner.CommitResponse getCommitResponse()</method>
</difference>
<difference>
<differenceType>7004</differenceType>
<className>com/google/cloud/spanner/DatabaseClient</className>
<method>com.google.cloud.spanner.TransactionRunner readWriteTransaction()</method>
<to>com.google.cloud.spanner.TransactionRunner readWriteTransaction(com.google.cloud.spanner.TransactionOption[])</to>
</difference>
<difference>
<differenceType>7004</differenceType>
<className>com/google/cloud/spanner/DatabaseClient</className>
<method>com.google.cloud.spanner.TransactionManager transactionManager()</method>
<to>com.google.cloud.spanner.TransactionManager transactionManager(com.google.cloud.spanner.TransactionOption[])</to>
</difference>
<difference>
<differenceType>7004</differenceType>
<className>com/google/cloud/spanner/DatabaseClient</className>
<method>com.google.cloud.spanner.AsyncRunner runAsync()</method>
<to>com.google.cloud.spanner.AsyncRunner runAsync(com.google.cloud.spanner.TransactionOption[])</to>
</difference>
<difference>
<differenceType>7004</differenceType>
<className>com/google/cloud/spanner/DatabaseClient</className>
<method>com.google.cloud.spanner.AsyncTransactionManager transactionManagerAsync()</method>
<to>com.google.cloud.spanner.AsyncTransactionManager transactionManagerAsync(com.google.cloud.spanner.TransactionOption[])</to>
</difference>

</differences>
Expand Up @@ -52,8 +52,14 @@ interface AsyncWork<R> {
<R> ApiFuture<R> runAsync(AsyncWork<R> work, Executor executor);

/**
* Returns the timestamp at which the transaction committed. {@link ApiFuture#get()} will throw an
* {@link ExecutionException} if the transaction did not commit.
* Returns the timestamp at which the transaction committed. The {@link ApiFuture#get()} will
* throw an {@link ExecutionException} if the transaction did not commit.
*/
ApiFuture<Timestamp> getCommitTimestamp();

/**
* Returns the {@link CommitResponse} of this transaction. The {@link ApiFuture#get()} will throw
* an {@link ExecutionException} if the transaction did not commit.
*/
ApiFuture<CommitResponse> getCommitResponse();
}
Expand Up @@ -16,19 +16,23 @@

package com.google.cloud.spanner;

import com.google.api.core.ApiFunction;
import com.google.api.core.ApiFuture;
import com.google.api.core.ApiFutures;
import com.google.api.core.SettableApiFuture;
import com.google.cloud.Timestamp;
import com.google.cloud.spanner.TransactionRunner.TransactionCallable;
import com.google.common.base.Preconditions;
import com.google.common.util.concurrent.MoreExecutors;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executor;

class AsyncRunnerImpl implements AsyncRunner {
private final TransactionRunnerImpl delegate;
private final SettableApiFuture<Timestamp> commitTimestamp = SettableApiFuture.create();
private final SettableApiFuture<CommitResponse> commitResponse = SettableApiFuture.create();

AsyncRunnerImpl(TransactionRunnerImpl delegate) {
this.delegate = delegate;
this.delegate = Preconditions.checkNotNull(delegate);
}

@Override
Expand All @@ -43,7 +47,7 @@ public void run() {
} catch (Throwable t) {
res.setException(t);
} finally {
setCommitTimestamp();
setCommitResponse();
}
}
});
Expand All @@ -66,16 +70,28 @@ public R run(TransactionContext transaction) throws Exception {
});
}

private void setCommitTimestamp() {
private void setCommitResponse() {
try {
commitTimestamp.set(delegate.getCommitTimestamp());
commitResponse.set(delegate.getCommitResponse());
} catch (Throwable t) {
Copy link
Contributor

Choose a reason for hiding this comment

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

letting it slide because it isn't changed in this PR, but catching Throwable is only rarely what you want. This is probably worth filing a bug on.

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 issue: #875

commitTimestamp.setException(t);
commitResponse.setException(t);
}
}

@Override
public ApiFuture<Timestamp> getCommitTimestamp() {
return commitTimestamp;
return ApiFutures.transform(
commitResponse,
new ApiFunction<CommitResponse, Timestamp>() {
@Override
public Timestamp apply(CommitResponse input) {
return input.getCommitTimestamp();
}
},
MoreExecutors.directExecutor());
}

public ApiFuture<CommitResponse> getCommitResponse() {
return commitResponse;
}
}
Expand Up @@ -191,6 +191,9 @@ public interface AsyncTransactionFunction<I, O> {
/** Returns the state of the transaction. */
TransactionState getState();

/** Returns the {@link CommitResponse} of this transaction. */
ApiFuture<CommitResponse> getCommitResponse();

/**
* Closes the manager. If there is an active transaction, it will be rolled back. Underlying
* session will be released back to the session pool.
Expand Down
Expand Up @@ -17,6 +17,7 @@
package com.google.cloud.spanner;

import com.google.api.core.ApiAsyncFunction;
import com.google.api.core.ApiFunction;
import com.google.api.core.ApiFuture;
import com.google.api.core.ApiFutureCallback;
import com.google.api.core.ApiFutures;
Expand All @@ -40,14 +41,16 @@ final class AsyncTransactionManagerImpl

private final SessionImpl session;
private Span span;
private final Options options;

private TransactionRunnerImpl.TransactionContextImpl txn;
private TransactionState txnState;
private final SettableApiFuture<Timestamp> commitTimestamp = SettableApiFuture.create();
private final SettableApiFuture<CommitResponse> commitResponse = SettableApiFuture.create();

AsyncTransactionManagerImpl(SessionImpl session, Span span) {
AsyncTransactionManagerImpl(SessionImpl session, Span span, Options options) {
this.session = session;
this.span = span;
this.options = options;
}

@Override
Expand Down Expand Up @@ -80,7 +83,7 @@ public TransactionContextFutureImpl beginAsync() {

private ApiFuture<TransactionContext> internalBeginAsync(boolean firstAttempt) {
txnState = TransactionState.STARTED;
txn = session.newTransaction();
txn = session.newTransaction(options);
if (firstAttempt) {
session.setActive(this);
}
Expand Down Expand Up @@ -126,28 +129,36 @@ public ApiFuture<Timestamp> commitAsync() {
SpannerExceptionFactory.newSpannerException(
ErrorCode.ABORTED, "Transaction already aborted"));
}
ApiFuture<Timestamp> res = txn.commitAsync();
ApiFuture<CommitResponse> res = txn.commitAsync();
Copy link
Contributor

Choose a reason for hiding this comment

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

no abbreviated variable names per google style.
Concretely I did not know what this was when I read it below and had to scroll up to find out.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed to commitResponseFuture

txnState = TransactionState.COMMITTED;
ApiFutures.addCallback(
res,
new ApiFutureCallback<Timestamp>() {
new ApiFutureCallback<CommitResponse>() {
@Override
public void onFailure(Throwable t) {
if (t instanceof AbortedException) {
txnState = TransactionState.ABORTED;
} else {
txnState = TransactionState.COMMIT_FAILED;
commitTimestamp.setException(t);
commitResponse.setException(t);
}
}

@Override
public void onSuccess(Timestamp result) {
commitTimestamp.set(result);
public void onSuccess(CommitResponse result) {
commitResponse.set(result);
}
},
MoreExecutors.directExecutor());
return ApiFutures.transform(
res,
new ApiFunction<CommitResponse, Timestamp>() {
@Override
public Timestamp apply(CommitResponse input) {
return input.getCommitTimestamp();
}
},
MoreExecutors.directExecutor());
return res;
}

@Override
Expand Down Expand Up @@ -184,6 +195,11 @@ public TransactionState getState() {
return txnState;
}

@Override
public ApiFuture<CommitResponse> getCommitResponse() {
return commitResponse;
}

@Override
public void invalidate() {
if (txnState == TransactionState.STARTED || txnState == null) {
Expand Down
Expand Up @@ -17,20 +17,31 @@
package com.google.cloud.spanner;

import com.google.cloud.Timestamp;
import com.google.common.base.Preconditions;
import java.util.Objects;

/** Represents a response from a commit operation. */
public class CommitResponse {

private final Timestamp commitTimestamp;
private final com.google.spanner.v1.CommitResponse proto;

public CommitResponse(Timestamp commitTimestamp) {
this.commitTimestamp = commitTimestamp;
CommitResponse(com.google.spanner.v1.CommitResponse proto) {
this.proto = proto;
}

/** Returns a {@link Timestamp} representing the commit time of the write operation. */
/** Returns a {@link Timestamp} representing the commit time of the transaction. */
public Timestamp getCommitTimestamp() {
return commitTimestamp;
return Timestamp.fromProto(proto.getCommitTimestamp());
}

/**
* Commit statistics are returned by a read/write transaction if specifically requested by passing
* in {@link Options#commitStats()} to the transaction.
*/
public CommitStats getCommitStats() {
Preconditions.checkState(
proto.hasCommitStats(), "The CommitResponse does not contain any commit statistics.");
return CommitStats.fromProto(proto.getCommitStats());
}

@Override
Expand All @@ -42,11 +53,11 @@ public boolean equals(Object o) {
return false;
}
CommitResponse that = (CommitResponse) o;
return Objects.equals(commitTimestamp, that.commitTimestamp);
return Objects.equals(proto, that.proto);
}

@Override
public int hashCode() {
return Objects.hash(commitTimestamp);
return Objects.hash(proto);
}
}
@@ -0,0 +1,66 @@
/*
* Copyright 2020 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.google.cloud.spanner;

import com.google.common.base.Preconditions;
import org.threeten.bp.Duration;
import org.threeten.bp.temporal.ChronoUnit;

/**
* Commit statistics are returned by a read/write transaction if specifically requested by passing
* in {@link Options#commitStats()} to the transaction.
*/
public class CommitStats {
private final long mutationCount;
private final Duration overloadDelay;

private CommitStats(long mutationCount, Duration overloadDelay) {
this.mutationCount = mutationCount;
this.overloadDelay = overloadDelay;
}

static CommitStats fromProto(com.google.spanner.v1.CommitResponse.CommitStats proto) {
Preconditions.checkNotNull(proto);
return new CommitStats(
proto.getMutationCount(),
Duration.of(proto.getOverloadDelay().getSeconds(), ChronoUnit.SECONDS)
.plusNanos(proto.getOverloadDelay().getNanos()));
}

/**
* The number of mutations that were executed by the transaction. Insert and update operations
* count with the multiplicity of the number of columns they affect. For example, inserting a new
* record may count as five mutations, if values are inserted into five columns. Delete and delete
* range operations count as one mutation regardless of the number of columns affected. Deleting a
* row from a parent table that has the ON DELETE CASCADE annotation is also counted as one
* mutation regardless of the number of interleaved child rows present. The exception to this is
* if there are secondary indexes defined on rows being deleted, then the changes to the secondary
* indexes will be counted individually. For example, if a table has 2 secondary indexes, deleting
Copy link
Contributor

Choose a reason for hiding this comment

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

will be --> are

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

* a range of rows in the table will count as 1 mutation for the table, plus 2 mutations for each
Copy link
Contributor

Choose a reason for hiding this comment

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

will count --> counts

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

* row that is deleted because the rows in the secondary index might be scattered over the
* key-space, making it impossible for Cloud Spanner to call a single delete range operation on
* the secondary indexes. Secondary indexes include the foreign keys backing indexes.
*/
public long getMutationCount() {
return mutationCount;
}

/** The duration that the commit was delayed due to overloaded servers. */
public Duration getOverloadDelay() {
return overloadDelay;
}
}