Improve the performance of IDP update operation #6025
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6025 +/- ##
=============================================
- Coverage 45.67% 28.53% -17.15%
+ Complexity 14056 7035 -7021
=============================================
Files 1632 1483 -149
Lines 100532 89312 -11220
Branches 17629 13155 -4474
=============================================
- Hits 45918 25485 -20433
- Misses 47925 60401 +12476
+ Partials 6689 3426 -3263
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Copilot reviewed 5 out of 11 changed files in this pull request and generated 1 suggestion.
Files not reviewed (6)
- components/application-mgt/org.wso2.carbon.identity.application.mgt/src/main/java/org/wso2/carbon/identity/application/mgt/dao/impl/ApplicationMgtDBQueries.java: Evaluated as low risk
- components/idp-mgt/org.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/IdentityProviderManager.java: Evaluated as low risk
- components/application-mgt/org.wso2.carbon.identity.application.mgt/src/main/java/org/wso2/carbon/identity/application/mgt/dao/impl/CacheBackedApplicationDAO.java: Evaluated as low risk
- components/idp-mgt/org.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/util/IdPManagementConstants.java: Evaluated as low risk
- components/idp-mgt/org.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/dao/CacheBackedIdPMgtDAO.java: Evaluated as low risk
- components/idp-mgt/org.wso2.carbon.idp.mgt/src/main/java/org/wso2/carbon/idp/mgt/IdpManager.java: Evaluated as low risk
Comments skipped due to low confidence (1)
components/application-mgt/org.wso2.carbon.identity.application.mgt/src/main/java/org/wso2/carbon/identity/application/mgt/dao/impl/ApplicationDAOImpl.java:6605
- [nitpick] The error message could be more descriptive by including the tenant domain. Suggestion: 'Failed to update local and outbound config of application: ' + applicationId + ' in tenant domain: ' + tenantDomain
throw new IdentityApplicationManagementException("Failed to update local and outbound config of application: " + applicationId, e);
....mgt/src/main/java/org/wso2/carbon/identity/application/mgt/dao/impl/ApplicationDAOImpl.java
Outdated
Show resolved
Hide resolved
|
PR builder started |
|
|
PR builder completed |
|
PR builder started |
|
PR builder completed |
|
PR builder started |
|
PR builder completed |
There was a problem hiding this comment.
Approving the pull request based on the successful pr build https://github.com/wso2/product-is/actions/runs/12441877083
...rg/wso2/carbon/identity/application/mgt/listener/ApplicationIdentityProviderMgtListener.java
Show resolved
Hide resolved
Purpose
Previously, when an IDP update was performed, we had configured listeners
doPreUpdateIdpanddoPostUpdateIdpto update connected applications. However, the issue arose when more than 1000 applications were connected to an IDP. In these listeners, we iterated over all the service providers one by one, checking if the IDP/Authenticator update (such as disabling) was valid. If the default authenticator was changed, and the federated authentication flow used this IDP, the listener would automatically update the authenticator mapping in theSP_FEDERATED_IDPtable. This process was very expensive, triggering over 18,000 SQL queries even for a small IDP/Authenticator update.In this PR, I've enhanced the IDP update performance by removing the blind iteration through all connected SPs. Instead, I've introduced a SQL statement that checks whether a service provider refers to the IDP ,Authenticator or Outbound Connector, and if so, it prevents the IDP or Authenticator from being disabled. Another improvement is that we now retrieve the SP resource ID list based on the
AUTH_TYPE="federated"and only iterate through the affected service provider list to update the database.Performance Improvement (~2000 Connected Apps)
Related Issue
[1] -
Tested Scenarios - https://docs.google.com/document/d/1EuwbFXqLA1JHy6tcX_klbp0Fxbs_QiPBJKWUb30loOg/edit?usp=sharing