You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by "taskain7 (via GitHub)" <gi...@apache.org> on 2023/06/14 00:58:48 UTC

[GitHub] [fineract] taskain7 opened a new pull request, #3253: [FINERACT-1941] change task executors to threadpool task executor

taskain7 opened a new pull request, #3253:
URL: https://github.com/apache/fineract/pull/3253

   ## Description
   
   Change task executors to threadpool task executor.
   Make threadpool task executors configurable via environment properties.


-- 
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


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

Posted by "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


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

Posted by "taskain7 (via GitHub)" <gi...@apache.org>.
taskain7 commented on code in PR #3253:
URL: https://github.com/apache/fineract/pull/3253#discussion_r1229224343


##########
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:
   replaced under the event properties



-- 
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


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

Posted by "taskain7 (via GitHub)" <gi...@apache.org>.
taskain7 commented on code in PR #3253:
URL: https://github.com/apache/fineract/pull/3253#discussion_r1229225669


##########
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:
   a configurable task executor has been made, which is a prototype bean



##########
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:
   a configurable task executor has been made, which is a prototype bean



-- 
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


[GitHub] [fineract] adamsaghy merged pull request #3253: [FINERACT-1941] change task executors to threadpool task executor

Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy merged PR #3253:
URL: https://github.com/apache/fineract/pull/3253


-- 
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


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

Posted by "taskain7 (via GitHub)" <gi...@apache.org>.
taskain7 commented on code in PR #3253:
URL: https://github.com/apache/fineract/pull/3253#discussion_r1229224752


##########
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:
   constants are used now



-- 
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