awaneetdecoder commented on code in PR #5671:
URL: https://github.com/apache/fineract/pull/5671#discussion_r3028813339
##########
fineract-savings/src/main/java/org/apache/fineract/portfolio/savings/service/SavingsSchedularInterestPoster.java:
##########
@@ -213,8 +213,24 @@ private void batchUpdate(final List<SavingsAccountData>
savingsAccountDataList)
savingsAccountData.setUpdatedTransactions(savingsAccountTransactionDataList);
}
- if (transRefNo.size() > 0) {
- this.jdbcTemplate.batchUpdate(queryForSavingsUpdate,
paramsForSavingsSummary);
+ if (!transRefNo.isEmpty()) {
+ int[] updateCounts =
this.jdbcTemplate.batchUpdate(queryForSavingsUpdate, paramsForSavingsSummary);
+
+ Set<Long> skippedAccountIds = new HashSet<>();
+ for (int i = 0; i < updateCounts.length; i++) {
+ if (updateCounts[i] == 0) {
+ Long accountId = savingsAccountDataList.get(i).getId();
+ skippedAccountIds.add(accountId);
+ log.warn("Optimistic lock failure for savings account
id={} — concurrent modification detected."
+ + " Rolling back. Will retry on next run.",
accountId);
+ }
+ }
+
+ if (!skippedAccountIds.isEmpty()) {
Review Comment:
@adamsaghy The exception is thrown to keep the batch atomic. If account A's
summary update succeeds but account B's fails the version check, without the
exception account A's transactions would still be inserted while account B's
summary remains out of sync — inconsistent state within the same batch. The
throw causes full rollback so all accounts retry together on the next run. This
behaviour is covered in SavingsSchedularInterestPosterTest —
testSkippedAccountIdsNotEmptyMeansExceptionShouldBeThrown validates this path.
--
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]
