beemines opened a new pull request, #16456:
URL: https://github.com/apache/dubbo/pull/16456
## What is the purpose of the change?
Fixes #16455.
`ActiveLimitFilter` and `ExecuteLimitFilter` skip error accounting for every
`LIMIT_EXCEEDED_EXCEPTION`. If a downstream invoker throws that exception after
admission, or completes its result future exceptionally with it, the acquired
concurrency slot is never released. At a limit of 1, later calls can time out
or be rejected despite no call remaining in flight.
Track whether each filter acquired a slot during the current invocation
attempt. Only locally rejected limit exceptions skip accounting; downstream
limit exceptions release the slot through the existing error callback. Scope
each marker by the target URL identity (also used by RpcStatus), since forking
calls can share an Invocation across providers. Reset the marker on entry so
sequential reuse does not reuse a previous admission decision. Existing timing,
wakeup, successful-response, and non-limit-error handling are preserved.
The regression tests use the production `CopyOfFilterChainNode` and
`CallbackRegistrationInvoker` lifecycle with local mock invokers and
controllable futures. For both filters they verify synchronous downstream
rejection, asynchronous exceptional completion, local rejection while another
call remains active, sequential invocation reuse, and target isolation when two
invokers share the same Invocation.
## Verification
JDK 17.0.17, Maven 3.9.4, Windows; the project's Java 8 artifact target is
unchanged.
```shell
./mvnw -B -pl dubbo-cluster -am \
-Dtest=LimitFilterSlotReleaseTest,ActiveLimitFilterTest,ExecuteLimitFilterTest,RpcStatusTest,DefaultFilterChainBuilderTest
\
-Dsurefire.failIfNoSpecifiedTests=false test
./mvnw -B -pl dubbo-cluster -am -Pcheckstyle \
-Dcheckstyle_unix.skip=true '-DspotlessFiles=.*Limit.*java' validate
spotless:check
```
- Before the fix on `dab47b7843`: 8 new regression cases, 4 assertion
failures, 0 errors. Both filters retain an active count of 1 after
synchronous/asynchronous downstream limit errors. The local-rejection controls
pass.
- After the final fix: all 26 selected tests pass, including 10 new cases; 0
failures, errors, or skips. The two additional cases verify that different
targets sharing an Invocation retain separate admission state.
- Java Checkstyle reports 0 violations across the selected reactor; Spotless
reports no changes needed for the checked limit-related files, and git diff
--check is clean.
On this Windows checkout, the initial Unix resource-line-ending check
reports 59 pre-existing CRLF resource files due to core.autocrlf=true. The
separate Unix check was disabled locally for subsequent validation; no
unrelated resources were modified. All three changed Java files use LF. The
final test run also enabled Java Checkstyle and applied Spotless without
further changes.
The regression targets exceptions delivered to `onError`, not business
exceptions carried inside a normally completed `AppResponse`. No registry or
external RPC service is required. The full repository test suite was not run.
AI assistance was used for investigation, code, and tests. The failing
behavior was reproduced locally before applying the production fix.
## Checklist
- [x] A GitHub issue describes the change: #16455.
- [x] The description explains the problem, behavior, and implementation.
- [x] Regression tests cover the fix and admission controls.
- [ ] GitHub Actions pass (pending upstream execution).
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
