-
Notifications
You must be signed in to change notification settings - Fork 10
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
feat(core): improve entitlements performance #1271
Conversation
Warning This pull request does not reference any issues. Please add a reference to an issue in the body of the pull request description. |
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.
@suchak1 this looks good. Could we add some comments to make sure others understand what's happening?
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.
This looks good to me.
@suchak1 Would the MatchSubjectMappings rpc have helped here? |
No, since the |
f1bb113
🤖 I have created a release *beep* *boop* --- ## [0.4.18](service/v0.4.17...service/v0.4.18) (2024-08-12) ### Features * **authz:** Remove external ers configuration from authorization ([#1265](#1265)) ([aa925a8](aa925a8)) * **authz:** Typed Entities ([#1249](#1249)) ([cfab3ad](cfab3ad)) * **core:** ability to run a set of isolated services ([#1245](#1245)) ([aa5636a](aa5636a)) * **core:** improve entitlements performance ([#1271](#1271)) ([f6a1b26](f6a1b26)) * **core:** policy support for LIST of kas grants (protos/db) ([#1317](#1317)) ([599fc56](599fc56)) * **core:** Simplifies support for kidless clients ([#1272](#1272)) ([dedeb32](dedeb32)) * **policy:** 1256 resource mapping groups db support ([#1270](#1270)) ([c020e9b](c020e9b)) * **policy:** 1277 add Resource Mapping Group to objects proto ([#1309](#1309)) ([514f1b8](514f1b8)), closes [#1277](#1277) ### Bug Fixes * **core:** Autobump service ([#1322](#1322)) ([9460fb5](9460fb5)) * **core:** casbin policy should support assign/remove/deactivate rpc naming ([#1298](#1298)) ([288921b](288921b)), closes [#1303](#1303) * **core:** put back proto breaking change detection in CI ([#1292](#1292)) ([9921962](9921962)), closes [#1293](#1293) * **core:** Update casbin policy for rewrap with unknown role ([#1305](#1305)) ([de5be3c](de5be3c)) * **policy:** deprecates and reserves value members from value object in protos ([#1151](#1151)) ([07fcc9e](07fcc9e)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>
Context:
While developing COP, we found that GetEntitlements could take around 5.5 seconds to return a response for the federal dataset. This forced us to increase the server timeout, increase the grpc message size, and implement caches.
After investigating the latency, we determined the two main causes: excess logs (33%) and excess database queries (66%).
Proposed Solution:
Primary
prepareValues
values function, so the new approach is strictly better (especially due to our ubiquitous use of maps).Rego Query Optimization:
Yet if we simply match subject mappings and attribute values, the rego query becomes massive (65 mb). It takes 3 seconds to build (20%) and evaluate (80%). To optimize for not only time but also space, we remove unrelated values for each fqn/attribute pair in the rego input (unless the attribute rule is hierarchical).
After all the optimizations, fetching entitlements using the federal dataset now takes about 125 ms. This is a latency reduction of 98%.
resolves: #1259