-
Notifications
You must be signed in to change notification settings - Fork 83
Allow the dataset to generate users with a limited number of unique passwords #1238
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow the dataset to generate users with a limited number of unique passwords #1238
Conversation
…asswords Closes keycloak#1237 Signed-off-by: Ryan Emerson <[email protected]>
af93f47 to
bc2782a
Compare
ahus1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the pull request, looks good to me.
+1 for adding a minimal set of docs. I think it will come in handy for those who want to test millions of users. It is open source, and we shouldn't hide this gem in the code but make it useful for everyone.
Would be updating Gatling be part if this issue, or a follow-up issue? I'm ok to make it a follow-up issue.
dataset/src/main/java/org/keycloak/benchmark/dataset/config/DatasetConfig.java
Outdated
Show resolved
Hide resolved
dataset/src/main/java/org/keycloak/benchmark/dataset/DatasetResourceProvider.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Ryan Emerson <[email protected]>
Signed-off-by: Ryan Emerson <[email protected]>
Signed-off-by: Ryan Emerson <[email protected]>
Signed-off-by: Ryan Emerson <[email protected]>
|
I have added the docs and gatling benchmark logic, I also updated the I didn't update any of the benchmark workflows as they already have the max number of inputs (10) and I figured if we do want to execute these tests (semi-)regularly, we probably would have a dedicated cron workflow. |
pruivo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Tested the create-realm and benchmark run with 1 unique password, and it worked.
I'll let @ahus1 review the docs :)
Signed-off-by: Alexander Schwartz <[email protected]>
ahus1
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the code changes and the docs. Approving!
Closes #1237
I have kept it simple for now and only reused credentials per
create-usersinvocation. The reason for this is that theunique-credential-countultimately determines the password, so storing and reloading the credentials in the DB means that we then need to handleunique-credential-countchanging.Passwords can be calcuated using:
password-$(i % unique-credential-count)whereiis the user number.Quick local benchmark:
4s
./dataset/dataset-import.sh -a create-users -u 10000 -U 100 -l http://localhost:8080/realms/master/datasetvs
45s
./dataset/dataset-import.sh -a create-users -u 10000 -l http://localhost:8080/realms/master/datasetI haven't added any docs yet, as I wasn't sure if this was functionality we want to advertise or are happy keeping it hidden for our own needs. Let me know if we require docs.