Skip to content

KEYCLOAK-18039: Optimise offline session load on startup#8012

Merged
hmlnarik merged 1 commit intokeycloak:masterfrom
hmlnarik:KEYCLOAK-18039
May 13, 2021
Merged

KEYCLOAK-18039: Optimise offline session load on startup#8012
hmlnarik merged 1 commit intokeycloak:masterfrom
hmlnarik:KEYCLOAK-18039

Conversation

@hmlnarik
Copy link
Copy Markdown
Contributor

@hmlnarik hmlnarik commented May 7, 2021

Updates and replaces #7902

@hmlnarik hmlnarik added kind/bug Categorizes a PR related to a bug area/storage Indicates an issue that touches storage (change in data layout or data manipulation) labels May 7, 2021
@hmlnarik hmlnarik added this to the 14.0.0 milestone May 7, 2021
@hmlnarik hmlnarik self-assigned this May 7, 2021
@hmlnarik hmlnarik requested a review from mposolda May 11, 2021 06:36
Copy link
Copy Markdown
Contributor

@mposolda mposolda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hmlnarik PR looks like great work, Thanks for it! I've added one comment inline. Did you tried some performance test? What DB you used? Do you have some numbers for the reference?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have KeycloakTestTimeService . How about moving it to model/infinispan to avoid using same code on multiple places?

Same probably applies to DefaultInfinispanConnectionProviderFactory.replaceComponent which is same code as InfinispanTestUtil.replaceComponent . This code uses infinispan internal stuff and it is a chance that during infinispan upgrade to newer version, there will be a need to change this code. So if we have same code with infinispan internals on two places in Keycloak codebase, it possibly increases the work for the infinispan upgrade in the future IMO.

Or just simplify the testsuite - maybe we can remove InfinispanTestUtil entirely and change TestingResourceProvider.setTestingInfinispanTimeService and TestingResourceProvider.revertTestingInfinispanTimeService to call DefaultInfinispanConnectionProviderFactory.setUseKeycloakTimeService or something like that? WDYT?

@hmlnarik hmlnarik requested a review from mposolda May 13, 2021 10:41
@hmlnarik
Copy link
Copy Markdown
Contributor Author

Model test failed is a known instability: https://issues.redhat.com/browse/KEYCLOAK-16999

Copy link
Copy Markdown
Contributor

@mposolda mposolda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hmlnarik Thanks for the update. Maybe one last thing: It looks class KeycloakTestTimeService can be entirely removed now as it is not used anymore? It seems to be used only in the javadoc to TestingResource.setTestingInfinispanTimeService() , but maybe that javadoc can be slightly changed to not reference it anymore?

Copy link
Copy Markdown
Contributor

@mposolda mposolda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hmlnarik Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/storage Indicates an issue that touches storage (change in data layout or data manipulation) kind/bug Categorizes a PR related to a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants