Skip to content

Commit 50397c6

Browse files
author
Carter Kozak
committed
propagate -> do-not-retry + javadoc
1 parent 0104881 commit 50397c6

File tree

3 files changed

+21
-20
lines changed

3 files changed

+21
-20
lines changed

errors/src/main/java/com/palantir/conjure/java/api/errors/QosReason.java

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -150,25 +150,26 @@ public QosReason build() {
150150
}
151151
}
152152

153+
/**
154+
* Conveys the servers opinion on whether a QoS failure should be retried, or propagate
155+
* back to the caller and result in a failure. There is no guarantee that these values
156+
* will be respected by all clients, and should be considered best-effort.
157+
*/
153158
public enum RetryHint {
154-
// TODO(ckozak): Should we declare the existing default value? Perhaps best if we only
155-
// declare the minimal api to opt into new behavior.
156-
// /** Clients should attempt to retry this failure. */
157-
// EAGER,
158159
/**
159160
* Clients should not attempt to retry this failure,
160161
* providing the failure as context back to the initial caller.
161162
*/
162-
PROPAGATE;
163+
DO_NOT_RETRY;
163164
}
164165

166+
/**
167+
* Describes the cause of a QoS failure when known to be non-default. By default, we assume that
168+
* a 503 Unavailable is the result of a node-wide limit being reached, and that a 429 is specific
169+
* to an individual endpoint on a node. These assumptions do not hold true in all cases, so
170+
* {@link DueTo} informs relays this intent.
171+
*/
165172
public enum DueTo {
166-
// TODO(ckozak): should we declare values that align with the default 503 and 429 behavior, which
167-
// impact rate limiters for the node and endpoint specifically? If we do, they will be propagated
168-
// in ways that don't make sense (despite being the default behavior today). Declaring a user limit
169-
// may be more reasonable since the user token is generally passed along, however per-endpoint limits
170-
// lose some meaning when rethrown by a calling service.
171-
172173
/**
173174
* A cause that the RPC system isn't directly aware of, for example a user or user-agent specific limit, or
174175
* based on a specific resource that's being accessed, as opposed to the target node as a whole, or endpoint.

errors/src/main/java/com/palantir/conjure/java/api/errors/QosReasons.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public final class QosReasons {
2828
private static final String RETRY_HINT_HEADER = "Qos-Retry-Hint";
2929

3030
private static final String DUE_TO_CUSTOM_HEADER_VALUE = "custom";
31-
private static final String RETRY_HINT_PROPAGATE_HEADER_VALUE = "propagate";
31+
private static final String RETRY_HINT_DO_NOT_RETRY_HEADER_VALUE = "do-not-retry";
3232

3333
public static <T> void encodeToResponse(
3434
QosReason reason, T response, QosResponseEncodingAdapter<? super T> adapter) {
@@ -76,8 +76,8 @@ static Optional<DueTo> parseDueTo(String dueTo) {
7676

7777
// VisibleForTesting
7878
static Optional<RetryHint> parseRetryHint(String retryHint) {
79-
if (RETRY_HINT_PROPAGATE_HEADER_VALUE.equalsIgnoreCase(retryHint)) {
80-
return Optional.of(RetryHint.PROPAGATE);
79+
if (RETRY_HINT_DO_NOT_RETRY_HEADER_VALUE.equalsIgnoreCase(retryHint)) {
80+
return Optional.of(RetryHint.DO_NOT_RETRY);
8181
}
8282
return Optional.empty();
8383
}
@@ -92,7 +92,7 @@ static String toHeaderValue(DueTo dueTo) {
9292
// VisibleForTesting
9393
static String toHeaderValue(RetryHint retryHint) {
9494
return switch (retryHint) {
95-
case PROPAGATE -> RETRY_HINT_PROPAGATE_HEADER_VALUE;
95+
case DO_NOT_RETRY -> RETRY_HINT_DO_NOT_RETRY_HEADER_VALUE;
9696
};
9797
}
9898

errors/src/test/java/com/palantir/conjure/java/api/errors/QosReasonsTest.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,10 @@ public void testReasonWithFields() {
4343
QosReason reason = QosReason.builder()
4444
.reason("reason")
4545
.dueTo(DueTo.CUSTOM)
46-
.retryHint(RetryHint.PROPAGATE)
46+
.retryHint(RetryHint.DO_NOT_RETRY)
4747
.build();
4848
QosReasons.encodeToResponse(reason, headers, Encoder.INSTANCE);
49-
assertThat(headers).isEqualTo(ImmutableMap.of("Qos-Due-To", "custom", "Qos-Retry-Hint", "propagate"));
49+
assertThat(headers).isEqualTo(ImmutableMap.of("Qos-Due-To", "custom", "Qos-Retry-Hint", "do-not-retry"));
5050
}
5151

5252
@Test
@@ -55,7 +55,7 @@ public void testRoundTrip() {
5555
QosReason original = QosReason.builder()
5656
.reason("reason")
5757
.dueTo(DueTo.CUSTOM)
58-
.retryHint(RetryHint.PROPAGATE)
58+
.retryHint(RetryHint.DO_NOT_RETRY)
5959
.build();
6060
QosReasons.encodeToResponse(original, headers, Encoder.INSTANCE);
6161
QosReason recreated = QosReasons.parseFromResponse(headers, Decoder.INSTANCE);
@@ -95,8 +95,8 @@ public void retryHintRoundTrip() {
9595

9696
@Test
9797
public void retryHintCaseSensitivity() {
98-
assertThat(QosReasons.parseRetryHint("propagate")).hasValue(RetryHint.PROPAGATE);
99-
assertThat(QosReasons.parseRetryHint("PROPAGATE")).hasValue(RetryHint.PROPAGATE);
98+
assertThat(QosReasons.parseRetryHint("do-not-retry")).hasValue(RetryHint.DO_NOT_RETRY);
99+
assertThat(QosReasons.parseRetryHint("DO-NOT-RETRY")).hasValue(RetryHint.DO_NOT_RETRY);
100100
}
101101

102102
private enum Encoder implements QosReasons.QosResponseEncodingAdapter<Map<String, String>> {

0 commit comments

Comments
 (0)