Describe the bug

For Redis user, the Modis "all index" rpush:notification:all will continue to grow even if they followed Wiki Using Redis - Database Cleanup:

This was previously pointed out in #525 (comment)

To Reproduce

Steps to reproduce the behavior:

1. Follow Wiki Using Redis - Database Cleanup to setup
2. Keep using Rpush for a while
3. Check the size taken by rpush:notification:all from Redis

Expected behavior

The size of rpush:notification:all should be reclaimed when rpush_notification:<id> are removed.

Logs or other outpus

After running Rpush for a while, rpush:notification:all is taking 3.7GB out of our 5GB Redis

Screenshots

System configuration (please complete the following information):

• OS:
• OS version:
• Ruby version: 3.3.4
• Rails version: 8.0.1
• Rpush version: 9.2.0

Additional context

Modis #destroy does both del from rpush:notifications:<id> as well as srem from rpush:notifications:all.

Proposed Solutions

A: Opt out of rpush:notification:all with enable_all_index false

I'm experimenting with #743
I didn't find any usage of Rpush::Client::Redis::Notification.all for now, but it feels a bit too simple, so please let me know if I missed anything obvious.

B: Asynchronous cleanup with another daily job

If we also want rpush:notifications:all to be cleanup after at least 24 hours (for some reason?), we can add another code snippet to the wiki for cleaning up orphaned IDs from rpush:notifications:all.

This works for me:

  def daily_rpush_notificatoin_index_cleanup
    Rails.logger.info '[Rpush] Starting cleanup of rpush:notifications:all set...'
    removed_count = 0
    batch_size = 1000

    Modis.with_connection do |redis|
      initial_count = redis.scard('rpush:notifications:all')
      Rails.logger.info "[Rpush] Initial count: #{initial_count} IDs in rpush:notifications:all set"

      cursor = '0'
      loop do
        cursor, ids = redis.sscan('rpush:notifications:all', cursor, count: batch_size)

        # Check existence of all keys in batch
        existence_results = redis.pipelined do |pipeline|
          ids.each do |id|
            pipeline.exists?("rpush:notifications:#{id}")
          end
        end

        ids_to_remove = ids.select.with_index { |id, idx| !existence_results[idx] }

        if ids_to_remove.any?
          removed = redis.srem('rpush:notifications:all', ids_to_remove)
          removed_count += removed
          Rails.logger.info "[Rpush] Removed #{removed} stale IDs from set (batch)"
        end

        break if cursor == '0'
      end

      final_count = redis.scard('rpush:notifications:all')
      Rails.logger.info "[Rpush] Cleanup task completed. Total: #{initial_count}, Removed: #{removed_count}, Remained: #{final_count}"
    end
  end

C: Synchronous cleanup with srem("rpush:notifications:all", [id])

Update Wiki Database Cleanup snippet to include srem("rpush:notifications:all", [id])

Something like:

    redis.keys('rpush:notifications:*').each do |key|
      next unless redis.ttl(key) == -1
      next unless redis.type(key) == 'hash'
      next if redis.hget(key, 'delivered').nil?
      redis.expire(key, 24.hours.to_i)
+     id = key.split(':').last
+     redis.srem('rpush:notifications:all', [id])
    end

Modis #destroy also use srem to update the all index (code).

This mean rpush:notifications:<id> is expired after 24h but <id> would be removed from all index immediately.

D: Recommend Modis #destroy for synchronous expiration

Like B, but replace both EXPIRE and SREM with just a Modis #destroy. However, I guess we didn't recommended this because we want asynchronous expiration for some reason?