Skip to content

waitTimeoutOnResponseBackpressureMs should be gt 0#3600

Closed
gaozhangmin wants to merge 1 commit into
apache:masterfrom
gaozhangmin:throttle-1
Closed

waitTimeoutOnResponseBackpressureMs should be gt 0#3600
gaozhangmin wants to merge 1 commit into
apache:masterfrom
gaozhangmin:throttle-1

Conversation

@gaozhangmin
Copy link
Copy Markdown
Contributor

update timeOut >= 0 to timeout > 0, it make no sense executing back pressure when waitTimeoutOnBackpressureMillis=0

@gaozhangmin gaozhangmin changed the title waitTimeoutOnResponseBackpressureMs gt 0 waitTimeoutOnResponseBackpressureMs should be gt 0 Nov 2, 2022
Copy link
Copy Markdown
Contributor

@hangc0276 hangc0276 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 to allow request to fail immediately


final long timeOut = requestProcessor.getWaitTimeoutOnBackpressureMillis();
if (timeOut >= 0 && !channel.isWritable()) {
if (timeOut > 0 && !channel.isWritable()) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

timeout is 0 should fail immediately, it is expected.

Copy link
Copy Markdown
Contributor Author

@gaozhangmin gaozhangmin Nov 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it ok when timeout is 0 and blacklistedChannels = Optional.empty() ? @zymap @hangc0276

if (waitTimeoutOnBackpressureMillis > 0) {
blacklistedChannels = Optional.of(CacheBuilder.newBuilder()
.expireAfterWrite(waitTimeoutOnBackpressureMillis, TimeUnit.MILLISECONDS)
.build());
} else {
blacklistedChannels = Optional.empty();
}

@gaozhangmin gaozhangmin closed this Nov 4, 2022
@gaozhangmin gaozhangmin reopened this Nov 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants