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: introduce java.time variables and methods #3495

Merged
merged 6 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -24,7 +24,6 @@
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,7 +108,7 @@ public void attemptCancelled() {
}

@Override
public void attemptFailed(Throwable error, Duration delay) {
public void attemptFailed(Throwable error, org.threeten.bp.Duration delay) {
lqiu96 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

@sakthivelmanii sakthivelmanii Nov 26, 2024

Choose a reason for hiding this comment

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

nit: Can we import java.time.Duration so that we can remove java.time. in attemptFailedDuration method? https://github.com/googleapis/java-spanner/blob/main/google-cloud-spanner/src/main/java/com/google/cloud/spanner/CompositeTracer.java#L119

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

for (ApiTracer child : children) {
child.attemptFailed(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();
java.time.Duration waitDuration = sessionPoolOptions.getWaitForMinSessions();
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: java.time.Duration is already imported in this class. Can't we just remove org.threeten.bp.

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 @@ -27,7 +30,6 @@
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 +165,18 @@ public void attemptCancelled() {
lastConnectionId = null;
}

/**
* This method is obsolete. Use {@link #attemptFailedDuration(Throwable, java.time.Duration)}
* instead.
*/
@Override
@ObsoleteApi("Use attemptFailedDuration(Throwable, java.time.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, java.time.Duration delay) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: Can we import java.time.Duration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

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
Loading