URL: https://github.com/SSSD/sssd/pull/5546 Author: pbrezina Title: #5546: kcm: add GET_CRED_LIST for faster iteration Action: opened
PR body: """ For large caches, one IPC operation per credential dominates the cost of iteration. Instead transfer the whole list of credentials to the client in one IPC operation.
Resolves: https://github.com/SSSD/sssd/issues/5545
This is a continuation of https://github.com/SSSD/sssd/pull/5375. The first pull requests addressed bottlenecks in sssd-kcm and reduced the test case run time from 30 minutes to 2 minutes, this new operation takes it down to 9 seconds. """
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/5546/head:pr5546 git checkout pr5546
URL: https://github.com/SSSD/sssd/pull/5546 Title: #5546: kcm: add GET_CRED_LIST for faster iteration
pbrezina commented: """ Note that you need to build krb5 from upstream to test it: https://github.com/krb5/krb5/pull/1165 (already merged) """
See the full comment at https://github.com/SSSD/sssd/pull/5546#issuecomment-803937302
URL: https://github.com/SSSD/sssd/pull/5546 Title: #5546: kcm: add GET_CRED_LIST for faster iteration
Label: +Waiting for review
URL: https://github.com/SSSD/sssd/pull/5546 Title: #5546: kcm: add GET_CRED_LIST for faster iteration
sumit-bose commented: """ Hi,
the patches work well for me.
The feature is already covered by the current KCM integration tests, i.e. if the krb5 client library supports the extension it will be used in the tests. However there is no indication in the tests if the GET_CRED_LIST or GET_CRED_UUID_LIST is used. I wonder if it would be good to have such indication to avoid regressions on platform where we know that the extension is supported?
About `KCM_MIT_OFFSET`, I think it would be worth to add a comment explaining where this is coming from, e.g. that it is the opcode of the first call of the MIT extension.
bye, Sumit """
See the full comment at https://github.com/SSSD/sssd/pull/5546#issuecomment-810092973
URL: https://github.com/SSSD/sssd/pull/5546 Author: pbrezina Title: #5546: kcm: add GET_CRED_LIST for faster iteration Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/5546/head:pr5546 git checkout pr5546
URL: https://github.com/SSSD/sssd/pull/5546 Title: #5546: kcm: add GET_CRED_LIST for faster iteration
pbrezina commented: """
The feature is already covered by the current KCM integration tests, i.e. if the krb5 client library supports the extension it will be used in the tests. However there is no indication in the tests if the GET_CRED_LIST or GET_CRED_UUID_LIST is used. I wonder if it would be good to have such indication to avoid regressions on platform where we know that the extension is supported?
We can grep the logs and produce a debug message saying which operation was executed but this does not fail the test. I think the right thing to do is to let QA write a performance test that would be able to catch it? They just did manual verification for [rhbz#1876514](https://bugzilla.redhat.com/show_bug.cgi?id=)
About `KCM_MIT_OFFSET`, I think it would be worth to add a comment explaining where this is coming from, e.g. that it is the opcode of the first call of the MIT extension.
Done.
"""
See the full comment at https://github.com/SSSD/sssd/pull/5546#issuecomment-811009261
URL: https://github.com/SSSD/sssd/pull/5546 Title: #5546: kcm: add GET_CRED_LIST for faster iteration
sumit-bose commented: """
The feature is already covered by the current KCM integration tests, i.e. if the krb5 client library supports the extension it will be used in the tests. However there is no indication in the tests if the GET_CRED_LIST or GET_CRED_UUID_LIST is used. I wonder if it would be good to have such indication to avoid regressions on platform where we know that the extension is supported?
We can grep the logs and produce a debug message saying which operation was executed but this does not fail the test. I think the right thing to do is to let QA write a performance test that would be able to catch it? They just did manual verification for [rhbz#1876514](https://bugzilla.redhat.com/show_bug.cgi?id=)
ok
About `KCM_MIT_OFFSET`, I think it would be worth to add a comment explaining where this is coming from, e.g. that it is the opcode of the first call of the MIT extension.
Done.
Thanks for the update. The CI failure on rawhide does not look related, so ACK.
bye, Sumit """
See the full comment at https://github.com/SSSD/sssd/pull/5546#issuecomment-813925536
URL: https://github.com/SSSD/sssd/pull/5546 Title: #5546: kcm: add GET_CRED_LIST for faster iteration
Label: +Accepted
URL: https://github.com/SSSD/sssd/pull/5546 Title: #5546: kcm: add GET_CRED_LIST for faster iteration
Label: -Waiting for review
URL: https://github.com/SSSD/sssd/pull/5546 Title: #5546: kcm: add GET_CRED_LIST for faster iteration
Label: +Ready to push
URL: https://github.com/SSSD/sssd/pull/5546 Title: #5546: kcm: add GET_CRED_LIST for faster iteration
Label: +Pushed
URL: https://github.com/SSSD/sssd/pull/5546 Title: #5546: kcm: add GET_CRED_LIST for faster iteration
pbrezina commented: """ Pushed PR: https://github.com/SSSD/sssd/pull/5546
* `master` * 560e247904ca4b9b1da9895548263c4b9772f92b - kcm: add GET_CRED_LIST for faster iteration * 81130b23206aff7a4076f13b04b310352be8a23f - kcm: add support for MIT extensions * 9a39ceba2d4d503bd1c37166322e88aae0187520 - kcm: remove unneeded kcm.h
"""
See the full comment at https://github.com/SSSD/sssd/pull/5546#issuecomment-813948062
URL: https://github.com/SSSD/sssd/pull/5546 Title: #5546: kcm: add GET_CRED_LIST for faster iteration
Label: -Accepted
URL: https://github.com/SSSD/sssd/pull/5546 Title: #5546: kcm: add GET_CRED_LIST for faster iteration
Label: -Ready to push
URL: https://github.com/SSSD/sssd/pull/5546 Author: pbrezina Title: #5546: kcm: add GET_CRED_LIST for faster iteration Action: closed
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/5546/head:pr5546 git checkout pr5546
sssd-devel@lists.fedorahosted.org
-
pbrezina -
sumit-bose