Skip to content

Commit 11733d8

Browse files
jcferrettijcferretti
committed
Do not retry by default non-idempotent operations. Fixes etcd-io#1444
Also use the chance to improve javadoc on some *Options objects Signed-off-by: Cristian Ferretti <jcferretti2020@gmail.com>
1 parent a240dc6 commit 11733d8

File tree

14 files changed

+323
-37
lines changed

14 files changed

+323
-37
lines changed

‎jetcd-core/src/main/java/io/etcd/jetcd/KV.java

+9
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import io.etcd.jetcd.options.DeleteOption;
2727
import io.etcd.jetcd.options.GetOption;
2828
import io.etcd.jetcd.options.PutOption;
29+
import io.etcd.jetcd.options.TxnOption;
2930
import io.etcd.jetcd.support.CloseableClient;
3031

3132
/**
@@ -115,4 +116,12 @@ public interface KV extends CloseableClient {
115116
* @return a Txn
116117
*/
117118
Txn txn();
119+
120+
/**
121+
* creates a transaction.
122+
*
123+
* @param option TxnOption
124+
* @return a Txn
125+
*/
126+
Txn txn(TxnOption option);
118127
}

‎jetcd-core/src/main/java/io/etcd/jetcd/impl/ElectionImpl.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ public CompletableFuture<CampaignResponse> campaign(ByteSequence electionName, l
8888
execute(
8989
() -> stubWithLeader().campaign(request),
9090
CampaignResponse::new,
91-
Errors::isRetryable));
91+
Errors::isRetryableForNoSafeRedoOp));
9292
}
9393

9494
@Override
@@ -111,7 +111,7 @@ public CompletableFuture<ProclaimResponse> proclaim(LeaderKey leaderKey, ByteSeq
111111
execute(
112112
() -> stubWithLeader().proclaim(request),
113113
ProclaimResponse::new,
114-
Errors::isRetryable));
114+
Errors::isRetryableForNoSafeRedoOp));
115115
}
116116

117117
@Override
@@ -126,7 +126,7 @@ public CompletableFuture<LeaderResponse> leader(ByteSequence electionName) {
126126
execute(
127127
() -> stubWithLeader().leader(request),
128128
response -> new LeaderResponse(response, namespace),
129-
Errors::isRetryable));
129+
Errors::isRetryableForNoSafeRedoOp));
130130
}
131131

132132
@Override
@@ -162,7 +162,7 @@ public CompletableFuture<ResignResponse> resign(LeaderKey leaderKey) {
162162
execute(
163163
() -> stubWithLeader().resign(request),
164164
ResignResponse::new,
165-
Errors::isRetryable));
165+
Errors::isRetryableForNoSafeRedoOp));
166166
}
167167

168168
private <S> CompletableFuture<S> wrapConvertException(CompletableFuture<S> future) {

‎jetcd-core/src/main/java/io/etcd/jetcd/impl/Impl.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,11 @@ protected <S> CompletableFuture<S> completable(
8989
*/
9090
protected <S, T> CompletableFuture<T> execute(
9191
Supplier<Future<S>> supplier,
92-
Function<S, T> resultConvert) {
92+
Function<S, T> resultConvert,
93+
boolean autoRetry) {
9394

94-
return execute(supplier, resultConvert, Errors::isRetryable);
95+
return execute(supplier, resultConvert,
96+
autoRetry ? Errors::isRetryableForSafeRedoOp : Errors::isRetryableForNoSafeRedoOp);
9597
}
9698

9799
/**

‎jetcd-core/src/main/java/io/etcd/jetcd/impl/KVImpl.java

+12-9
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,7 @@
2929
import io.etcd.jetcd.kv.PutResponse;
3030
import io.etcd.jetcd.kv.TxnResponse;
3131
import io.etcd.jetcd.op.TxnImpl;
32-
import io.etcd.jetcd.options.CompactOption;
33-
import io.etcd.jetcd.options.DeleteOption;
34-
import io.etcd.jetcd.options.GetOption;
35-
import io.etcd.jetcd.options.PutOption;
32+
import io.etcd.jetcd.options.*;
3633
import io.etcd.jetcd.support.Errors;
3734
import io.etcd.jetcd.support.Requests;
3835

@@ -65,7 +62,7 @@ public CompletableFuture<PutResponse> put(ByteSequence key, ByteSequence value,
6562
return execute(
6663
() -> stub.put(Requests.mapPutRequest(key, value, option, namespace)),
6764
response -> new PutResponse(response, namespace),
68-
Errors::isRetryable);
65+
option.isAutoRetry() ? Errors::isRetryableForSafeRedoOp : Errors::isRetryableForNoSafeRedoOp);
6966
}
7067

7168
@Override
@@ -81,7 +78,7 @@ public CompletableFuture<GetResponse> get(ByteSequence key, GetOption option) {
8178
return execute(
8279
() -> stub.range(Requests.mapRangeRequest(key, option, namespace)),
8380
response -> new GetResponse(response, namespace),
84-
Errors::isRetryable);
81+
Errors::isRetryableForSafeRedoOp);
8582
}
8683

8784
@Override
@@ -97,7 +94,7 @@ public CompletableFuture<DeleteResponse> delete(ByteSequence key, DeleteOption o
9794
return execute(
9895
() -> stub.deleteRange(Requests.mapDeleteRequest(key, option, namespace)),
9996
response -> new DeleteResponse(response, namespace),
100-
Errors::isRetryable);
97+
option.isAutoRetry() ? Errors::isRetryableForSafeRedoOp : Errors::isRetryableForNoSafeRedoOp);
10198
}
10299

103100
@Override
@@ -116,15 +113,21 @@ public CompletableFuture<CompactResponse> compact(long rev, CompactOption option
116113
return execute(
117114
() -> stub.compact(request),
118115
CompactResponse::new,
119-
Errors::isRetryable);
116+
Errors::isRetryableForSafeRedoOp);
120117
}
121118

122119
@Override
123120
public Txn txn() {
121+
return txn(TxnOption.DEFAULT);
122+
}
123+
124+
@Override
125+
public Txn txn(TxnOption option) {
124126
return TxnImpl.newTxn(
125127
request -> execute(
126128
() -> stub.txn(request),
127-
response -> new TxnResponse(response, namespace), Errors::isRetryable),
129+
response -> new TxnResponse(response, namespace),
130+
option.isAutoRetry() ? Errors::isRetryableForSafeRedoOp : Errors::isRetryableForNoSafeRedoOp),
128131
namespace);
129132
}
130133
}

‎jetcd-core/src/main/java/io/etcd/jetcd/impl/LeaseImpl.java

+8-4
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,8 @@ public CompletableFuture<LeaseGrantResponse> grant(long ttl) {
8484
LeaseGrantRequest.newBuilder()
8585
.setTTL(ttl)
8686
.build()),
87-
LeaseGrantResponse::new);
87+
LeaseGrantResponse::new,
88+
true);
8889
}
8990

9091
@Override
@@ -94,7 +95,8 @@ public CompletableFuture<LeaseGrantResponse> grant(long ttl, long timeout, TimeU
9495
LeaseGrantRequest.newBuilder()
9596
.setTTL(ttl)
9697
.build()),
97-
LeaseGrantResponse::new);
98+
LeaseGrantResponse::new,
99+
true);
98100
}
99101

100102
@Override
@@ -104,7 +106,8 @@ public CompletableFuture<LeaseRevokeResponse> revoke(long leaseId) {
104106
LeaseRevokeRequest.newBuilder()
105107
.setID(leaseId)
106108
.build()),
107-
LeaseRevokeResponse::new);
109+
LeaseRevokeResponse::new,
110+
true);
108111
}
109112

110113
@Override
@@ -118,7 +121,8 @@ public CompletableFuture<LeaseTimeToLiveResponse> timeToLive(long leaseId, Lease
118121

119122
return execute(
120123
() -> this.stub.leaseTimeToLive(leaseTimeToLiveRequest),
121-
LeaseTimeToLiveResponse::new);
124+
LeaseTimeToLiveResponse::new,
125+
true);
122126
}
123127

124128
@Override

‎jetcd-core/src/main/java/io/etcd/jetcd/impl/LockImpl.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public CompletableFuture<LockResponse> lock(ByteSequence name, long leaseId) {
6969
return execute(
7070
() -> stubWithLeader().lock(request),
7171
response -> new LockResponse(response, namespace),
72-
Errors::isRetryable);
72+
Errors::isRetryableForSafeRedoOp);
7373
}
7474

7575
@Override
@@ -83,6 +83,6 @@ public CompletableFuture<UnlockResponse> unlock(ByteSequence lockKey) {
8383
return execute(
8484
() -> stubWithLeader().unlock(request),
8585
UnlockResponse::new,
86-
Errors::isRetryable);
86+
Errors::isRetryableForSafeRedoOp);
8787
}
8888
}

‎jetcd-core/src/main/java/io/etcd/jetcd/kv/GetResponse.java

+3
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,9 @@ public boolean isMore() {
5959

6060
/**
6161
* Returns the number of keys within the range when requested.
62+
* Note this value is never affected by filtering options (limit, min or max created or modified revisions)
63+
* Count is the count for keys on the range part of a request. Filters restrict the
64+
* returned values from that range, but not the count.
6265
*
6366
* @return count.
6467
*/

‎jetcd-core/src/main/java/io/etcd/jetcd/options/DeleteOption.java

+39-2
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,13 @@ public final class DeleteOption {
2929
private final ByteSequence endKey;
3030
private final boolean prevKV;
3131
private final boolean prefix;
32+
private final boolean autoRetry;
3233

33-
private DeleteOption(ByteSequence endKey, boolean prevKV, boolean prefix) {
34+
private DeleteOption(ByteSequence endKey, boolean prevKV, boolean prefix, final boolean autoRetry) {
3435
this.endKey = endKey;
3536
this.prevKV = prevKV;
3637
this.prefix = prefix;
38+
this.autoRetry = autoRetry;
3739
}
3840

3941
public Optional<ByteSequence> getEndKey() {
@@ -49,10 +51,25 @@ public boolean isPrevKV() {
4951
return prevKV;
5052
}
5153

54+
/**
55+
* Whether to treat this deletion as deletion by prefix
56+
*
57+
* @return true if deletion by prefix.
58+
*/
5259
public boolean isPrefix() {
5360
return prefix;
5461
}
5562

63+
/**
64+
* Whether to treat a delete operation as idempotent from the point of view of automated retries.
65+
* Note under failure scenarios this may mean a single delete is attempted more than once.
66+
*
67+
* @return true if automated retries should happen.
68+
*/
69+
public boolean isAutoRetry() {
70+
return autoRetry;
71+
}
72+
5673
/**
5774
* Returns the builder.
5875
*
@@ -65,6 +82,11 @@ public static Builder newBuilder() {
6582
return builder();
6683
}
6784

85+
/**
86+
* Returns the builder.
87+
*
88+
* @return the builder
89+
*/
6890
public static Builder builder() {
6991
return new Builder();
7092
}
@@ -73,6 +95,7 @@ public static final class Builder {
7395
private ByteSequence endKey;
7496
private boolean prevKV = false;
7597
private boolean prefix = false;
98+
private boolean autoRetry = false;
7699

77100
private Builder() {
78101
}
@@ -144,8 +167,22 @@ public Builder withPrevKV(boolean prevKV) {
144167
return this;
145168
}
146169

170+
/**
171+
* When autoRetry is set, the delete operation is treated as idempotent from the point of view of automated retries.
172+
* Note under some failure scenarios true may make a delete operation be attempted more than once, where
173+
* a first attempt succeeded but its result did not reach the client; by default (autoRetry=false),
174+
* the client won't retry since it is not safe to assume on such a failure the operation did not happen.
175+
* Requesting withAutoRetry means the client is explicitly asking for retry nevertheless.
176+
*
177+
* @return builder
178+
*/
179+
public Builder withAutoRetry() {
180+
this.autoRetry = true;
181+
return this;
182+
}
183+
147184
public DeleteOption build() {
148-
return new DeleteOption(endKey, prevKV, prefix);
185+
return new DeleteOption(endKey, prevKV, prefix, autoRetry);
149186
}
150187

151188
}

0 commit comments

Comments
 (0)