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: long running transaction clean up background task. Adding configuration options for closing inactive transactions. #2419

Merged
merged 57 commits into from
Aug 1, 2023
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
edc5bbf
fix: prevent illegal negative timeout values into thread sleep() meth…
arpan14 Feb 6, 2023
49a85df
Merge pull request #1 from arpan14/retryerror
arpan14 Feb 8, 2023
4cd497b
Fixing lint issues.
arpan14 Feb 8, 2023
4a6aa8e
Merge branch 'googleapis:main' into main
arpan14 Mar 13, 2023
b2aa09d
Merge branch 'googleapis:main' into main
arpan14 Mar 15, 2023
8d6d71e
Merge branch 'googleapis:main' into main
arpan14 May 9, 2023
34502d1
feat: long running transaction clean up background task. Adding confi…
arpan14 Apr 21, 2023
660cbcf
fix: linting issues and tests.
arpan14 May 9, 2023
82f9a09
fix: avoid refactoring existing tests.
arpan14 May 9, 2023
65a42f7
fix:lint issues and unit tests.
arpan14 May 9, 2023
d76b0b1
docs:adding documentation.
arpan14 May 10, 2023
fcf1565
fix:linting issues.
arpan14 May 10, 2023
7858d7c
Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Se…
arpan14 Jun 6, 2023
342eed8
Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Se…
arpan14 Jun 6, 2023
227dd27
Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Se…
arpan14 Jun 6, 2023
ab05e08
Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Se…
arpan14 Jun 6, 2023
3f07bf6
Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Se…
arpan14 Jun 6, 2023
cbc93e6
Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Se…
arpan14 Jun 6, 2023
5f36519
Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Se…
arpan14 Jun 6, 2023
cf5f8a3
Apply suggestions from code review
arpan14 Jun 6, 2023
3469bed
fix:comments on PR.
arpan14 Jun 6, 2023
c27eaa9
fix:rename variables.
arpan14 Jun 6, 2023
ded6d75
fix:variable name rename.
arpan14 Jun 6, 2023
b4b4033
Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Se…
arpan14 Jun 6, 2023
4423664
Revert "Update google-cloud-spanner/src/main/java/com/google/cloud/sp…
arpan14 Jun 6, 2023
a9da9dd
fix:variable name
arpan14 Jun 6, 2023
6239429
fix:removed lock.
arpan14 Jun 12, 2023
acdc9ac
fix:add handling to prevent duplicate session leak logs.
arpan14 Jun 13, 2023
480c719
fix:avoid long delays in integration tests.
arpan14 Jun 19, 2023
019e100
fix:lint issues.
arpan14 Jun 19, 2023
ff144ae
fix:pr comments around pool options, mock spanner.
arpan14 Jun 20, 2023
efe888f
fix: cleaned up session being returned back to the pool.
arpan14 Jun 22, 2023
d0d2c91
fix:mock timers to avoid flaky tests.
arpan14 Jun 25, 2023
9f0a100
fix:flaky tests.
arpan14 Jun 25, 2023
b96e940
fix:flaky tests.
arpan14 Jun 25, 2023
ecd2f4b
feat: support a new option to silently close inactive transactions wi…
arpan14 Jun 25, 2023
6b72583
fix:variable names.
arpan14 Jun 26, 2023
8ca552b
Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Se…
arpan14 Jun 28, 2023
ed84ad1
Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Se…
arpan14 Jun 28, 2023
a1b4b5f
Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Se…
arpan14 Jun 28, 2023
c011d4c
Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Se…
arpan14 Jun 28, 2023
6c76a05
Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Se…
arpan14 Jun 28, 2023
dee86d0
fix:review comments.
arpan14 Jul 4, 2023
b3b45e2
Merge branch 'googleapis:main' into session-leak-pr
arpan14 Jul 6, 2023
e82c54f
Merge branch 'googleapis:main' into session-leak-pr
arpan14 Jul 10, 2023
c3e39ff
fix:add more unit tests for session pool options.
arpan14 Jul 10, 2023
db37f9d
Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/Se…
arpan14 Jul 11, 2023
4edaa52
docs:update as per PR comment.
arpan14 Jul 11, 2023
416ca61
Merge branch 'googleapis:main' into session-leak-pr
arpan14 Jul 18, 2023
ee3ec84
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Jul 25, 2023
943c2c5
Merge branch 'googleapis:main' into session-leak-pr
arpan14 Jul 25, 2023
a4faf05
fix: solve race conditions for PDML transactions.
arpan14 Jul 25, 2023
3f1e752
fix: make method scope to be package-private.
arpan14 Jul 25, 2023
d1f599e
test: add benchmark for long-running sessions clean up task.
arpan14 Jul 25, 2023
c41fbd7
fix: disable the feature by default.
arpan14 Aug 1, 2023
ea39b03
Merge branch 'googleapis:main' into session-leak-pr
arpan14 Aug 1, 2023
d9ce080
fix: remove benchmark code.
arpan14 Aug 1, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
import com.google.cloud.spanner.Options.TransactionOption;
import com.google.cloud.spanner.Options.UpdateOption;
import com.google.cloud.spanner.SessionClient.SessionConsumer;
import com.google.cloud.spanner.SessionPoolOptions.InactiveTransactionRemovalOptions;
import com.google.cloud.spanner.SpannerException.ResourceNotFoundException;
import com.google.cloud.spanner.SpannerImpl.ClosedException;
import com.google.common.annotations.VisibleForTesting;
Expand Down Expand Up @@ -1365,6 +1366,7 @@ final class PooledSession implements Session {
private volatile Instant lastUseTime;
private volatile SpannerException lastException;
private volatile boolean allowReplacing = true;
private volatile boolean isLongRunning = false;

@GuardedBy("lock")
private SessionState state;
Expand All @@ -1385,6 +1387,11 @@ void setAllowReplacing(boolean allowReplacing) {
this.allowReplacing = allowReplacing;
}

@VisibleForTesting
void setIsLongRunning(boolean isLongRunning) {
this.isLongRunning = isLongRunning;
}

@Override
public Timestamp write(Iterable<Mutation> mutations) throws SpannerException {
return writeWithOptions(mutations).getCommitTimestamp();
Expand Down Expand Up @@ -1422,6 +1429,7 @@ public long executePartitionedUpdate(Statement stmt, UpdateOption... options)
throws SpannerException {
try {
markUsed();
markLongRunning();
return delegate.executePartitionedUpdate(stmt, options);
} catch (SpannerException e) {
throw lastException = e;
Expand Down Expand Up @@ -1572,6 +1580,14 @@ void markUsed() {
lastUseTime = clock.instant();
}

/**
* Method to mark a session occupied by a long-running transaction. Any transaction that is
* expected to be long-running (for ex - Partitioned DML, Batch Read) must use this method.
*/
void markLongRunning() {
isLongRunning = true;
}

@Override
public TransactionManager transactionManager(TransactionOption... options) {
return delegate.transactionManager(options);
Expand Down Expand Up @@ -1651,6 +1667,10 @@ private PooledSession pollUninterruptiblyWithTimeout(long timeoutMillis) {
* <li>Keeps alive sessions that have not been used for a user configured time in order to keep
* MinSessions sessions alive in the pool at any time. The keep-alive traffic is smeared out
* over a window of 10 minutes to avoid bursty traffic.
* <li>Removes unexpected long running transactions from the pool. Only certain transaction
arpan14 marked this conversation as resolved.
Show resolved Hide resolved
arpan14 marked this conversation as resolved.
Show resolved Hide resolved
* types (for ex - Partitioned DML / Batch Reads) can be long running. This tasks checks the
* sessions which have been executing for a longer than usual duration (60 minutes) and
* returns such sessions back to the pool.
* </ul>
*/
final class PoolMaintainer {
Expand All @@ -1659,16 +1679,24 @@ final class PoolMaintainer {
private final Duration windowLength = Duration.ofMillis(TimeUnit.MINUTES.toMillis(10));
// Frequency of the timer loop.
@VisibleForTesting final long loopFrequency = options.getLoopFrequency();
// Number of loop iterations in which we need to to close all the sessions waiting for closure.
// Number of loop iterations in which we need to close all the sessions waiting for closure.
@VisibleForTesting final long numClosureCycles = windowLength.toMillis() / loopFrequency;
private final Duration keepAliveMillis =
Duration.ofMillis(TimeUnit.MINUTES.toMillis(options.getKeepAliveIntervalMinutes()));
// Number of loop iterations in which we need to keep alive all the sessions
@VisibleForTesting final long numKeepAliveCycles = keepAliveMillis.toMillis() / loopFrequency;

Instant lastResetTime = Instant.ofEpochMilli(0);
int numSessionsToClose = 0;
int sessionsToClosePerLoop = 0;
/**
* Variable maintaining the last execution time of the long-running transaction cleanup task.
*
* <p>The long-running transaction cleanup needs to be performed every X minutes. The X minutes
* recurs multiple times within the invocation of the pool maintainer thread. For ex - If the
* main thread runs every 10s and the long-running transaction clean-up needs to be performed
* every 2 minutes, then we need to keep a track of when was the last time that this task
* executed and makes sure we only execute it every 2 minutes and not every 10 seconds.
*/
@VisibleForTesting public volatile Instant lastExecutionTime;
arpan14 marked this conversation as resolved.
Show resolved Hide resolved

boolean closed = false;

@GuardedBy("lock")
Expand All @@ -1678,6 +1706,7 @@ final class PoolMaintainer {
boolean running;

void init() {
lastExecutionTime = clock.instant();
// Scheduled pool maintenance worker.
synchronized (lock) {
scheduledFuture =
Expand Down Expand Up @@ -1723,6 +1752,7 @@ void maintainPool() {
decrementPendingClosures(1);
}
}
closeLongRunningTransactions(currTime);
}

private void removeIdleSessions(Instant currTime) {
Expand All @@ -1736,7 +1766,13 @@ private void removeIdleSessions(Instant currTime) {
PooledSession session = iterator.next();
if (session.lastUseTime.isBefore(minLastUseTime)) {
if (session.state != SessionState.CLOSING) {
removeFromPool(session);
boolean isRemoved = removeFromPool(session);
if (isRemoved) {
numIdleSessionsRemoved++;
if (idleSessionRemovedListener != null) {
idleSessionRemovedListener.apply(session);
}
}
iterator.remove();
}
}
Expand Down Expand Up @@ -1792,6 +1828,60 @@ private void replenishPool() {
}
}
}

// cleans up transactions which are unexpectedly long-running.
void closeLongRunningTransactions(Instant currentTime) {
try {
synchronized (lock) {
if (SessionPool.this.isClosed()) {
return;
}
final InactiveTransactionRemovalOptions inactiveTransactionRemovalOptions =
options.getInactiveTransactionRemovalOptions();
final Instant minExecutionTime =
lastExecutionTime.plus(inactiveTransactionRemovalOptions.getInterval());
if (currentTime.isBefore(minExecutionTime)) {
return;
}
lastExecutionTime = currentTime; // update this only after we have decided to execute task
if (options.closeInactiveTransactions() || options.warnInactiveTransactions()) {
removeLongRunningSessions(currentTime, inactiveTransactionRemovalOptions);
}
}
} catch (final Throwable t) {
logger.log(Level.WARNING, "Failed removing long running transactions", t);
}
}

private void removeLongRunningSessions(
final Instant currentTime,
final InactiveTransactionRemovalOptions inactiveTransactionRemovalOptions) {
synchronized (lock) {
arpan14 marked this conversation as resolved.
Show resolved Hide resolved
final double usedSessionsRatio = getRatioOfSessionsInUse();
if (usedSessionsRatio > inactiveTransactionRemovalOptions.getUsedSessionsRatioThreshold()) {
Iterator<PooledSessionFuture> iterator = checkedOutSessions.iterator();
while (iterator.hasNext()) {
final PooledSessionFuture sessionFuture = iterator.next();
// the below get() call on future object is non-blocking since checkedOutSessions
// collection is populated only when the get() method in {@code PooledSessionFuture} is
// called.
final PooledSession session = sessionFuture.get();
final Duration durationFromLastUse = Duration.between(session.lastUseTime, currentTime);
if (!session.isLongRunning
&& durationFromLastUse.toMillis()
> inactiveTransactionRemovalOptions.getExecutionTimeThreshold().toMillis()) {
logger.log(
Level.WARNING, "Removing long running session", sessionFuture.leakedException);
numInactiveSessionsRemoved++;
arpan14 marked this conversation as resolved.
Show resolved Hide resolved
if (options.closeInactiveTransactions() && session.state != SessionState.CLOSING) {
removeFromPool(session);
iterator.remove();
}
}
}
}
}
}
}

private enum Position {
Expand Down Expand Up @@ -1872,6 +1962,9 @@ private enum Position {
@GuardedBy("lock")
private long numIdleSessionsRemoved = 0;

@GuardedBy("lock")
private long numInactiveSessionsRemoved = 0;

private AtomicLong numWaiterTimeouts = new AtomicLong();

@GuardedBy("lock")
Expand Down Expand Up @@ -2015,18 +2108,24 @@ int getNumberOfSessionsInUse() {
}
}

void removeFromPool(PooledSession session) {
@VisibleForTesting
double getRatioOfSessionsInUse() {
synchronized (lock) {
final int maxSessions = options.getMaxSessions();
if (maxSessions == 0) return 0;
arpan14 marked this conversation as resolved.
Show resolved Hide resolved
return (double) numSessionsInUse / maxSessions;
}
}

boolean removeFromPool(PooledSession session) {
synchronized (lock) {
if (isClosed()) {
decrementPendingClosures(1);
return;
return false;
}
session.markClosing();
allSessions.remove(session);
numIdleSessionsRemoved++;
}
if (idleSessionRemovedListener != null) {
asthamohta marked this conversation as resolved.
Show resolved Hide resolved
idleSessionRemovedListener.apply(session);
return true;
}
}

Expand All @@ -2036,6 +2135,13 @@ long numIdleSessionsRemoved() {
}
}

@VisibleForTesting
long numInactiveSessionsRemoved() {
synchronized (lock) {
return numInactiveSessionsRemoved;
}
}

@VisibleForTesting
int getNumberOfSessionsInPool() {
synchronized (lock) {
Expand Down
Loading