7:18 p.m.
I sent PR https://github.com/keycloak/keycloak/pull/5716 for fixes and
performance improvements about the preloading of offline sessions. With
this, the preloading of offline sessions at startup is still required,
however it is highly optimized. Without this, preloading of 500K
sessions on my laptop took 16 minutes (some customers reported 40
minutes in real environment). Now it is 18 seconds on my laptop.
Moreover the preloading time was previously exponentially dependent to
the total count of sessions, now it is lineary dependent.
I know it will be even better to remove a need for preloading entirely,
but there are few kinks around it. Hopefully in the future, we can
delegate persistence of userSessions to infinispan entirely.
The summary of changes in the PR:
- It is not required anymore to lookup User at the startup when the
UserSession is preloaded. In case that user is not available anymore at
the time of preloading (which can happen especially with 3rd party
UserStorage providers, for example when user was directly deleted from
LDAP), then UserSession will be removed anyway later from both
infinispan and DB due the fact that it will become expired as user
doesn't exists and it won't be possible to refresh token.
- The pagination SQL queries are based on "seeking" rather then classic
pagination based on offset. See
https://www.eversql.com/faster-pagination-in-mysql-why-order-by-with-limi...
and https://use-the-index-luke.com/sql/partial-results/fetch-next-page
for details. This proved to have much better performance (at least for
MySQL, but probably for the other DBs too)
- DB Index added for column LAST_SESSION_REFRESH of table
OFFLINE_USER_SESSION for the seeking above
- Sessions are put to infinispan through the "cache.putAll" instead of
many separate calls of "cache.put" . This is important especially for
cluster and cross-dc, as when you call "cache.put" 100 times, you may
have around 100 network calls, when with "cache.putAll" it is 1 bigger
network call.
- When some offline sessions are refreshed, there is bulk update of
lastSessionRefresh times to DB every minute. So there is no DB write for
each token refresh, but bulk update similarly like we have for cross-dc.
This allows more proper tracking of expired sessions from both DB and
Infinispan and there is no need for a workaround to automatically update
all sessions at startup to the current time (which was problematic for
environments with often restarts as offline sessions were defacto never
expired). JIRA with the details is
https://issues.jboss.org/browse/KEYCLOAK-5479 .
A side-effect is, that UserSessionInitializerTest seems to be fixed now
- https://issues.jboss.org/browse/KEYCLOAK-8723 . TBH I am not sure
about exact cause why it is failing on master, however with latest
master, it failed for me after 50 runs when running this test in a loop
on my laptop. Similar behaviour was seen by Sebastian as well on his
laptop. With this PR, I had more than 300 runs without any failure.
Marek
10:42 a.m.
On Tue, Nov 13, 2018 at 7:47 PM Marek Posolda <mposolda(a)redhat.com> wrote:
> Marek
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
I sent PR https://github.com/keycloak/keycloak/pull/5716 for fixes
and
performance improvements about the preloading of offline sessions. With
this, the preloading of offline sessions at startup is still required,
however it is highly optimized. Without this, preloading of 500K
sessions on my laptop took 16 minutes (some customers reported 40
minutes in real environment). Now it is 18 seconds on my laptop.
Moreover the preloading time was previously exponentially dependent to
the total count of sessions, now it is lineary dependent.
I know it will be even better to remove a need for preloading entirely,
but there are few kinks around it. Hopefully in the future, we can
delegate persistence of userSessions to infinispan entirely.
The summary of changes in the PR:
- It is not required anymore to lookup User at the startup when the
UserSession is preloaded. In case that user is not available anymore at
the time of preloading (which can happen especially with 3rd party
UserStorage providers, for example when user was directly deleted from
LDAP), then UserSession will be removed anyway later from both
infinispan and DB due the fact that it will become expired as user
doesn't exists and it won't be possible to refresh token.
- The pagination SQL queries are based on "seeking" rather then classic
pagination based on offset. See
https://www.eversql.com/faster-pagination-in-mysql-why-order-by-with-limi...
and https://use-the-index-luke.com/sql/partial-results/fetch-next-page
for details. This proved to have much better performance (at least for
MySQL, but probably for the other DBs too)
- DB Index added for column LAST_SESSION_REFRESH of table
OFFLINE_USER_SESSION for the seeking above
- Sessions are put to infinispan through the "cache.putAll" instead of
many separate calls of "cache.put" . This is important especially for
cluster and cross-dc, as when you call "cache.put" 100 times, you may
have around 100 network calls, when with "cache.putAll" it is 1 bigger
network call.
- When some offline sessions are refreshed, there is bulk update of
lastSessionRefresh times to DB every minute. So there is no DB write for
each token refresh, but bulk update similarly like we have for cross-dc.
This allows more proper tracking of expired sessions from both DB and
Infinispan and there is no need for a workaround to automatically update
all sessions at startup to the current time (which was problematic for
environments with often restarts as offline sessions were defacto never
expired). JIRA with the details is
https://issues.jboss.org/browse/KEYCLOAK-5479 .
A side-effect is, that UserSessionInitializerTest seems to be fixed now
- https://issues.jboss.org/browse/KEYCLOAK-8723 . TBH I am not sure
about exact cause why it is failing on master, however with latest
master, it failed for me after 50 runs when running this test in a loop
on my laptop. Similar behaviour was seen by Sebastian as well on his
laptop. With this PR, I had more than 300 runs without any failure.
That's good news Marek!
So when do we plan to merge the refactoring PR? Since we focus now on
stabilizing tests, this should get in ASAP...
11:40 a.m.
On 15/11/18 10:42, Sebastian Laskawiec wrote:
On Tue, Nov 13, 2018 at 7:47 PM Marek Posolda <mposolda(a)redhat.com
<mailto:mposolda@redhat.com>> wrote:
I sent PR https://github.com/keycloak/keycloak/pull/5716 for fixes
and
performance improvements about the preloading of offline sessions.
With
this, the preloading of offline sessions at startup is still
required,
however it is highly optimized. Without this, preloading of 500K
sessions on my laptop took 16 minutes (some customers reported 40
minutes in real environment). Now it is 18 seconds on my laptop.
Moreover the preloading time was previously exponentially
dependent to
the total count of sessions, now it is lineary dependent.
I know it will be even better to remove a need for preloading
entirely,
but there are few kinks around it. Hopefully in the future, we can
delegate persistence of userSessions to infinispan entirely.
The summary of changes in the PR:
- It is not required anymore to lookup User at the startup when the
UserSession is preloaded. In case that user is not available
anymore at
the time of preloading (which can happen especially with 3rd party
UserStorage providers, for example when user was directly deleted
from
LDAP), then UserSession will be removed anyway later from both
infinispan and DB due the fact that it will become expired as user
doesn't exists and it won't be possible to refresh token.
- The pagination SQL queries are based on "seeking" rather then
classic
pagination based on offset. See
https://www.eversql.com/faster-pagination-in-mysql-why-order-by-with-limi...
and
https://use-the-index-luke.com/sql/partial-results/fetch-next-page
for details. This proved to have much better performance (at least
for
MySQL, but probably for the other DBs too)
- DB Index added for column LAST_SESSION_REFRESH of table
OFFLINE_USER_SESSION for the seeking above
- Sessions are put to infinispan through the "cache.putAll"
instead of
many separate calls of "cache.put" . This is important especially for
cluster and cross-dc, as when you call "cache.put" 100 times, you may
have around 100 network calls, when with "cache.putAll" it is 1
bigger
network call.
- When some offline sessions are refreshed, there is bulk update of
lastSessionRefresh times to DB every minute. So there is no DB
write for
each token refresh, but bulk update similarly like we have for
cross-dc.
This allows more proper tracking of expired sessions from both DB and
Infinispan and there is no need for a workaround to automatically
update
all sessions at startup to the current time (which was problematic
for
environments with often restarts as offline sessions were defacto
never
expired). JIRA with the details is
https://issues.jboss.org/browse/KEYCLOAK-5479 .
A side-effect is, that UserSessionInitializerTest seems to be
fixed now
- https://issues.jboss.org/browse/KEYCLOAK-8723 . TBH I am not sure
about exact cause why it is failing on master, however with latest
master, it failed for me after 50 runs when running this test in a
loop
on my laptop. Similar behaviour was seen by Sebastian as well on his
laptop. With this PR, I had more than 300 runs without any failure.
That's good news Marek!
So when do we plan to merge the refactoring PR? Since we focus now on
stabilizing tests, this should get in ASAP...
I hope for today afternoon/evening.
Marek
Marek
_______________________________________________
keycloak-dev mailing list
keycloak-dev(a)lists.jboss.org <mailto:keycloak-dev@lists.jboss.org>
https://lists.jboss.org/mailman/listinfo/keycloak-dev
2266
days inactive
2268
days old
2 comments
2 participants
participants (2)
-
Marek Posolda -
Sebastian Laskawiec