Add design proposal and implementation for ZedTokens table #257
Conversation
While we have some guarantees that we try to make around consistency in permissions-api, particularly around ZedTokens, these were not explicitly spelled out anywhere. This commit adds a design proposal for using CRDB itself to store ZedTokens for resources, rather than relying on streaming from SpiceDB or lookups in NATS KV. The motivations for the CRDB-based approach are that it grants us the consistency guarantees we want while also allowing for more flexibility than NATS provides around the location of a given leader for coordinating reads, while also removing a dependency from the permissions-api critical path. Signed-off-by: John Schaeffer <[email protected]>
This commit just tidies up the module file. Signed-off-by: John Schaeffer <[email protected]>
This commit removes the ZedToken cache from the query engine since we're not using it anymore. Signed-off-by: John Schaeffer <[email protected]>
jnschaeffer
force-pushed
the
zedtokens-table
branch
from
b1b7c7b
to
bbf69c5
Compare
This commit simplifies DeleteRelationships to use SpiceDB's WriteRelationships call under the hood and also updates updateRelationshipZedTokens to do work in an out of band transaction. Signed-off-by: John Schaeffer <[email protected]>
This commit updates ZedToken reads so that permissions-api doesn't read expired ZedTokens. Signed-off-by: John Schaeffer <[email protected]>
|
|
||
| As designed, this only provides immediately visible updates for a given resource; other indirectly affected resources are not updated (but could be in the future). Additionally, given CRDB leaseholder behavior, multi-region deployments may see longer durations when performing permissions checks if the ZedToken table is not a global table. | ||
|
|
||
| ## Alternatives considered |
There was a problem hiding this comment.
I'm still trying to understand this full stack, so excuse my ignorance. It seems like another alternative is no ZedTokens at all. Reading this post seems to indicate that this won't happen in CRDB. Also it seems even less likely because of our prefixing.
Am I missing something in my reading of that post and our usage of SpiceDB?
There was a problem hiding this comment.
So that blog post is primarily about controlling write ordering in CRDB, not ACL updates. There are a few strategies we can adopt if the current default of using a single key for ordering transactions becomes a problem, but we haven't run into scaling issues there.
Where things get more interesting is around ZedTokens. The use of ZedTokens isn't really specific to CRDB, but rather inherent to how Zanzibar and SpiceDB operate. SpiceDB revisions (known as snapshots in the Zanzibar paper) are generally some number of seconds long and reads happen within a given revision. All datastores, including CRDB, use revisions for reading data.
ZedTokens allow for the client to request data newer than whatever a given SpiceDB replica might have available in its cache or whatever the current revision it's using is. In a CRDB deployment environment that uses follower reads, this would mean requesting data that might be newer than what the nearest replica has, thus incurring a one-time round trip to the leaseholder coordinating work. Following this, the data are cached in memory in SpiceDB for the remainder of the revision duration. Other storage engines don't have the same horizontal/multi-region scaling capability that CRDB offers.
Not using ZedTokens would require either waiting for a new revision to elapse before ACL updates are visible or leveraging full consistency in permissions checks, which means we get none of the caching benefits SpiceDB provides.
While we have some guarantees that we try to make around consistency in permissions-api, particularly around ZedTokens, these were not explicitly spelled out anywhere. This PR adds a design proposal for using CRDB itself to store ZedTokens for resources, rather than relying on streaming from SpiceDB or lookups in NATS KV. It also adds the implementation, since the work involved is pretty minimal and demonstrating that this approach is workable is good, IMO.
The motivations for the CRDB-based approach are that it grants us the consistency guarantees spelled out in the proposal while also allowing for more flexibility than NATS provides around the location of a given leader for coordinating reads. Using CRDB also removes a dependency from the permissions-api critical path.