spring-projects / spring-data-relational

Spring Data Relational. Home of Spring Data JDBC and Spring Data R2DBC.
https://spring.io/projects/spring-data-jdbc
Apache License 2.0
767 stars 345 forks source link

[Resilience Features for SQL execution] Timeout for @Query [DATAJDBC-638] #810

Closed spring-projects-issues closed 3 years ago

spring-projects-issues commented 3 years ago

MahatmaFatalError opened DATAJDBC-638 and commented

Motivation

I consider a relational database similar to any other external system in a distributed environment, hence I want to apply common resillience patterns like circuit brakers, timeout, bulkheads, rate-limiting etc.

 

Actually, some of those aspects can be handled to a certain degree by connection pool like Hikari. Similarly, often you can configure some general query timeouts on the jdbc driver level or on the concrete DataSource config.

However, what is missing is an easy per-query configuration for those queries that are special and should be treated differently. (Use case would be a quite long general timeout and for a few well-known queries a much shorter one)

Options to tackle this are e.g.:

JdbcTemplate template = new JdbcTemplate(dataSource);
 template.setQueryTimeout(1); //1 second
Statement stmt = connection.prepareStatement("SELECT * FROM TAB");
stmt.setQueryTimeout(10);//Timeout of 10 seconds

However, those options are too low-level and don't feel really spring-data-like.

Another option that comes into mind is spring-retry and @Transactional(timeout=1, readOnly=true). However, it feels not clean to me to play around with transactions (and potentially mess up with transaction propagation) only for a timeout.

Of course one can do it manuall:

Supplier<Iterable<Book>> supplier = () -> bookRepository.findAll(); // encapsulate db call into supplier
try {
        return CompletableFuture.supplyAsync(supplier).get(500, TimeUnit.MILLISECONDS);
} catch (InterruptedException | ExecutionException e) {
    throw new RuntimeException(e);
} catch ( TimeoutException e) {
    // Deal with the timeout, fallback to default or ignore it...
    throw new TransactionTimedOutException("Query timed out", e);
}

but this comes with heavy exception boiler plate.

If resillience4j is already in your stack one could go with heavy customization:

Retry retry = Retry.ofDefaults("db");

ThreadPoolBulkhead threadPoolBulkhead = ThreadPoolBulkhead.ofDefaults("db");

ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(10);
TimeLimiter timeLimiter = TimeLimiter.of(Duration.ofSeconds(1));

Supplier<Iterable<Book>> supplier = () -> bookRepository.findAll(); // encapsulate db call into supplier
Iterable<Book> result = Decorators.ofSupplier(supplier)
        .withRetry(retry)
        .withThreadPoolBulkhead(threadPoolBulkhead) // needed for time-limiter
        .withTimeLimiter(timeLimiter, scheduler)
        .get()
        .toCompletableFuture()
        .get();

None of these options feels great to configure resillience patterns like retry or timeout on a global level for DB connections and on the same time allow to overwrite the configuration for some individual queries in a handy manner.

Proposal

From my app-developer point of view the most convenient solution would be to add an optional timeout parameter to the @Query annotation. This way I could configure a global query timeout on the DataSource level and overwrite it for those queries (potentiall in combination with @Retryable) where I need it.

@Retryable( value = TransactionTimedOutException.class, maxAttempts = 2, backoff = @Backoff(delay = 100))
public interface MyRepository extends CrudRepository<Book, String> {

        @Query(timeOut = Duration.ofSeconds(1))
    Book mySpecialQuery();
}

 

 

 


No further details from DATAJDBC-638

spring-projects-issues commented 3 years ago

Jens Schauder commented

Thanks for the proposal. While we see your point at this point we don't want to get into resilience patterns.

You already described a couple of possible solutions. One thing I'd like to point out is that one has to take care of

a) possibly multiple threads accessing the same JdbcTemplate which all would be affected when a timeout gets changed, so one probably want something stored in a ThreadLocal to handle the timeout.

b) A single method might execute multiple queries and one needs to decide how to handle timeouts for those

spring-projects-issues commented 3 years ago

MahatmaFatalError commented

Jens Schauder Thank you for your explanations. Does it make sense to raise this topic to the JdbcTemplate colleagues?