8000 JPA map storage: User / client session no-downtime store by vramik · Pull Request #12241 · keycloak/keycloak · GitHub
[go: up one dir, main page]

Skip to content

JPA map storage: User / client session no-downtime store - #12241

Merged
abstractj merged 1 commit into
keycloak:mainfrom
vramik:9666-user-client-sessio-store
Jul 14, 2022
Merged

abstractj merged 1 commit into
keycloak:mainfrom
vramik:9666-user-client-sessio-store

Conversation

@vramik
@vramik vramik commented May 30, 2022
Copy link
Copy Markdown
Contributor

Closes #9666

@vramik
vramik force-pushed the 9666-user-client-sessio-store branch from 93bc998 to bb3a8e9 Compare May 31, 2022 06:04
@vramik
vramik force-pushed the 9666-user-client-sessio-store branch from 9a93e6d to 7838a98 Compare June 9, 2022 09:44
@hmlnarik
hmlnarik requested a review from sguilhen June 16, 2022 10:31
@vramik
vramik force-pushed the 9666-user-client-sessio-store branch from 7838a98 to 4f94557 Compare June 23, 2022 15:56
@vramik
vramik force-pushed the 9666-user-client-sessio-store branch from 4f94557 to 56db171 Compare June 23, 2022 16:55
@vramik
vramik marked this pull request as ready for review June 23, 2022 16:56
@vramik vramik added status/ready area/storage Storage layer — JPA entities, database schemas, Liquibase migrations, caching labels Jun 23, 2022
@ahus1 ahus1 left a comment
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi @vramik - great work! I'm happy to see that all the uglies of my first attempts to tackle to concurrency problems are now gone.

See below for a comment about CockroachDB, and I wondered if the table kc_user_session_client_session would be necessary.

@vramik
vramik force-pushed the 9666-user-client-sessio-store branch 2 times, most recently from ff9d967 to b0d9709 Compare June 26, 2022 19:25
@vramik
vramik force-pushed the 9666-user-client-sessio-store branch 4 times, most recently from 6c07b35 to 97cf6fc Compare July 13, 2022 19:38
@vramik
vramik requested review from ahus1 and sguilhen July 13, 2022 21:05
@ahus1 ahus1 left a comment
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi @vramik - this looks great; the table structure with client and user sessions looks now a lot simpler! I tested it locally it it all works as expected.

With your comments, you identified the two locking scenarios that are different from the others places. I've added my replies there on where the locking should be moved.

Co-authored-by: Alexander Schwartz <alexander.schwartz@gmx.net>

Closes keycloak#9666
@vramik
vramik force-pushed the 9666-user-client-sessio-store branch from 97cf6fc to 3969abe Compare July 14, 2022 10:08
@ahus1
ahus1 self-requested a review July 14, 2022 12:40
@ahus1 ahus1 left a comment
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi @vramik - this looks great! And all green!

@sguilhen sguilhen left a comment
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.

Great work, @vramik ! I will run it with CRDB later but I don't think we should wait for those results given that it works with Postgres. Approving.

@abstractj
abstractj merged commit ec853a6 into keycloak:main Jul 14, 2022
@vramik
vramik deleted the 9666-user-client-sessio-store branch July 14, 2022 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

5711
area/storage Storage layer — JPA entities, database schemas, Liquibase migrations, caching

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JPA map storage: User / client session no-downtime store

5 participants

0