Skip to content

Commit

Permalink
feat: introduce java.time variables and methods (#3495)
Browse files Browse the repository at this point in the history
This PR introduces `java.time` alternatives to existing `org.threeten.bp.*` methods, as well as switching internal variables (if any) to `java.time`

The main constraint is to keep the changes backwards compatible, so for each existing threeten method "`method1(org.threeten.bp.Duration)`" we will add an alternative with a _Duration_ (or _Timestamp_ when applicable) suffix: "`method1Duration(java.time.Duration)`".

For most cases, the implementation will be held in the `java.time` method and the old threeten method will just delegate the call to it. However, for the case of abstract classes, the implementation will be kept in the threeten method to avoid breaking changes (i.e. users that already overloaded the method in their user code).
  • Loading branch information
diegomarquezp authored Dec 5, 2024
1 parent 4ed455a commit 8a7d533
Show file tree
Hide file tree
Showing 64 changed files with 577 additions and 396 deletions.
4 changes: 0 additions & 4 deletions google-cloud-spanner-executor/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -145,10 +145,6 @@
<groupId>com.google.api</groupId>
<artifactId>gax-grpc</artifactId>
</dependency>
<dependency>
<groupId>org.threeten</groupId>
<artifactId>threetenbp</artifactId>
</dependency>
<dependency>
<groupId>com.google.code.findbugs</groupId>
<artifactId>jsr305</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,8 @@
import java.io.Serializable;
import java.math.BigDecimal;
import java.text.ParseException;
import java.time.Duration;
import java.time.LocalDate;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
Expand All @@ -186,8 +188,6 @@
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.apache.commons.io.FileUtils;
import org.threeten.bp.Duration;
import org.threeten.bp.LocalDate;

/**
* Implementation of the SpannerExecutorProxy gRPC service that proxies action request through the
Expand Down Expand Up @@ -818,13 +818,13 @@ private synchronized Spanner getClient(long timeoutSeconds, boolean useMultiplex
}
RetrySettings retrySettings =
RetrySettings.newBuilder()
.setInitialRetryDelay(Duration.ofSeconds(1))
.setInitialRetryDelayDuration(Duration.ofSeconds(1))
.setRetryDelayMultiplier(1.3)
.setMaxRetryDelay(Duration.ofSeconds(32))
.setInitialRpcTimeout(rpcTimeout)
.setMaxRetryDelayDuration(Duration.ofSeconds(32))
.setInitialRpcTimeoutDuration(rpcTimeout)
.setRpcTimeoutMultiplier(1.0)
.setMaxRpcTimeout(rpcTimeout)
.setTotalTimeout(rpcTimeout)
.setMaxRpcTimeoutDuration(rpcTimeout)
.setTotalTimeoutDuration(rpcTimeout)
.build();

com.google.cloud.spanner.SessionPoolOptions.Builder poolOptionsBuilder =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

package com.google.cloud.spanner;

import org.threeten.bp.Instant;
import java.time.Instant;

/**
* Wrapper around current time so that we can fake it in tests. TODO(user): Replace with Java 8
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,17 @@

package com.google.cloud.spanner;

import static com.google.api.gax.util.TimeConversionUtils.toJavaTimeDuration;

import com.google.api.core.InternalApi;
import com.google.api.gax.tracing.ApiTracer;
import com.google.api.gax.tracing.BaseApiTracer;
import com.google.api.gax.tracing.MetricsTracer;
import com.google.common.collect.ImmutableList;
import java.time.Duration;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import org.threeten.bp.Duration;

@InternalApi
public class CompositeTracer extends BaseApiTracer {
Expand Down Expand Up @@ -109,14 +111,14 @@ public void attemptCancelled() {
}

@Override
public void attemptFailed(Throwable error, Duration delay) {
public void attemptFailed(Throwable error, org.threeten.bp.Duration delay) {
for (ApiTracer child : children) {
child.attemptFailed(error, delay);
child.attemptFailedDuration(error, toJavaTimeDuration(delay));
}
}

@Override
public void attemptFailedDuration(Throwable error, java.time.Duration delay) {
public void attemptFailedDuration(Throwable error, Duration delay) {
for (ApiTracer child : children) {
child.attemptFailedDuration(error, delay);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,14 @@
import com.google.common.collect.AbstractIterator;
import com.google.common.util.concurrent.Uninterruptibles;
import com.google.spanner.v1.PartialResultSet;
import java.time.Duration;
import java.util.Optional;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.TimeUnit;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.Nullable;
import org.threeten.bp.Duration;

/** Adapts a streaming read/query call into an iterator over partial result sets. */
@VisibleForTesting
Expand Down Expand Up @@ -77,7 +77,8 @@ public void setCall(SpannerRpc.StreamingCall call, boolean withBeginTransaction)
this.call = call;
this.withBeginTransaction = withBeginTransaction;
ApiCallContext callContext = call.getCallContext();
Duration streamWaitTimeout = callContext == null ? null : callContext.getStreamWaitTimeout();
Duration streamWaitTimeout =
callContext == null ? null : callContext.getStreamWaitTimeoutDuration();
if (streamWaitTimeout != null) {
// Determine the timeout unit to use. This reduces the precision to seconds if the timeout
// value is more than 1 second, which is lower than the precision that would normally be
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@
import com.google.cloud.spanner.SpannerOptions.FixedCloseableExecutorProvider;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.time.Duration;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.ThreadLocalRandom;
import org.threeten.bp.Duration;

public class LatencyTest {

Expand All @@ -42,7 +42,7 @@ public static void main(String[] args) throws Exception {
Paths.get("/Users/loite/Downloads/appdev-soda-spanner-staging.json"))))
.setSessionPoolOption(
SessionPoolOptions.newBuilder()
.setWaitForMinSessions(Duration.ofSeconds(5L))
.setWaitForMinSessionsDuration(Duration.ofSeconds(5L))
// .setUseMultiplexedSession(true)
.build())
.setUseVirtualThreads(true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ public void onSessionCreateFailure(Throwable t, int createFailureForSessionCount

private static void maybeWaitForSessionCreation(
SessionPoolOptions sessionPoolOptions, ApiFuture<SessionReference> future) {
org.threeten.bp.Duration waitDuration = sessionPoolOptions.getWaitForMinSessions();
Duration waitDuration = sessionPoolOptions.getWaitForMinSessions();
if (waitDuration != null && !waitDuration.isZero()) {
long timeoutMillis = waitDuration.toMillis();
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@

package com.google.cloud.spanner;

import static com.google.api.gax.util.TimeConversionUtils.toJavaTimeDuration;

import com.google.api.core.ObsoleteApi;
import com.google.api.gax.tracing.ApiTracer;
import com.google.api.gax.tracing.ApiTracerFactory.OperationType;
import com.google.common.base.Preconditions;
Expand All @@ -24,10 +27,10 @@
import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.api.trace.StatusCode;
import java.time.Duration;
import java.util.concurrent.atomic.AtomicLong;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.threeten.bp.Duration;

/**
* {@link com.google.api.gax.tracing.ApiTracer} for use with OpenTelemetry. Based on {@link
Expand Down Expand Up @@ -163,8 +166,15 @@ public void attemptCancelled() {
lastConnectionId = null;
}

/** This method is obsolete. Use {@link #attemptFailedDuration(Throwable, Duration)} instead. */
@Override
@ObsoleteApi("Use attemptFailedDuration(Throwable, Duration) instead")
public void attemptFailed(Throwable error, org.threeten.bp.Duration delay) {
attemptFailedDuration(error, toJavaTimeDuration(delay));
}

@Override
public void attemptFailed(Throwable error, Duration delay) {
public void attemptFailedDuration(Throwable error, Duration delay) {
AttributesBuilder builder = baseAttemptAttributesBuilder();
if (delay != null) {
builder.put(RETRY_DELAY_KEY, delay.toMillis());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@
import com.google.longrunning.Operation.ResultCase;
import com.google.protobuf.Any;
import com.google.rpc.Status;
import java.time.Duration;
import java.util.concurrent.ExecutionException;
import javax.annotation.Nullable;
import org.threeten.bp.Duration;

/**
* Represents a long-running operation.
Expand All @@ -43,11 +43,11 @@ public class Operation<R, M> {

private final RetrySettings DEFAULT_OPERATION_WAIT_SETTINGS =
RetrySettings.newBuilder()
.setTotalTimeout(Duration.ofHours(12L))
.setInitialRetryDelay(Duration.ofMillis(500L))
.setTotalTimeoutDuration(Duration.ofHours(12L))
.setInitialRetryDelayDuration(Duration.ofMillis(500L))
.setRetryDelayMultiplier(1.0)
.setJittered(false)
.setMaxRetryDelay(Duration.ofMinutes(500L))
.setMaxRetryDelayDuration(Duration.ofMinutes(500L))
.build();

interface Parser<R, M> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,12 @@
import com.google.spanner.v1.TransactionOptions;
import com.google.spanner.v1.TransactionSelector;
import io.grpc.Status;
import java.time.Duration;
import java.time.temporal.ChronoUnit;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.logging.Level;
import java.util.logging.Logger;
import org.threeten.bp.Duration;
import org.threeten.bp.temporal.ChronoUnit;

@InternalApi
public class PartitionedDmlTransaction implements SessionImpl.SessionTransaction {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,13 @@
import com.google.spanner.v1.RequestOptions;
import com.google.spanner.v1.Transaction;
import com.google.spanner.v1.TransactionOptions;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ExecutionException;
import javax.annotation.Nullable;
import org.threeten.bp.Instant;

/**
* Implementation of {@link Session}. Sessions are managed internally by the client library, and
Expand Down Expand Up @@ -203,7 +203,7 @@ public long executePartitionedUpdate(Statement stmt, UpdateOption... options) {
PartitionedDmlTransaction txn =
new PartitionedDmlTransaction(this, spanner.getRpc(), Ticker.systemTicker());
return txn.executeStreamingPartitionedUpdate(
stmt, spanner.getOptions().getPartitionedDmlTimeout(), options);
stmt, spanner.getOptions().getPartitionedDmlTimeoutDuration(), options);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@
import io.opentelemetry.api.metrics.Meter;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.time.Duration;
import java.time.Instant;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.Iterator;
Expand All @@ -115,8 +117,6 @@
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import javax.annotation.concurrent.GuardedBy;
import org.threeten.bp.Duration;
import org.threeten.bp.Instant;

/**
* Maintains a pool of sessions. This class itself is thread safe and is meant to be used
Expand Down Expand Up @@ -2079,7 +2079,7 @@ private void removeIdleSessions(Instant currTime) {
// Determine the minimum last use time for a session to be deemed to still be alive. Remove
// all sessions that have a lastUseTime before that time, unless it would cause us to go
// below MinSessions.
Instant minLastUseTime = currTime.minus(options.getRemoveInactiveSessionAfter());
Instant minLastUseTime = currTime.minus(options.getRemoveInactiveSessionAfterDuration());
Iterator<PooledSession> iterator = sessions.descendingIterator();
while (iterator.hasNext()) {
PooledSession session = iterator.next();
Expand Down
Loading

0 comments on commit 8a7d533

Please sign in to comment.