Skip to content

fix: limit the scope of when a single transaction is used for import - #37990

Merged
shawkins merged 1 commit into
keycloak:mainfrom
shawkins:iss34364
Mar 24, 2025
Merged

fix: limit the scope of when a single transaction is used for import#37990
shawkins merged 1 commit into
keycloak:mainfrom
shawkins:iss34364

Conversation

@shawkins

Copy link
Copy Markdown
Contributor

closes: #34364

closes: keycloak#34364

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
@Override
public boolean useExistingSession() {
return true;
return ExportImportConfig.isSingleTransaction();

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.

This was already true before the change. We're now calling ExportImportConfig.setSingleTransaction(true); which should have the same effect, no? Are we sure this is the correct fix?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We're setting it to true now for only one specific import case - when we also need to create the bootstrap admin user.

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.

Ah, right. Thanks for the explanation.

@vmuzikar vmuzikar 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.

Thanks you for the PR, looks good to me.

I wonder if we should run the benchmark with this change just to be sure.
@ahus1 Does the benchmark include import/export scenarios with larger amount of users?

@Override
public boolean useExistingSession() {
return true;
return ExportImportConfig.isSingleTransaction();

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.

Ah, right. Thanks for the explanation.

@stylepatrick

Copy link
Copy Markdown

Thanks for the fix! Looks good!

@ahus1

ahus1 commented Mar 15, 2025

Copy link
Copy Markdown
Member

@vmuzikar - no, there is export/import scenario with a larger amount of users in the benchmark project.

@shawkins shawkins added this to the 26.2.0 milestone Mar 18, 2025
@shawkins
shawkins merged commit c0da146 into keycloak:main Mar 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

User import gets exponentially slow

5 participants