ShoppingCart-dev-009-high-concurrency - Part 2

[yennanliu]

Approach 4 — Redisson distributed lock:

• RedissonConfig: single-server Redisson client bean
• CartService.addToCart: per-user lock (cart:user:)
• OrderService.placeOrder: per-user lock (order:user:) prevents duplicate orders

Approach 5 — Read replica routing:

• DataSourceConfig: AbstractRoutingDataSource routes read-only txns to replica
• Enabled via app.datasource.replica.enabled=true (off by default)
• ProductService, CategoryService: @transactional(readOnly=true) on read methods

Approach 7 — Horizontal scaling:

• docker-compose-ha.yml: multi-replica app behind Nginx (--scale app=N)
• nginx/nginx.conf: least_conn upstream, health endpoint
• Actuator health endpoint exposed for load-balancer probes

All 87 tests pass.

[gemini-code-assist]

Code Review

This pull request introduces distributed locking using Redisson, read-replica routing, and horizontal scaling with Nginx. Key changes include the implementation of a RoutingDataSource for database read/write splitting and the application of distributed locks in the CartService and OrderService to handle concurrency. Review feedback identifies critical issues regarding the read-replica routing logic, specifically the requirement to wrap the data source in a LazyConnectionDataSourceProxy and the need to use Spring's @transactional annotation instead of the Jakarta version. Additionally, feedback was provided to ensure atomicity and idempotency during the order placement process.

[gemini-code-assist]

To support read-replica routing based on the @Transactional(readOnly = true) attribute, you should add the import for LazyConnectionDataSourceProxy.

Suggested change

	import org.springframework.transaction.support.TransactionSynchronizationManager;
	import org.springframework.transaction.support.TransactionSynchronizationManager;
	import org.springframework.jdbc.datasource.LazyConnectionDataSourceProxy;

[gemini-code-assist]

The RoutingDataSource must be wrapped in a LazyConnectionDataSourceProxy. Without this, Spring's transaction manager will fetch a connection from the data source before the transaction synchronization (and thus the read-only status) is initialized, causing the routing to always default to the primary data source.

Suggested change

	@Primary
	@Bean
	public DataSource dataSource(
	@Qualifier("primaryDataSource") DataSource primary,
	@Qualifier("replicaDataSource") DataSource replica) {
	RoutingDataSource routing = new RoutingDataSource();
	Map<Object, Object> targets = new HashMap<>();
	targets.put("primary", primary);
	targets.put("replica", replica);
	routing.setTargetDataSources(targets);
	routing.setDefaultTargetDataSource(primary);
	return routing;
	}
	@Primary
	@Bean
	public DataSource dataSource(
	@Qualifier("primaryDataSource") DataSource primary,
	@Qualifier("replicaDataSource") DataSource replica) {
	RoutingDataSource routing = new RoutingDataSource();
	Map<Object, Object> targets = new HashMap<>();
	targets.put("primary", primary);
	targets.put("replica", replica);
	routing.setTargetDataSources(targets);
	routing.setDefaultTargetDataSource(primary);
	return new LazyConnectionDataSourceProxy(routing);
	}

[gemini-code-assist]

The placeOrder method lacks atomicity. It performs multiple database writes (saving the order, saving multiple order items, and deleting cart items) which must be wrapped in a transaction to ensure consistency. Additionally, when using a distributed lock for idempotency, you should check if the cart is empty after acquiring the lock to avoid creating duplicate empty orders if a previous request already processed the cart.

Note: Be aware that the lock should ideally wrap the transaction scope to prevent race conditions between the unlock and the transaction commit.

    @Transactional
    public void placeOrder(User user, String sessionId) {

        RLock lock = redissonClient.getLock("order:user:" + user.getId());
        try {
            lock.lock(30, TimeUnit.SECONDS);

            CartDto cartDto = cartService.listCartItems(user);
            List<CartItemDto> cartItemDtoList = cartDto.getCartItems();

            if (cartItemDtoList.isEmpty()) {
                return;
            }

            Order newOrder = new Order();
            newOrder.setCreatedDate(new Date());
            newOrder.setSessionId(sessionId);
            newOrder.setUser(user);
            newOrder.setTotalPrice(cartDto.getTotalCost());
            orderRepository.save(newOrder);

            for (CartItemDto cartItemDto : cartItemDtoList) {
                OrderItem orderItem = new OrderItem();
                orderItem.setCreatedDate(new Date());
                orderItem.setPrice(cartItemDto.getProduct().getPrice());
                orderItem.setProduct(cartItemDto.getProduct());
                orderItem.setQuantity(cartItemDto.getQuantity());
                orderItem.setOrder(newOrder);
                orderItemsRepository.save(orderItem);
            }

            cartService.deleteUserCartItems(user);
        } finally {
            if (lock.isHeldByCurrentThread()) lock.unlock();
        }
    }

[gemini-code-assist]

To support the read-replica routing feature (Approach 5), you should use org.springframework.transaction.annotation.Transactional instead of jakarta.transaction.Transactional, as the latter does not support the readOnly attribute used by the routing logic.

Suggested change

	import org.redisson.api.RLock;
	import org.redisson.api.RedissonClient;
	import org.redisson.api.RLock;
	import org.redisson.api.RedissonClient;
	import org.springframework.transaction.annotation.Transactional;

[yennanliu]

================ 2 ================

[yennanliu]

/gemini review

[gemini-code-assist]

Code Review

This pull request significantly hardens the application for high-concurrency workloads by introducing a multi-stage Docker architecture, horizontal scaling via Nginx, and Redis-based distributed locking for critical sections in the cart and order services. It also implements read-replica routing for database operations and updates the stack to Spring Boot 3.2 and Java 17. Review feedback identifies several critical improvements: the placeOrder method requires @Transactional for atomicity, distributed locks should ideally use tryLock with timeouts to prevent thread exhaustion, and the Redisson connection pool size should be increased to handle higher loads. Additionally, it is recommended to replace hardcoded data source routing keys with constants for better maintainability.

[gemini-code-assist]

Using lock.lock(10, TimeUnit.SECONDS) is a blocking operation that waits indefinitely for the lock to become available. In a high-concurrency system, this can lead to thread pool exhaustion if Redis latency increases or contention is high. It is generally safer to use tryLock with a timeout to fail fast and avoid hanging threads. Note that tryLock throws a checked InterruptedException which would need to be handled.

[gemini-code-assist]

The placeOrder method is missing the @Transactional annotation. Since it performs multiple database operations across different repositories (orderRepository, orderItemsRepository) and services (cartService), it must be atomic to prevent partial data persistence in case of failure.

Additionally, when combining distributed locks with transactions, the lock should be acquired before the transaction starts and released after the transaction commits. If you annotate this method with @Transactional, the transaction will start before the lock is acquired (due to Spring's AOP proxy), which can lead to race conditions where a second thread acquires the lock before the first thread's changes are committed to the database. Consider splitting this into a non-transactional wrapper that handles the lock and calls a separate transactional method for the business logic.

[gemini-code-assist]

The routing keys "primary" and "replica" are hardcoded as string literals in multiple places (lines 51, 52, 54, 63, 64). It is recommended to define these as constants to ensure consistency and avoid potential bugs from typos during future maintenance.

[gemini-code-assist]

A connection pool size of 10 is likely too low for a high-concurrency application, especially when distributed locks are used frequently. This could become a bottleneck under heavy load as threads wait for available Redis connections.

Suggested change

	.setConnectionPoolSize(10);
	.setConnectionPoolSize(64);