avivijay19 commented on code in PR #6069:
URL: https://github.com/apache/fineract/pull/6069#discussion_r3633702951
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/service/increasedateby1day/increasecobdateby1day/IncreaseCobDateBy1DayTasklet.java:
##########
@@ -39,7 +41,13 @@ public class IncreaseCobDateBy1DayTasklet implements Tasklet
{
@Override
public RepeatStatus execute(StepContribution contribution, ChunkContext
chunkContext) throws Exception {
if (configurationDomainService.isBusinessDateEnabled()) {
-
businessDateWritePlatformService.increaseDateByTypeByOneDay(BusinessDateType.COB_DATE);
+ try {
Review Comment:
Reverted, together with the retry in both tasklets and the unit tests
covering it.
For the record ahead of the follow-up: `INCREASE_BUSINESS_DATE_BY_1_DAY` and
`INCREASE_COB_DATE_BY_1_DAY` can execute concurrently and both insert the
initial `m_business_date` row, causing one of them to fail on a duplicate key.
It manifested on this branch as a `SchedulerJobsTest` failure on the MySQL
shard. It is nevertheless a pre-existing race condition rather than one
introduced by Spring Boot 4, so I agree it does not belong in this PR. I will
raise it separately, using an upsert rather than a retry, which is the more
appropriate fix.
Could you please confirm which JIRA issue I should raise this under, or
whether a new one should be created? I will reference it in the commit message.
##########
fineract-provider/src/test/java/org/apache/fineract/infrastructure/jobs/service/increasedateby1day/increasebusinessdateby1day/IncreaseBusinessDateBy1DayTaskletTest.java:
##########
@@ -77,4 +80,27 @@ public void
shouldBusinessDateJobBeProcessedWhenBusinessDateIsEnabled() throws E
assertEquals(RepeatStatus.FINISHED, repeatStatus);
}
+ @Test
+ public void shouldRetryOnceWhenConcurrentDateCreationFails() throws
Exception {
Review Comment:
Reverted, together with the retry logic these tests covered.
Could you please confirm which JIRA issue the follow-up should be raised
under, or whether a new one should be created?
##########
fineract-provider/src/test/java/org/apache/fineract/infrastructure/jobs/service/increasedateby1day/increasecobdateby1day/IncreaseCobDateBy1DayTaskletTest.java:
##########
@@ -77,4 +80,27 @@ public void
shouldBusinessDateJobBeProcessedWhenBusinessDateIsEnabled() throws E
assertEquals(RepeatStatus.FINISHED, repeatStatus);
}
+ @Test
+ public void shouldRetryOnceWhenConcurrentDateCreationFails() throws
Exception {
Review Comment:
Reverted, together with the retry logic these tests covered.
Could you please confirm which JIRA issue the follow-up should be raised
under, or whether a new one should be created?
##########
fineract-cob/src/main/java/org/apache/fineract/cob/tasklet/ApplyCommonLockTasklet.java:
##########
Review Comment:
Applied as suggested.
##########
fineract-cob/src/main/java/org/apache/fineract/cob/COBBusinessStepServiceImpl.java:
##########
@@ -56,6 +57,10 @@ public class COBBusinessStepServiceImpl implements
COBBusinessStepService {
@SuppressWarnings({ "unchecked" })
@Override
+ // Spring Batch 6 ChunkOrientedStep runs item processing on task-executor
threads OUTSIDE the chunk transaction,
+ // so the business step chain must open its own transaction there; in
sequential mode REQUIRED simply joins the
+ // chunk transaction, matching the Batch 5 behavior
+ @Transactional
Review Comment:
Applied, using your wording verbatim.
##########
fineract-command-jdbc/src/main/java/org/apache/fineract/command/jdbc/store/JdbcCommandStore.java:
##########
Review Comment:
Applied. `asText()` is deprecated in Jackson 3, where it is a `final` alias
delegating to `asString()`, so this is a like-for-like replacement. All four
call sites in this class have been updated; they were the only remaining uses
of `asText()` on a Jackson 3 node in the repository.
##########
fineract-provider/src/main/java/org/apache/fineract/cob/loan/LoanCOBWorkerConfiguration.java:
##########
@@ -106,8 +108,13 @@ public Step loanCOBWorkerStep() {
.listener(cobWorkerStepListener()) //
.transactionManager(transactionManager);
+ // Batch 6's chunk-oriented step no longer scans item components for
step-listener
+ // annotations, so register the reader and processor as
StepExecutionListeners explicitly
+ stepBuilder.listener(cobWorkerItemReader());
+ stepBuilder.listener(cobWorkerItemProcessor());
+
if (propertyService.getThreadPoolMaxPoolSize(LoanCOBConstant.JOB_NAME)
> 1) {
- stepBuilder.taskExecutor(cobTaskExecutor());
+ stepBuilder.taskExecutor((AsyncTaskExecutor) cobTaskExecutor());
Review Comment:
You are correct. The cast is only safe because the enclosing condition
duplicates the branch inside `cobTaskExecutor()`, both reading
`getThreadPoolMaxPoolSize("LOAN_COB")`. Should those ever diverge, the result
would be a `ClassCastException`.
I have replaced it with a type check so that the bean remains the single
source of truth:
```java
if (cobTaskExecutor() instanceof AsyncTaskExecutor asyncTaskExecutor) {
stepBuilder.taskExecutor(asyncTaskExecutor);
}
```
`SyncTaskExecutor` implements only `TaskExecutor`, and Spring Batch 6
narrowed `taskExecutor(..)` to accept `AsyncTaskExecutor`. Leaving the step
executor unset therefore produces exactly the sequential execution that a
synchronous executor already implied.
--
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]
