Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat : use better way to delegate database #1491

Merged
merged 3 commits into from
Nov 2, 2024
Merged

feat : use better way to delegate database #1491

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5b1e463
feat : use better way to delegate database
rajadilipkolli Nov 2, 2024
9f20024
fix : Issues with Junit
rajadilipkolli Nov 2, 2024
7672175
adds more documentation
rajadilipkolli Nov 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions jpa/boot-read-replica-postgresql/docker-compose.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
version: '2'

services:
postgresql-master:
image: 'bitnami/postgresql:latest'
Expand Down
10 changes: 7 additions & 3 deletions jpa/boot-read-replica-postgresql/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<parent>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-parent</artifactId>
<version>3.3.5</version>
<version>3.4.0-RC1</version>
<relativePath /> <!-- lookup parent from repository -->
</parent>
<groupId>com.example.demo</groupId>
Expand Down Expand Up @@ -50,12 +50,16 @@
<groupId>org.liquibase</groupId>
<artifactId>liquibase-core</artifactId>
</dependency>

<dependency>
<groupId>org.postgresql</groupId>
<artifactId>postgresql</artifactId>
<scope>runtime</scope>
</dependency>
<dependency>
<groupId>net.ttddyy.observation</groupId>
<artifactId>datasource-micrometer-spring-boot</artifactId>
<version>1.0.5</version>
</dependency>

<dependency>
<groupId>org.springframework.boot</groupId>
Expand Down Expand Up @@ -105,7 +109,7 @@
<configuration>
<java>
<googleJavaFormat>
<version>1.22.0</version>
<version>1.24.0</version>
<style>AOSP</style>
</googleJavaFormat>
</java>
Expand Down
22 changes: 7 additions & 15 deletions ...-replica-postgresql/src/main/java/com/example/demo/readreplica/config/DatabaseConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
package com.example.demo.readreplica.config;

import com.example.demo.readreplica.config.routing.RoutingDataSource;
import com.zaxxer.hikari.HikariDataSource;
import java.util.HashMap;
import java.util.Map;
import javax.sql.DataSource;
import org.springframework.boot.autoconfigure.jdbc.DataSourceProperties;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Primary;
import org.springframework.jdbc.datasource.LazyConnectionDataSourceProxy;

@Configuration(proxyBeanMethods = false)
class DatabaseConfig {
Expand All @@ -24,7 +22,7 @@ DataSourceProperties primaryDataSourceProperties() {
}

@Bean
@ConfigurationProperties(PRIMARY_DATABASE_PROPERTY_KEY_PREFIX + ".configuration")
@ConfigurationProperties(PRIMARY_DATABASE_PROPERTY_KEY_PREFIX + ".hikari")
DataSource primaryDataSource(final DataSourceProperties primaryDataSourceProperties) {
return primaryDataSourceProperties
.initializeDataSourceBuilder()
Expand All @@ -39,7 +37,7 @@ DataSourceProperties replicaDataSourceProperties() {
}

@Bean
@ConfigurationProperties(REPLICA_DATABASE_PROPERTY_KEY_PREFIX + ".configuration")
@ConfigurationProperties(REPLICA_DATABASE_PROPERTY_KEY_PREFIX + ".hikari")
DataSource replicaDataSource(final DataSourceProperties replicaDataSourceProperties) {
return replicaDataSourceProperties
.initializeDataSourceBuilder()
Expand All @@ -50,15 +48,9 @@ DataSource replicaDataSource(final DataSourceProperties replicaDataSourcePropert
@Bean
@Primary
DataSource dataSource(final DataSource primaryDataSource, final DataSource replicaDataSource) {
final RoutingDataSource routingDataSource = new RoutingDataSource();

final Map<Object, Object> targetDataSources = new HashMap<>();
targetDataSources.put(RoutingDataSource.Route.PRIMARY, primaryDataSource);
targetDataSources.put(RoutingDataSource.Route.REPLICA, replicaDataSource);

routingDataSource.setTargetDataSources(targetDataSources);
routingDataSource.setDefaultTargetDataSource(primaryDataSource);

return routingDataSource;
LazyConnectionDataSourceProxy lazyConnectionDataSourceProxy =
new LazyConnectionDataSourceProxy(primaryDataSource);
lazyConnectionDataSourceProxy.setReadOnlyDataSource(replicaDataSource);
return lazyConnectionDataSourceProxy;
Comment on lines +51 to +54
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Potential issue with setReadOnlyDataSource method

The LazyConnectionDataSourceProxy class does not have a setReadOnlyDataSource method in the standard Spring Framework. This may result in a NoSuchMethodError at runtime.

Consider the following options to address this:

  • Option 1: Verify if there's a custom subclass or an extension of LazyConnectionDataSourceProxy that includes the setReadOnlyDataSource method. If so, ensure it's correctly imported and used.
  • Option 2: Use AbstractRoutingDataSource to implement dynamic data source routing between primary and replica databases.

Apply this diff if opting for AbstractRoutingDataSource:

@@ -49,8 +49,15 @@
     @Bean
     @Primary
     DataSource dataSource(final DataSource primaryDataSource, final DataSource replicaDataSource) {
-        LazyConnectionDataSourceProxy lazyConnectionDataSourceProxy =
-                new LazyConnectionDataSourceProxy(primaryDataSource);
-        lazyConnectionDataSourceProxy.setReadOnlyDataSource(replicaDataSource);
-        return lazyConnectionDataSourceProxy;
+        Map<Object, Object> targetDataSources = new HashMap<>();
+        targetDataSources.put("primary", primaryDataSource);
+        targetDataSources.put("replica", replicaDataSource);
+
+        AbstractRoutingDataSource routingDataSource = new AbstractRoutingDataSource() {
+            @Override
+            protected Object determineCurrentLookupKey() {
+                return TransactionSynchronizationManager.isCurrentTransactionReadOnly() ? "replica" : "primary";
+            }
+        };
+        routingDataSource.setDefaultTargetDataSource(primaryDataSource);
+        routingDataSource.setTargetDataSources(targetDataSources);
+        return routingDataSource;
     }
 }

Would you like assistance in implementing the routing logic using AbstractRoutingDataSource?

Committable suggestion skipped: line range outside the PR's diff.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai, as of spring framework 6.1 we have setReadOnlyDataSource in LazyConnectionDataSourceProxy

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rajadilipkolli, thank you for the clarification. Since the project is using Spring Framework 6.1, the setReadOnlyDataSource method in LazyConnectionDataSourceProxy is available. I've noted this for future reviews.

✏️ Learnings added 
Learnt from: rajadilipkolli
PR: rajadilipkolli/my-spring-boot-experiments#1491
File: jpa/boot-read-replica-postgresql/src/main/java/com/example/demo/readreplica/config/DatabaseConfig.java:51-54
Timestamp: 2024-11-02T15:00:53.580Z
Learning: In Java Spring Boot projects using Spring Framework 6.1 or later, the `LazyConnectionDataSourceProxy` class includes the `setReadOnlyDataSource` method, as used in `DatabaseConfig.java`.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

}
}
35 changes: 0 additions & 35 deletions ...resql/src/main/java/com/example/demo/readreplica/config/aop/ReadOnlyRouteInterceptor.java
View file
Open in desktop

This file was deleted.

26 changes: 0 additions & 26 deletions ...stgresql/src/main/java/com/example/demo/readreplica/config/routing/RoutingDataSource.java
View file
Open in desktop

This file was deleted.

53 changes: 46 additions & 7 deletions jpa/boot-read-replica-postgresql/src/main/resources/application.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,62 @@ spring:
datasource:
password: postgres_write
username: postgres_write
driver: org.postgresql.Driver
driverClassName: org.postgresql.Driver
url: jdbc:postgresql://localhost:5432/my_database
configuration:
poolName: primaryHikariPool
hikari:
auto-commit: false
pool-name: primaryHikariPool
data-source-properties:
ApplicationName: ${spring.application.name}
Comment on lines +7 to +13
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider enhancing primary datasource configuration with additional Hikari settings.

The primary datasource configuration looks good, but could benefit from additional Hikari pool settings for production readiness.

Consider adding these important Hikari settings:

      hikari:
        maximum-pool-size: 10
        minimum-idle: 5
        idle-timeout: 300000
        connection-timeout: 20000
        max-lifetime: 1200000

replica:
datasource:
password: repl_password
username: repl_user
driver: org.postgresql.Driver
driverClassName: org.postgresql.Driver
url: jdbc:postgresql://localhost:15432/my_database
configuration:
hikari:
auto-commit: false
poolName: replicaHikariPool

################ Database #####################
data:
jpa:
repositories:
bootstrap-mode: deferred
jpa:
open-in-view: false
show-sql: false
hibernate:
ddl-auto: validate
properties:
hibernate:
connection:
provider_disables_autocommit: true
jdbc:
time_zone: UTC
batch_size: 25
lob.non_contextual_creation : true
generate_statistics: false
order_inserts: true
order_updates: true
query:
fail_on_pagination_over_collection_fetch: true
in_clause_parameter_padding: true
plan_cache_max_size: 4096
threads:
virtual:
enabled: true
enabled: true

# spring boot log level property
logging:
level:
read-replica-logger: DEBUG
jdbc:
datasource-proxy:
enabled: true
multiline: false
logging: slf4j
query:
logger-name: read-replica-logger
log-level: DEBUG
enable-logging: true
excluded-data-source-bean-names: dataSource
28 changes: 18 additions & 10 deletions ...ca-postgresql/src/test/java/com/example/demo/readreplica/ReadReplicaApplicationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.awaitility.Awaitility.await;

import com.example.demo.readreplica.config.routing.RoutingDataSource;
import java.time.LocalDateTime;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import javax.sql.DataSource;
import net.ttddyy.dsproxy.support.ProxyDataSource;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.MethodOrderer;
import org.junit.jupiter.api.Order;
Expand All @@ -19,13 +18,17 @@
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.jdbc.UncategorizedSQLException;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.datasource.LazyConnectionDataSourceProxy;
import org.springframework.test.context.ActiveProfiles;
import org.springframework.test.util.ReflectionTestUtils;

@ActiveProfiles("test")
@SpringBootTest
@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
class ReadReplicaApplicationTests {

@Autowired private RoutingDataSource routingDataSource;
@Autowired private DataSource dataSource;

private JdbcTemplate primaryJdbcTemplate;

Expand All @@ -38,13 +41,18 @@ class ReadReplicaApplicationTests {

@BeforeAll
void setUp() {
assertThat(routingDataSource).isNotNull();
Map<Object, DataSource> resolvedDataSources = routingDataSource.getResolvedDataSources();
assertThat(resolvedDataSources).isNotEmpty().hasSize(2);
primaryJdbcTemplate =
new JdbcTemplate(resolvedDataSources.get(RoutingDataSource.Route.PRIMARY));
replicaJdbcTemplate =
new JdbcTemplate(resolvedDataSources.get(RoutingDataSource.Route.REPLICA));
assertThat(dataSource).isNotNull().isInstanceOf(LazyConnectionDataSourceProxy.class);
LazyConnectionDataSourceProxy lazyConnectionDataSourceProxy =
(LazyConnectionDataSourceProxy) dataSource;
DataSource targetDataSource = lazyConnectionDataSourceProxy.getTargetDataSource();
assertThat(targetDataSource).isNotNull().isInstanceOf(ProxyDataSource.class);
ProxyDataSource proxyDataSource = (ProxyDataSource) targetDataSource;
primaryJdbcTemplate = new JdbcTemplate(proxyDataSource.getDataSource());
Object readOnlyDataSource =
ReflectionTestUtils.getField(lazyConnectionDataSourceProxy, "readOnlyDataSource");
assertThat(readOnlyDataSource).isNotNull().isInstanceOf(ProxyDataSource.class);
proxyDataSource = (ProxyDataSource) readOnlyDataSource;
replicaJdbcTemplate = new JdbcTemplate(proxyDataSource.getDataSource());
}

@Test
Expand Down
2 changes: 2 additions & 0 deletions jpa/boot-read-replica-postgresql/src/test/resources/application-test.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
#since tests uses only jdbc template, connection optimizations handled by jps is gone, to enabling autocommit
spring.primary.datasource.hikari.auto-commit=true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Multiple datasource configurations found in test environment

The verification revealed additional datasource configurations in test files that need attention:

  • Found conflicting auto-commit settings:
    • spring.primary.datasource.hikari.auto-commit=false in main configuration
    • spring.primary.datasource.hikari.auto-commit=true in test override
  • Found multiple database configurations including PostgreSQL, Oracle, and MariaDB drivers
  • The comment suggests auto-commit is enabled only for JDBC template tests, but this isn't clearly isolated
🔗 Analysis chain

Configuration looks good and aligns with the new architecture.

The auto-commit setting is appropriately scoped to the test environment and makes sense given the removal of routing-based approach in favor of LazyConnectionDataSourceProxy.

Let's verify this is the only test-specific datasource configuration:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any other test-specific datasource configurations
rg -g '*/test/**/application*.{properties,yml}' 'spring.*datasource'

Length of output: 219

Script:

#!/bin/bash
# Let's try a broader search for test configuration files first
fd "application.*\.(properties|yml)" test/

# Then search for datasource configurations in these files
find . -path "*/test/*" -name "application*.properties" -o -name "application*.yml" | xargs cat 2>/dev/null

Length of output: 4019