Expose configured backlog timeout in LifoBlockingLimiter #223
Conversation
| */ | ||
| public Builder<ContextT> backlogTimeoutMillis(long timeout) { | ||
| this.maxBacklogTimeoutMillis = context -> timeout; | ||
| this.configuredBacklogTimeoutMillis = timeout; |
There was a problem hiding this comment.
If a function is set through backlogTimeout, should we clear the configuredBacklogTimeoutMillis?
There was a problem hiding this comment.
That’s a great suggestion. For cleanup purposes, we probably should. If we switch to a dynamic function, there’s no single fixed timeout anymore, so configuredBacklogTimeoutMillis could be cleared. I’m going to make configuredBacklogTimeoutMillis a boxed value so we can clear it by assigning null.
| private final Limiter<ContextT> delegate; | ||
| private int maxBacklogSize = 100; | ||
| private Function<ContextT, Long> maxBacklogTimeoutMillis = context -> 1_000L; | ||
| private long configuredBacklogTimeoutMillis = 1_000L; |
There was a problem hiding this comment.
Nit: is the goal to track the configured timeout or to quickly identify the value of any fixed timeout? If the latter, fixedBacklogTimeoutMillis would be a better name.
There was a problem hiding this comment.
The goal is to expose the value only when a fixed timeout is configured, not to track historical configuration. fixedBacklogTimeoutMillis is indeed a more accurate name. I’ll update the field and getter accordingly.
| public long getConfiguredBacklogTimeoutMillis() { | ||
| return this.configuredBacklogTimeoutMillis; | ||
| } | ||
|
|
There was a problem hiding this comment.
I believe that we also need to add a similar accessor to BlockingLimiter.
There was a problem hiding this comment.
Should we? I dont think we use BlockingLimiter internally and it feels to it is a different kind of limiter(pls correct me if I am wrong). maxBacklogSize is not exposed into BlockingLimiter either.
I feel like my change is solving a very specific problem: expose configured backlog timeout in LifoBlockingLimiter
agshing
force-pushed
the
add-backlog-timeout-getter
branch
from
fff42fb to
5a198dc
Compare
agshing
force-pushed
the
add-backlog-timeout-getter
branch
from
5a198dc to
09a3971
Compare
3 participants
Adds a simple getter,
getFixedBacklogTimeoutMillis(), toLifoBlockingLimiterso the fixed backlog timeout configured via the builder can be inspected and serialized.This does not change limiter behavior; it only exposes existing configuration for introspection and tooling.