TeeleTitan opened a new pull request, #1474:
URL: https://github.com/apache/commons-lang/pull/1474
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
https://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
Thanks for your contribution to [Apache
Commons](https://commons.apache.org/)! Your help is appreciated!
Before you push a pull request, review this list:
- [X ] Read the [contribution guidelines](CONTRIBUTING.md) for this project.
- [X ] Read the [ASF Generative Tooling
Guidance](https://www.apache.org/legal/generative-tooling.html) if you use
Artificial Intelligence (AI).
- [X] I used AI to create any part of, or all of, this pull request.
- [X ] Run a successful build using the default
[Maven](https://maven.apache.org/) goal with `mvn`; that's `mvn` on the command
line by itself.
- [X] Write unit tests that match behavioral changes, where the tests fail
if the changes to the runtime are not applied. This may not always be possible,
but it is a best practice.
- [X ] Write a pull request description that is detailed enough to
understand what the pull request does, how, and why.
- [X ] Each commit in the pull request should have a meaningful subject line
and body. Note that a maintainer may squash commits during the merge process.
**Description of Issue**
This PR adresses "LANG[1790] - Fairness setting in TimedSemaphore" :
https://issues.apache.org/jira/browse/LANG-1790
The existing TimedSemaphore implementation provides rate limiting across
fixed time periods, but uses manual synchronisation (wait() / notifyAll()) for
blocking. Under heavy contention, this approach can lead to non-deterministic
thread ordering and inconsistent blocking behaviour. Additionally, dynamically
changing the limit during an active period did not immediately update the
available permits, allowing extra acquires within that window.
**Implementation**
1. Fairness Support: Added a new builder flag that enables optional FIFO
fairness for permit acquires. Fairness is handled by a backing
'java.util.concurrent.Semaphore' constructed with the fair flag, ensuring that
waiting threads acquire permits in order.
2. Refactored Permit Management: Replaced counter synchronisation with a
dedicated Semaphore to manage permits. The semaphore now handles all blocking,
acquisition, and wake-up behavior, simplifying the internal logic and improving
correctness under contention.
3. Dynamic Limit Adjustment: Updated setLimit(int) to immediately resize the
active permit capacity for the current period.
4. End of period refill: The endOfPeriod() method now explicitly tops up the
semaphore to the configured limit at each interval, ensuring clean period
rollover and accurate permit replenishment.
**Validation and Testing**
1) FIFO ordering , Method: testFairnessFIFOOrdering_acrossPeriods()
When fair = true, and the limit is set to 1, threads that arrive earlier
should be served earlier across consecutive periods. The method acquires T1
(thread 1) first, then T2 and T3. Through manually calling endofPeriod() twice,
the completion order asserted is as follows:
1) T1
2) T2
3) T3
This test proves that the fairness flag actually allows for FIFO queuing
2) Dynamic resizing of the limit , Method:
testSetLimitResizesBucketWithinPeriod()
It checks that, in the middle of a period, given a decrease in the limit
takes place, the code immediately prevents further acquires within the same
period if the new limit has already been reached. It starts with limit = 3,
acquires twice, then it decreases the limit to 1. It first tries tryAcquire(),
which is expected to fail. Upon failure, endofPeriod() is called, and only 1
permit should be available in the new period.
This validates the behaviour of setLimit() and the coordination between
statistical counters.
3) Period rollover and unblocking , Method:
testEndOfPeriodTopUpReleasesBlockedThreads()
It checks that endofPeriod() refills permits to the configured limit, and
‘wakes’ blocked threads through notifyAll(), which allows for the next thread
waiting to acquire. It starts with assignment limit = 1, T1 acquires and T2
blocks. After manually calling endOfPeriod(), T2 should complete straight away.
This confirms the new top-up and unblocking logic are correctly implemented
4) NO_LIMIT short circuit with fairness , Method:
testAcquireNoLimitWithFairnessEnabled()
Through ‘limit <= NO_LIMIT’, acquire() never blocks, even when fairness is
enabled. It does this via building through Builder with NO_LIMIT and assuring
fair = true. Then, acquire() is called many times without waiting for period
rollover
This confirms the short circuit path ignores the Semaphore cleanly and
remains viable alongside the fairness option
5) Permits after increase , Method:
testGetAvailablePermitsAfterLimitIncreaseWithinPeriod()
It checks that whether the limit increase in the middle of a period
influences the remaining (non-blocking) available permits, reported via
getAvaliablePermits() alongside tryAcquire(). It does this as, when limit = 1,
acquire occurs once, then the limit increase to 3, then two more non-blocking
acquires should now pass and be valid, and then finally the 4th one should fail
This verifies that counters and semaphore capacity stay consistent when
upscaling the limit during a period
--
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]
