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

fix: only replace refresh result if successful or current result is invalid #561

Merged
merged 10 commits into from
Jul 26, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -301,9 +301,6 @@ String getPreferredIp(List<String> preferredTypes) {
* @return {@code true} if successfully scheduled, or {@code false} otherwise.
*/
boolean forceRefresh() {
if (!forcedRenewRateLimiter.tryAcquire()) {
return false;
}
synchronized (instanceDataGuard) {
// If a scheduled refresh hasn't started, perform one immediately
if (nextInstanceData.cancel(false)) {
Expand All @@ -323,6 +320,8 @@ boolean forceRefresh() {
* would expire.
*/
private ListenableFuture<InstanceData> performRefresh() {
// To avoid unreasonable SQL Admin API usage, use a rate limit to throttle our usage.
forcedRenewRateLimiter.acquire(1);
shubha-rajan marked this conversation as resolved.
Show resolved Hide resolved
// Use the Cloud SQL Admin API to return the Metadata and Certificate
ListenableFuture<Metadata> metadataFuture = executor.submit(this::fetchMetadata);
ListenableFuture<Certificate> ephemeralCertificateFuture =
Expand Down Expand Up @@ -364,18 +363,40 @@ private ListenableFuture<InstanceData> performRefresh() {
executor,
metadataFuture,
sslContextFuture);
refreshFuture.addListener(
() -> {
synchronized (instanceDataGuard) {
// update currentInstanceData with the most recent results
currentInstanceData = refreshFuture;
// schedule a replacement before the SSLContext expires;
nextInstanceData = executor
.schedule(this::performRefresh, secondsUntilRefresh(),
TimeUnit.SECONDS);
Futures.addCallback(refreshFuture,
new FutureCallback<InstanceData>() {
public void onSuccess(InstanceData instanceData) {
synchronized (instanceDataGuard) {
// update currentInstanceData with the most recent results
currentInstanceData = refreshFuture;
// schedule a replacement before the SSLContext expires;
nextInstanceData = executor
.schedule(() -> performRefresh(),
secondsUntilRefresh(),
TimeUnit.SECONDS);
}
}
},
executor);

public void onFailure(Throwable t) {
logger.log(Level.WARNING,
"An error occurred while performing refresh. Retrying immediately.", t);
kurtisvg marked this conversation as resolved.
Show resolved Hide resolved
synchronized (instanceDataGuard) {
try {
InstanceData instanceData = getInstanceData();
Instant expiration = instanceData.getExpiration().toInstant();
if (expiration.isBefore(Instant.now())) {
// replace current if it is expired
currentInstanceData = refreshFuture;
}
} catch (Exception e) {
// replace current if accessing it throws an error
currentInstanceData = refreshFuture;
}
shubha-rajan marked this conversation as resolved.
Show resolved Hide resolved
nextInstanceData = Futures.immediateFuture(performRefresh());
}
}
}, executor);

return refreshFuture;
}

Expand Down