[
https://issues.apache.org/jira/browse/FINERACT-2787?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Lea Fan updated FINERACT-2787:
------------------------------
Description:
Background
The submitGSIMApplication method submits multiple client savings applications
in a single request and associates the created savings accounts with a GSIM
account.
Code location
File:
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsApplicationProcessWritePlatformServiceJpaRepositoryImpl.java
Method:
submitGSIMApplication(JsonCommand command)
Current implementation
The method creates a new lock object and uses it to wrap the client-processing
loop:
final Object lock = new Object();
synchronized (lock) {
for (JsonElement gsimApplication : gsimApplications) {
result = submitApplication(...);
}
}
Problem
The lock object is created inside submitGSIMApplication. Therefore, every
method invocation creates a different lock instance.
For example:
- Request A creates and locks object A.
- Request B creates and locks object B.
- Because object A and object B are different objects, both requests can enter
the synchronized block at the same time.
As a result, the current synchronized block cannot coordinate concurrent GSIM
submission requests or prevent them from executing at the same time.
Within a single request, submitApplication is already called sequentially by
the for-loop. One iteration must finish before the next iteration begins, even
without the synchronized block.
Database transaction handling is provided separately by @Transactional. The
synchronized block does not provide transaction commit or rollback behavior.
The current lock is therefore ineffective for cross-request concurrency control
and does not add ordering within a single request.
This issue does not claim that a user-visible concurrency failure has already
been reproduced. The intended concurrency requirement should be clarified
before choosing the implementation.
Expected behavior
Clarify the intended concurrency behavior of GSIM application submission:
- If only sequential processing within one request is required, remove the
redundant method-local lock.
- If concurrent submissions for the same group or application must be
coordinated, use an appropriate mechanism scoped to the relevant business
identifier.
- If coordination must work across multiple Fineract instances, use an
appropriate transactional, database-level, or distributed mechanism instead of
a JVM-local lock.
Acceptance criteria
- The intended concurrency behavior is confirmed.
- The ineffective method-local lock is removed or replaced with a mechanism
that provides the required concurrency control.
- Relevant tests cover the confirmed behavior.
- The implementation does not imply concurrency protection that it cannot
provide.
Related issues
- FINERACT-603 - Original GSIM and GLIM implementation
https://issues.apache.org/jira/browse/FINERACT-603
- FINERACT-2237 - GSIM parent account creation fix
https://issues.apache.org/jira/browse/FINERACT-2237
was:
Background
The submitGSIMApplication method in
SavingsApplicationProcessWritePlatformServiceJpaRepositoryImpl class submits
savings applications for the clients belonging to a GSIM group.
Current implementation
The method creates a new lock object for each invocation:
final Object lock = new Object();
synchronized (lock) {
// submitApplication(...)
}
Problem
Because the lock object is created inside the method, every invocation receives
a different lock instance.
Therefore, two concurrent requests executing submitGSIMApplication do not
synchronize with each other. Each request locks only its own private object.
Within a single invocation, the existing for-loop already invokes
submitApplication sequentially, so the synchronized block does not provide
additional ordering for that loop.
This may give readers the misleading impression that concurrent GSIM
application submissions are coordinated, although the lock cannot provide
cross-request mutual exclusion.
This issue does not claim that a user-visible concurrency failure has already
been reproduced. The intended concurrency requirement should first be clarified.
Expected behavior
Clarify the intended concurrency semantics of GSIM application submission:
* If cross-request mutual exclusion is not required, remove the ineffective
local lock.
* If cross-request coordination is required, use an appropriate shared,
transactional, or database-level mechanism based on the business invariant
being protected.
Acceptance criteria
* The method-local lock is removed or replaced with a mechanism that actually
implements the intended concurrency behavior.
* Tests cover the intended GSIM application submission behavior.
* The implementation does not imply concurrency protection that it cannot
provide.
Related issues
* FINERACT-603 - Original GSIM and GLIM implementation
* FINERACT-2237 - GSIM parent account creation fix
> Ineffective method-local synchronization in GSIM application submission
> -----------------------------------------------------------------------
>
> Key: FINERACT-2787
> URL: https://issues.apache.org/jira/browse/FINERACT-2787
> Project: Apache Fineract
> Issue Type: Improvement
> Components: Savings
> Reporter: Lea Fan
> Priority: Minor
>
> Background
> The submitGSIMApplication method submits multiple client savings applications
> in a single request and associates the created savings accounts with a GSIM
> account.
> Code location
> File:
> fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsApplicationProcessWritePlatformServiceJpaRepositoryImpl.java
> Method:
> submitGSIMApplication(JsonCommand command)
> Current implementation
> The method creates a new lock object and uses it to wrap the
> client-processing loop:
> final Object lock = new Object();
> synchronized (lock) {
> for (JsonElement gsimApplication : gsimApplications) {
> result = submitApplication(...);
> }
> }
> Problem
> The lock object is created inside submitGSIMApplication. Therefore, every
> method invocation creates a different lock instance.
> For example:
> - Request A creates and locks object A.
> - Request B creates and locks object B.
> - Because object A and object B are different objects, both requests can
> enter the synchronized block at the same time.
> As a result, the current synchronized block cannot coordinate concurrent GSIM
> submission requests or prevent them from executing at the same time.
> Within a single request, submitApplication is already called sequentially by
> the for-loop. One iteration must finish before the next iteration begins,
> even without the synchronized block.
> Database transaction handling is provided separately by @Transactional. The
> synchronized block does not provide transaction commit or rollback behavior.
> The current lock is therefore ineffective for cross-request concurrency
> control and does not add ordering within a single request.
> This issue does not claim that a user-visible concurrency failure has already
> been reproduced. The intended concurrency requirement should be clarified
> before choosing the implementation.
> Expected behavior
> Clarify the intended concurrency behavior of GSIM application submission:
> - If only sequential processing within one request is required, remove the
> redundant method-local lock.
> - If concurrent submissions for the same group or application must be
> coordinated, use an appropriate mechanism scoped to the relevant business
> identifier.
> - If coordination must work across multiple Fineract instances, use an
> appropriate transactional, database-level, or distributed mechanism instead
> of a JVM-local lock.
> Acceptance criteria
> - The intended concurrency behavior is confirmed.
> - The ineffective method-local lock is removed or replaced with a mechanism
> that provides the required concurrency control.
> - Relevant tests cover the confirmed behavior.
> - The implementation does not imply concurrency protection that it cannot
> provide.
> Related issues
> - FINERACT-603 - Original GSIM and GLIM implementation
> https://issues.apache.org/jira/browse/FINERACT-603
> - FINERACT-2237 - GSIM parent account creation fix
> https://issues.apache.org/jira/browse/FINERACT-2237
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
