spring-data-redis icon indicating copy to clipboard operation
spring-data-redis copied to clipboard

Add multiple reactive keys exist checker

Open AnneMayor opened this issue 1 year ago • 5 comments

  • [x] You have read the Spring Data contribution guidelines.
  • [x] You use the code formatters provided here and have them applied to your changes. Don’t submit any formatting related changes.
  • [x] You submit test cases (unit or integration tests) that back your changes.
  • [ ] You added yourself as author in the headers of the classes you touched. Amend the date range in the Apache license header if needed. For new types, add the license header (copy from another file and set the current year only).

Resolved #2883

AnneMayor avatar May 26 '24 14:05 AnneMayor

Hey @AnneMayor, I just wanted to follow up on the topic :) Is this being planned to be merged soon?

kutlueren avatar Aug 23 '24 09:08 kutlueren

Hey @kutlueren , Unfortunately I have no feedback from our maintainer🥲 I am still waiting for their feedback.

AnneMayor avatar Aug 23 '24 13:08 AnneMayor

Hey @kutlueren , Unfortunately I have no feedback from our maintainer🥲 I am still waiting for their feedback.

@mp911de would you mind taking a look at this one when you have time?

kutlueren avatar Aug 23 '24 15:08 kutlueren

@mp911de , Please let me know if this PR has some issues🙏

AnneMayor avatar Sep 03 '24 09:09 AnneMayor

I did a review pass and left a comment regarding the design and implementation. Care to have a look?

Thanks a lot. I am going to apply your review within this weekend 👍

AnneMayor avatar Sep 04 '24 14:09 AnneMayor

Any luck solving this?

I have the same issue. Wanting to check the existence of multiple keys using exists in a single network call, without resorting to Lua, but using Springs own implementation

dreamstar-enterprises avatar Jan 15 '25 17:01 dreamstar-enterprises

My current workaround, since the spring API does not support exists with multiple keys yet.

/**
     * Executes expired session cleanup in Redis.
     *
     * Spring Session Redis uses multiple keys per session:
     * 1. Hash key (namespace:sessions:{id}) - Stores session data with 35 min TTL
     * 2. ZSet entry - Tracks session expiration times for batch cleanup
     *
     * Process flow:
     * 1. Query ZSet for sessions that should be expired
     * 2. Access their hash keys to trigger Redis expiration events
     *    - Redis only guarantees expiration events when keys are accessed
     *    - Spring Session listens for these events to cleanup related keys
     * 3. Remove processed entries from the ZSet
     *
     * Implementation details:
     * - Uses Lua script for efficient multi-key EXISTS checks
     * - Handles quoted session IDs in ZSet vs unquoted in hash keys
     * - Relies on Redis keyspace events for actual session deletion
     * - Bounded elastic scheduling for background processing
     *
     * @param context Operation boundaries and limits for ZSet query
     * @return Mono<Void> completing when cleanup process finishes
     * @throws RedisConnectionException if Redis operations fail
     *
     * @see "https://docs.spring.io/spring-session/reference/configuration/reactive-redis-indexed.html#how-spring-session-cleans-up-expired-sessions"
     */
    private fun cleanupExpiredSessions(context: CleanupContext): Mono<Void> =
        redisOperations.opsForZSet()
            .reverseRangeByScore(redisKeyExpirations, context.range, context.limit)
            .collectList()
            .flatMap { sessionIds ->
                if (sessionIds.isEmpty()) {
                    logger.debug("No expired sessions found")
                    Mono.empty()
                } else {
                    logger.debug("Found {} expired sessions to remove", sessionIds.size)

                    // Prepare session keys (remove quotes around session id in zset)
                    val sessionKeys = sessionIds.map { sessionId ->
                        "${sessionProperties.redis?.sessionNamespace}:sessions:${sessionId.trim('"')}"
                    }.also { keys ->
                        logger.debug("Checking Redis keys: {}", keys)
                    }

                    // First read the hash keys to trigger expiration event
                    val script = """
                       local keys = ARGV
                       local count = 0
                       for i, key in ipairs(keys) do
                           count = count + redis.call('EXISTS', key)
                       end
                       return count
                    """

                    redisOperations.execute(
                        RedisScript.of(script, Long::class.java),
                        listOf(),  // KEYS list is empty
                        sessionKeys
                    )
                    .doOnNext { count ->
                        logger.debug("Accessed {} session hash keys", count)
                    }
                    .flatMap { _ ->
                        redisOperations.opsForZSet()
                            .remove(redisKeyExpirations, *sessionIds.toTypedArray())
                            .doOnSuccess { logger.debug("Successfully removed {} expired sessions", sessionIds.size) }
                            .doOnError { e -> logger.error("Error during removal: ${e.message}") }
                    }
                    .then()
                }
            }
            .subscribeOn(Schedulers.boundedElastic())

dreamstar-enterprises avatar Jan 15 '25 17:01 dreamstar-enterprises

@dreamstar-enterprises Oh, for real? I am gonna check. Sorry for the late response. Since I have some business, I ended up keeping going this PR again. Will review your issue and give some feedback.

AnneMayor avatar Jan 18 '25 11:01 AnneMayor

Sorry for being late. I have lots of personal business and I am going to resolve this PR comments until this weekend. Please, keep that in mind.

AnneMayor avatar Mar 06 '25 14:03 AnneMayor

@mp911de could you review this PR again for me? Also, I have a question. Do you think that it would be better if I add this exist method into class ReactiveRedisOperations?

AnneMayor avatar Mar 08 '25 13:03 AnneMayor

No worries, some pull requests remain open for quite some time because we have to prioritize sometimes other things and so sometimes, pull requests don't get much attention.

In any case, thank you for your contribution. That's merged and polished now.

mp911de avatar Mar 12 '25 13:03 mp911de