Skip to content
This repository has been archived by the owner on Nov 14, 2024. It is now read-only.

deprecate old executor usage; allow for specifying your own #2457

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
import com.palantir.lock.v2.LockToken;
import com.palantir.lock.v2.TimelockService;

public class LockRefresher implements AutoCloseable {
public class LockRefresher {
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this an api break?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

depends when it gets merged... I added it in yesterday


private final Logger log = LoggerFactory.getLogger(LockRefresher.class);

Expand Down Expand Up @@ -76,9 +76,4 @@ public void registerLock(LockToken token) {
public void unregisterLocks(Collection<LockToken> tokens) {
tokensToRefresh.removeAll(tokens);
}

@Override
public void close() throws Exception {
executor.shutdown();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@

package com.palantir.lock.client;

import java.util.Optional;
import java.util.Set;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.util.concurrent.ThreadFactoryBuilder;
import com.palantir.lock.v2.LockImmutableTimestampRequest;
import com.palantir.lock.v2.LockImmutableTimestampResponse;
Expand All @@ -39,19 +41,47 @@ public class LockRefreshingTimelockService implements AutoCloseable, TimelockSer

private final TimelockService delegate;
private final LockRefresher lockRefresher;

private final Optional<ScheduledExecutorService> managedExecutor;

/**
* @deprecated Use {@link #create(TimelockService, ScheduledExecutorService)} instead.
*
* @param timelockService The {@link TimelockService} to wrap
* @return A {@link TimelockService} that automatically refreshes locks
*/
@Deprecated
public static LockRefreshingTimelockService createDefault(TimelockService timelockService) {
ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor(new ThreadFactoryBuilder()
.setNameFormat(LockRefreshingTimelockService.class.getSimpleName() + "-%d")
.setDaemon(true)
.build());
LockRefresher lockRefresher = new LockRefresher(executor, timelockService, REFRESH_INTERVAL_MILLIS);
return new LockRefreshingTimelockService(timelockService, lockRefresher);
return new LockRefreshingTimelockService(timelockService, lockRefresher, executor);
}

/**
* Creates a {@link TimelockService} that uses the specified executor to regularly refresh
* locks that have been taken out.
*
* @param timelockService The {@link TimelockService} proxy to wrap
* @param executor An executor service whose lifecycle is managed by the consumer of this method
* @return The {@link TimelockService} that automatically refreshes locks
*/
public static LockRefreshingTimelockService create(
TimelockService timelockService,
ScheduledExecutorService executor) {
LockRefresher lockRefresher = new LockRefresher(executor, timelockService, REFRESH_INTERVAL_MILLIS);
return new LockRefreshingTimelockService(timelockService, lockRefresher, null);
}

public LockRefreshingTimelockService(TimelockService delegate, LockRefresher lockRefresher) {
@VisibleForTesting
LockRefreshingTimelockService(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is technically an API break as it's no longer public, but I think that's fine?

TimelockService delegate,
LockRefresher lockRefresher,
ScheduledExecutorService executor) {
this.delegate = delegate;
this.lockRefresher = lockRefresher;
this.managedExecutor = Optional.ofNullable(executor);
}

@Override
Expand Down Expand Up @@ -108,6 +138,6 @@ public long currentTimeMillis() {

@Override
public void close() throws Exception {
lockRefresher.close();
managedExecutor.ifPresent(ScheduledExecutorService::shutdown);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public class LockRefreshingTimelockServiceTest {

private final LockRefresher refresher = mock(LockRefresher.class);
private final TimelockService delegate = mock(TimelockService.class);
private final TimelockService timelock = new LockRefreshingTimelockService(delegate, refresher);
private final TimelockService timelock = new LockRefreshingTimelockService(delegate, refresher, null);

private static final long TIMEOUT = 10_000;

Expand Down