You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by "galovics (via GitHub)" <gi...@apache.org> on 2023/04/13 16:58:19 UTC
[GitHub] [fineract] galovics commented on a diff in pull request #3120: FINERACT-1913-Initializing-db-connection-pool-at-startup
galovics commented on code in PR #3120:
URL: https://github.com/apache/fineract/pull/3120#discussion_r1165802413
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/service/database/TomcatJdbcDataSourcePerTenantService.java:
##########
@@ -73,4 +83,29 @@ public DataSource retrieveDataSource() {
return actualDataSource;
}
+
+ @Override
+ public void onApplicationEvent(ContextRefreshedEvent event) {
+ final List<FineractPlatformTenant> allTenants = tenantDetailsService.findAllTenants();
+ for (final FineractPlatformTenant tenant : allTenants) {
+ initializeDataSourceConnection(tenant);
+ }
+ }
+
+ private void initializeDataSourceConnection(FineractPlatformTenant tenant) {
+ log.debug("Initializing database connection for {}", tenant.getName());
+ final FineractPlatformTenantConnection tenantConnection = tenant.getConnection();
+ if (!TENANT_TO_DATA_SOURCE_MAP.containsKey(tenantConnection.getConnectionId())) {
Review Comment:
This is not thread-safe. The application events by default are processed synchronously by the event listeners but Fineract has a different strategy configured, i.e. it's using a custom task executor to execute the event listeners (see SpringConfig class in Fineract, applicationEventMulticaster).
Therefore the map operations are needed to be made thread-safe.
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/service/database/TomcatJdbcDataSourcePerTenantService.java:
##########
@@ -73,4 +83,29 @@ public DataSource retrieveDataSource() {
return actualDataSource;
}
+
+ @Override
+ public void onApplicationEvent(ContextRefreshedEvent event) {
+ final List<FineractPlatformTenant> allTenants = tenantDetailsService.findAllTenants();
+ for (final FineractPlatformTenant tenant : allTenants) {
+ initializeDataSourceConnection(tenant);
+ }
+ }
+
+ private void initializeDataSourceConnection(FineractPlatformTenant tenant) {
+ log.debug("Initializing database connection for {}", tenant.getName());
+ final FineractPlatformTenantConnection tenantConnection = tenant.getConnection();
+ if (!TENANT_TO_DATA_SOURCE_MAP.containsKey(tenantConnection.getConnectionId())) {
+ DataSource tenantSpecificDataSource = dataSourcePerTenantServiceFactory.createNewDataSourceFor(tenantConnection);
+ try (Connection connection = tenantSpecificDataSource.getConnection()) {
+ String url = connection.getMetaData().getURL();
+ log.debug("Established database connection with URL {}", url);
+ } catch (SQLException e) {
+ log.error("Error while initializing database connection for {}", tenant.getName(), e.getMessage());
Review Comment:
I think it's reasonable to catch it since the initialization process failing is not fatal to the application itself. One thing I'd do however is to pass the exception itself in the log message, not the exception message.
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/service/database/TomcatJdbcDataSourcePerTenantService.java:
##########
@@ -73,4 +83,29 @@ public DataSource retrieveDataSource() {
return actualDataSource;
}
+
+ @Override
+ public void onApplicationEvent(ContextRefreshedEvent event) {
+ final List<FineractPlatformTenant> allTenants = tenantDetailsService.findAllTenants();
+ for (final FineractPlatformTenant tenant : allTenants) {
+ initializeDataSourceConnection(tenant);
+ }
+ }
+
+ private void initializeDataSourceConnection(FineractPlatformTenant tenant) {
+ log.debug("Initializing database connection for {}", tenant.getName());
+ final FineractPlatformTenantConnection tenantConnection = tenant.getConnection();
+ if (!TENANT_TO_DATA_SOURCE_MAP.containsKey(tenantConnection.getConnectionId())) {
+ DataSource tenantSpecificDataSource = dataSourcePerTenantServiceFactory.createNewDataSourceFor(tenantConnection);
+ try (Connection connection = tenantSpecificDataSource.getConnection()) {
+ String url = connection.getMetaData().getURL();
+ log.debug("Established database connection with URL {}", url);
+ } catch (SQLException e) {
+ log.error("Error while initializing database connection for {}", tenant.getName(), e.getMessage());
+ }
+ TENANT_TO_DATA_SOURCE_MAP.put(tenantConnection.getConnectionId(), tenantSpecificDataSource);
Review Comment:
As I wrote above, not thread-safe.
--
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