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

The 0.7.6 version does not throw an exception when keepAliveOnce failed #1256

Merged
merged 1 commit into from
Oct 23, 2023
Merged
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
19 changes: 16 additions & 3 deletions jetcd-core/src/main/java/io/etcd/jetcd/impl/LeaseImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,11 @@
import java.util.concurrent.atomic.AtomicReference;

import io.etcd.jetcd.Lease;
import io.etcd.jetcd.api.*;
import io.etcd.jetcd.api.LeaseGrantRequest;
import io.etcd.jetcd.api.LeaseKeepAliveRequest;
import io.etcd.jetcd.api.LeaseRevokeRequest;
import io.etcd.jetcd.api.LeaseTimeToLiveRequest;
import io.etcd.jetcd.api.VertxLeaseGrpc;
import io.etcd.jetcd.common.Service;
import io.etcd.jetcd.common.exception.ErrorCode;
import io.etcd.jetcd.lease.LeaseGrantResponse;
Expand All @@ -40,7 +44,9 @@
import io.grpc.stub.StreamObserver;
import io.vertx.core.streams.WriteStream;

import static io.etcd.jetcd.common.exception.EtcdExceptionFactory.*;
import static io.etcd.jetcd.common.exception.EtcdExceptionFactory.newClosedLeaseClientException;
import static io.etcd.jetcd.common.exception.EtcdExceptionFactory.newEtcdException;
import static io.etcd.jetcd.common.exception.EtcdExceptionFactory.toEtcdException;
import static java.util.Objects.requireNonNull;

/**
Expand Down Expand Up @@ -146,7 +152,14 @@ public CompletableFuture<LeaseKeepAliveResponse> keepAliveOnce(long leaseId) {
ref.set(s);
s.write(req);
})
.handler(r -> future.complete(new LeaseKeepAliveResponse(r)))
.handler(r -> {
if (r.getTTL() != 0) {
future.complete(new LeaseKeepAliveResponse(r));
} else {
future.completeExceptionally(
newEtcdException(ErrorCode.NOT_FOUND, "etcdserver: requested lease not found"));
}
})
.exceptionHandler(future::completeExceptionally);

return future.whenComplete((r, t) -> ref.get().end(req));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public class LeaseMemoryLeakTest {
private KV kvClient;
private Lease leaseClient;

private static final Logger LOGGER = LoggerFactory.getLogger(LeaseMemoryLeakTest.class);
private static final Logger LOGGER = LoggerFactory.getLogger(LeaseOnceErrorTest.class);
private static final long TTL = 10;
private static final int ITERATIONS = 5;
private static final Pattern GO_ROUTINES_EXTRACT_PATTERN = Pattern.compile("go_goroutines (\\d+)");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* Copyright 2016-2021 The jetcd authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.etcd.jetcd.impl;

import java.util.concurrent.TimeUnit;

import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Timeout;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.etcd.jetcd.Client;
import io.etcd.jetcd.Lease;
import io.etcd.jetcd.common.exception.EtcdException;
import io.etcd.jetcd.lease.LeaseKeepAliveResponse;
import io.etcd.jetcd.lease.LeaseRevokeResponse;
import io.etcd.jetcd.test.EtcdClusterExtension;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

@Timeout(value = 45, unit = TimeUnit.SECONDS)
public class LeaseOnceErrorTest {

@RegisterExtension
public static final EtcdClusterExtension CLUSTER = EtcdClusterExtension.builder()
.withNodes(1)
.build();

private Client client;
private Lease leaseClient;

private static final long TTL = 2;

@BeforeEach
public void setUp() {
client = TestUtil.client(CLUSTER).build();
leaseClient = client.getLeaseClient();
}

@AfterEach
public void tearDown() {
if (client != null) {
client.close();
}
}

// https://github.com/etcd-io/jetcd/issues/1254
@Test
public void testKeepAliveError() throws Exception {
final long leaseID = leaseClient.grant(TTL).get(1, TimeUnit.SECONDS).getID();

LeaseKeepAliveResponse ka1 = leaseClient.keepAliveOnce(leaseID).get(1, TimeUnit.SECONDS);
assertThat(ka1.getTTL()).isNotZero();

LeaseRevokeResponse r1 = leaseClient.revoke(leaseID).get(1, TimeUnit.SECONDS);
assertThat(r1).isNotNull();

assertThatThrownBy(() -> leaseClient.keepAliveOnce(leaseID).get(1, TimeUnit.SECONDS))
.hasCauseInstanceOf(EtcdException.class)
.hasMessageContaining("etcdserver: requested lease not found");
}
}
7 changes: 7 additions & 0 deletions jetcd-launcher/src/main/java/io/etcd/jetcd/launcher/Etcd.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ public static class Builder {
private String prefix;
private int nodes = 1;
private boolean ssl = false;
private boolean debug = false;
private List<String> additionalArgs;
private Network network;
private boolean shouldMountDataDirectory = true;
Expand All @@ -78,6 +79,11 @@ public Builder withSsl(boolean ssl) {
return this;
}

public Builder withDebug(boolean debug) {
this.debug = debug;
return this;
}

public Builder withAdditionalArgs(Collection<String> additionalArgs) {
this.additionalArgs = Collections.unmodifiableList(new ArrayList<>(additionalArgs));
return this;
Expand Down Expand Up @@ -105,6 +111,7 @@ public EtcdCluster build() {
prefix,
nodes,
ssl,
debug,
additionalArgs,
network != null ? network : Network.newNetwork(),
shouldMountDataDirectory);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public EtcdClusterImpl(
String prefix,
int nodes,
boolean ssl,
boolean debug,
Collection<String> additionalArgs,
Network network,
boolean shouldMountDataDirectory) {
Expand All @@ -53,6 +54,7 @@ public EtcdClusterImpl(
.map(e -> new EtcdContainer(image, e, endpoints)
.withClusterToken(clusterName)
.withSll(ssl)
.withDebug(debug)
.withAdditionalArgs(additionalArgs)
.withNetwork(network)
.withShouldMountDataDirectory(shouldMountDataDirectory))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,11 @@ public Builder withSsl(boolean ssl) {
return this;
}

public Builder withDebug(boolean debug) {
builder.withDebug(debug);
return this;
}

public Builder withAdditionalArgs(Collection<String> additionalArgs) {
builder.withAdditionalArgs(additionalArgs);
return this;
Expand Down