@ahus1, I'm opening this draft PR here as I need your expertise. I'm using PGAdmin to test the indexes, but PostgreSQL seems to refuse to use them. In the queries below, the table has 7000 rows, and the queries match all rows.

Query 1

select sess.user_session_id, sess.user_id from public.offline_user_session sess 
where sess.realm_id = 'realm-0' 
AND sess.offline_flag = '0' 
AND sess.remember_me = true
AND sess.created_on > 1000

With remember_me = true, it uses the wrong index (!?)

Index Scan using idx_user_session_expiration_last_refresh on offline_user_session as sess
Filter: (created_on > 1000)
Index Cond: (((realm_id)::text = 'realm-0'::text) AND ((offline_flag)::text = '0'::text) AND (remember_me = true))

With remember_me = false, it does a sequential scan.

 Seq Scan on offline_user_session as sess
Filter: ((NOT remember_me) AND (created_on > 1000) AND ((realm_id)::text = 'realm-0'::text) AND ((offline_flag)::text = '0'::text))

Query 2

select sess.user_session_id, sess.user_id from public.offline_user_session sess 
where sess.realm_id = 'realm-0' 
AND sess.offline_flag = '0' 
AND sess.remember_me = true
AND sess.last_session_refresh > 1000

With remember_me = true, it uses the correct index

 Index Scan using idx_user_session_expiration_last_refresh on offline_user_session as sess
Index Cond: (((realm_id)::text = 'realm-0'::text) AND ((offline_flag)::text = '0'::text) AND (remember_me = true) AND (last_session_refresh > 1000))

With remember_me = false, it does a sequential scan.

 Seq Scan on offline_user_session as sess
Filter: ((NOT remember_me) AND (last_session_refresh > 1000) AND ((realm_id)::text = 'realm-0'::text) AND ((offline_flag)::text = '0'::text))

The disadvantage is the premature removal of valid sessions, as the UserSessionPersisterProvider#removeExpired(Realm) method will use the "non remember me" settings.

+1 for not migrating the sessions not during Keycloak upgrade. Still, I don't think a premature removal is a valid approach. Instead we would need to lazily update the values as part of the expiry job. I didn't look into the code yet, I might have a more specific suggestion later.

I'm using PGAdmin to test the indexes, but PostgreSQL seems to refuse to use them.

Did you run ANALYZE for the table first?

The disadvantage is the premature removal of valid sessions, as the UserSessionPersisterProvider#removeExpired(Realm) method will use the "non remember me" settings.

+1 for not migrating the sessions not during Keycloak upgrade. Still, I don't think a premature removal is a valid approach. Instead we would need to lazily update the values as part of the expiry job. I didn't look into the code yet, I might have a more specific suggestion later.

To allow for a graceful migration of the user sessions that are remember me vs. non-remember me, I suggest to allow null values in the REMEMBER_ME column.

I don't like adding extra complexity or cost to the expiration logic, as it will be executed frequently on all nodes.
I would rather implement UserSessionProvider#migrate(String) 😅

I'm using PGAdmin to test the indexes, but PostgreSQL seems to refuse to use them.

Did you run ANALYZE for the table first?

I did not; I only clicked in the "explain" query button 🤣

I don't like adding extra complexity or cost to the expiration logic, as it will be executed frequently on all nodes. I would rather implement UserSessionProvider#migrate(String)

I hear you. I'd say a java based migration will be too slow when there are millions of entries. Can you think of a plan SQL UPDATE statement that can be part of the liquibase changeset? It would do a lot or database IO (full table scan, updating some rows), but it would not need to do any batch handling in Java. WDYT?

Migrating the sessions table is then on the critical path for a Keycloak upgrade, and the table will be locked for changes when doing that.

Alternative approache:

• Backport the change to KC 26.4 to add the column, make in nullable, and fill it opportunistically, but never use it.
• When migrating to KC 26.5, update the missing null values on migration time (which is then hopefully shorter), and make it then non-null, and start using the column. This might then still be a full table scan, but would hopefully be faster as it would update fewer rows.

rebased and applied spotless rules.