You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "merlimat (via GitHub)" <gi...@apache.org> on 2026/05/03 15:37:19 UTC

[PR] PIP-468: V5 policy options use builder pattern [pulsar]

merlimat opened a new pull request, #25657:
URL: https://github.com/apache/pulsar/pull/25657

   ## Summary
   
   Convert the V5 client policy options from records (with public canonical constructors) into final classes whose only construction paths are an explicit `Builder` and a small set of named factory methods. This keeps the public surface to deliberate, documented combinations and lets the classes evolve without breaking source compatibility every time a new field is added.
   
   Refactored: `BackoffPolicy`, `BatchingPolicy`, `ChunkingPolicy`, `CompressionPolicy`, `ConnectionPolicy`, `DeadLetterPolicy`, `EncryptionPolicy`, `ProcessingTimeoutPolicy`, `TlsPolicy`, `TransactionPolicy`.
   
   Each now has:
   - Private constructor.
   - private final fields with record-style accessors (`foo()` — backward compatible with existing callers).
   - A static `builder()` plus a fluent inner `Builder` with javadoc on each knob.
   - Small, opinionated factories where there's an obvious dominant case (e.g. `CompressionPolicy.disabled()`, `TlsPolicy.ofInsecure()`, `BatchingPolicy.ofDisabled()`, `BackoffPolicy.fixed`/`exponential(initial, max)`, `ProcessingTimeoutPolicy.of(timeout)`, `EncryptionPolicy.forProducer`/`forConsumer`).
   - Lombok `@EqualsAndHashCode` + `@ToString` to preserve value-type semantics.
   
   Updated callers — `Examples.java`, `V5DeadLetterPolicyTest`, `V5TransactionTest`, `V5ProducerBatchingTest` — to use the builder/factory entry points.
   
   ## Test plan
   
   - [x] `pulsar-client-api-v5` and `pulsar-broker` checkstyle clean.
   - [x] V5 affected tests pass: `V5DeadLetterPolicyTest`, `V5TransactionTest`, `V5ProducerBatchingTest`, `V5ProcessingTimeoutTest`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] PIP-468: V5 policy options use builder pattern [pulsar]

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #25657:
URL: https://github.com/apache/pulsar/pull/25657#discussion_r3178442501


##########
pulsar-client-api-v5/src/main/java/org/apache/pulsar/client/api/v5/config/BackoffPolicy.java:
##########
@@ -20,27 +20,55 @@
 
 import java.time.Duration;
 import java.util.Objects;
+import lombok.EqualsAndHashCode;
+import lombok.ToString;
 
 /**
  * Backoff configuration for broker reconnection attempts.
  *
  * <p>The delay for attempt {@code n} is {@code min(initialInterval * multiplier^(n-1), maxInterval)}.
  *
- * @param initialInterval the delay before the first reconnection attempt
- * @param maxInterval     the maximum delay between reconnection attempts
- * @param multiplier      the multiplier applied after each attempt
+ * <p>Use {@link #fixed(Duration, Duration)} or {@link #exponential(Duration, Duration)} for
+ * the common cases, or {@link #builder()} to configure all knobs explicitly.
  */
-public record BackoffPolicy(
-        Duration initialInterval,
-        Duration maxInterval,
-        double multiplier
-) {
-    public BackoffPolicy {
+@EqualsAndHashCode
+@ToString
+public final class BackoffPolicy {

Review Comment:
   Is there a way to configure random jitter for the `BackoffPolicy`?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] PIP-468: V5 policy options use builder pattern [pulsar]

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #25657:
URL: https://github.com/apache/pulsar/pull/25657#discussion_r3178489720


##########
pulsar-client-api-v5/src/main/java/org/apache/pulsar/client/api/v5/config/BackoffPolicy.java:
##########
@@ -20,27 +20,55 @@
 
 import java.time.Duration;
 import java.util.Objects;
+import lombok.EqualsAndHashCode;
+import lombok.ToString;
 
 /**
  * Backoff configuration for broker reconnection attempts.
  *
  * <p>The delay for attempt {@code n} is {@code min(initialInterval * multiplier^(n-1), maxInterval)}.
  *
- * @param initialInterval the delay before the first reconnection attempt
- * @param maxInterval     the maximum delay between reconnection attempts
- * @param multiplier      the multiplier applied after each attempt
+ * <p>Use {@link #fixed(Duration, Duration)} or {@link #exponential(Duration, Duration)} for
+ * the common cases, or {@link #builder()} to configure all knobs explicitly.
  */
-public record BackoffPolicy(
-        Duration initialInterval,
-        Duration maxInterval,
-        double multiplier
-) {
-    public BackoffPolicy {
+@EqualsAndHashCode
+@ToString
+public final class BackoffPolicy {

Review Comment:
   That's fine for now, perhaps we take a look later if there's a need to have jitter on all attempts. It could be useful.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] PIP-468: V5 policy options use builder pattern [pulsar]

Posted by "merlimat (via GitHub)" <gi...@apache.org>.
merlimat merged PR #25657:
URL: https://github.com/apache/pulsar/pull/25657


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] PIP-468: V5 policy options use builder pattern [pulsar]

Posted by "merlimat (via GitHub)" <gi...@apache.org>.
merlimat commented on code in PR #25657:
URL: https://github.com/apache/pulsar/pull/25657#discussion_r3178461519


##########
pulsar-client-api-v5/src/main/java/org/apache/pulsar/client/api/v5/config/BackoffPolicy.java:
##########
@@ -20,27 +20,55 @@
 
 import java.time.Duration;
 import java.util.Objects;
+import lombok.EqualsAndHashCode;
+import lombok.ToString;
 
 /**
  * Backoff configuration for broker reconnection attempts.
  *
  * <p>The delay for attempt {@code n} is {@code min(initialInterval * multiplier^(n-1), maxInterval)}.
  *
- * @param initialInterval the delay before the first reconnection attempt
- * @param maxInterval     the maximum delay between reconnection attempts
- * @param multiplier      the multiplier applied after each attempt
+ * <p>Use {@link #fixed(Duration, Duration)} or {@link #exponential(Duration, Duration)} for
+ * the common cases, or {@link #builder()} to configure all knobs explicitly.
  */
-public record BackoffPolicy(
-        Duration initialInterval,
-        Duration maxInterval,
-        double multiplier
-) {
-    public BackoffPolicy {
+@EqualsAndHashCode
+@ToString
+public final class BackoffPolicy {

Review Comment:
   eg: `        return Duration.ofMillis(Math.max(initialMillis, currentMillis));`
   
   but `initialMillis` is already the minimum and the first time `currentMillis` is like `initialMillis - ~10%`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] PIP-468: V5 policy options use builder pattern [pulsar]

Posted by "merlimat (via GitHub)" <gi...@apache.org>.
merlimat commented on code in PR #25657:
URL: https://github.com/apache/pulsar/pull/25657#discussion_r3178454348


##########
pulsar-client-api-v5/src/main/java/org/apache/pulsar/client/api/v5/config/BackoffPolicy.java:
##########
@@ -20,27 +20,55 @@
 
 import java.time.Duration;
 import java.util.Objects;
+import lombok.EqualsAndHashCode;
+import lombok.ToString;
 
 /**
  * Backoff configuration for broker reconnection attempts.
  *
  * <p>The delay for attempt {@code n} is {@code min(initialInterval * multiplier^(n-1), maxInterval)}.
  *
- * @param initialInterval the delay before the first reconnection attempt
- * @param maxInterval     the maximum delay between reconnection attempts
- * @param multiplier      the multiplier applied after each attempt
+ * <p>Use {@link #fixed(Duration, Duration)} or {@link #exponential(Duration, Duration)} for
+ * the common cases, or {@link #builder()} to configure all knobs explicitly.
  */
-public record BackoffPolicy(
-        Duration initialInterval,
-        Duration maxInterval,
-        double multiplier
-) {
-    public BackoffPolicy {
+@EqualsAndHashCode
+@ToString
+public final class BackoffPolicy {

Review Comment:
   Not really: 
   
   https://github.com/apache/pulsar/blob/77b05364f70811dc0dba13281fba68ac5a619478/pulsar-common/src/main/java/org/apache/pulsar/common/util/Backoff.java#L133-L138
   
   It's hardcoded to 10% and, worse, it's not applied the first time, so all first attempts are coming together



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org