[GitHub] [fineract] galovics commented on a diff in pull request #3253: [FINERACT-1941] change task executors to threadpool task executor

["galovics (via GitHub)" <gi...@apache.org>]

galovics commented on code in PR #3253:
URL: https://github.com/apache/fineract/pull/3253#discussion_r1229155419


##########
fineract-core/src/main/java/org/apache/fineract/infrastructure/core/config/FineractProperties.java:
##########
@@ -190,6 +192,30 @@ public static class FineractEventsProperties {
         private FineractExternalEventsProperties external;
     }
 
+    @Getter
+    @Setter
+    public static class FineractTaskExecutor {
+
+        private FineractEventTaskExecutor eventTaskExecutor;

Review Comment:
   I don't think this is in the right place. The property organization was horizontal until this point and now you're making it vertical. 
   
   The event related configuration should be with the rest of the event properties.



##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/campaigns/jobs/sendmessagetosmsgateway/SendMessageToSmsGatewayConfig.java:
##########
@@ -46,6 +48,9 @@ public class SendMessageToSmsGatewayConfig {
     private NotificationSenderService notificationSenderService;
     @Autowired
     private SmsConfigUtils smsConfigUtils;
+    @Autowired
+    @Qualifier("fineractDefaultThreadPoolTaskExecutor")

Review Comment:
   I think you could create a constant on these specific bean name(s) so you don't have to put this string value everywhere.



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/jobs/recalculateinterestforloan/RecalculateInterestForLoanTasklet.java:
##########
@@ -91,8 +91,8 @@ public RepeatStatus execute(StepContribution contribution, ChunkContext chunkCon
 
     private void recalculateInterest(OfficeData office, int threadPoolSize, int batchSize) {
         final int pageSize = batchSize * threadPoolSize;
-
-        final ExecutorService executorService = Executors.newFixedThreadPool(threadPoolSize);
+        taskExecutor.setCorePoolSize(threadPoolSize);

Review Comment:
   Sooo, are we updating the global default task executor configuration? This mustn't be right..



##########
fineract-provider/src/main/java/org/apache/fineract/portfolio/savings/jobs/postinterestforsavings/PostInterestForSavingTasklet.java:
##########
@@ -55,15 +55,18 @@ public class PostInterestForSavingTasklet implements Tasklet {
     private final ConfigurationDomainService configurationDomainService;
     private final Queue<List<SavingsAccountData>> queue = new ArrayDeque<>();
     private final ApplicationContext applicationContext;
+    @Qualifier("fineractDefaultThreadPoolTaskExecutor")
+    private final ThreadPoolTaskExecutor taskExecutor;
     private final int queueSize = 1;
 
     @Override
     public RepeatStatus execute(StepContribution contribution, ChunkContext chunkContext) throws Exception {
         final int threadPoolSize = Integer.parseInt((String) chunkContext.getStepContext().getJobParameters().get("thread-pool-size"));
+        taskExecutor.setCorePoolSize(threadPoolSize);

Review Comment:
   Same as for the other one. This shouldn't be done at all.



-- 
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: commits-unsubscribe@fineract.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org