node_access() builds queries based on hook_node_access_grants() and hook_node_access_records().

These queries can get huge if there are large numbers of gids and realms. EclipseCG was discussing such a problem on an actual site today in IRC, in fact.

We need to document this limitation of the grant/realm system, and suggest that if your system would have a lot of realms/grants, not to use hook_node_access_grants() and hook_node_access_records() but instead to use hook_node_access() and build your own record-keeping and queries, which would probably also involve a hook_node_update() and other hooks to trigger saving the node access info when the node is created/updated.

We should put a note on the the grants/records hooks, and more detailed information about this suggestion on http://api.drupal.org/api/drupal/modules--node--node.module/group/node_a...

CommentFileSizeAuthor
#1 1027212.patch5.41 KBjhodgdon

Comments

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new 1027212.patch5.41 KB

Here's a patch. It updates the doc for hook_node_access_grants() and the Node Access topic.

moshe weitzman’s picture

moshe weitzman commented

IMO, this is too much information for source code comments. This belongs in docs external to drupal. All systems can be abused. Should we put warnings on the role system not to add a role per user? Thats basically what we are warning against here. For me, this is Won't Fix.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Do we have information about this in the Handbook that we can refer to then? If so, we should link to it.

moshe weitzman’s picture

moshe weitzman commented

I am not aware of pages in the handbook that caution against creating thousands of roles nor thousands of gids.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Version: 7.x-dev » 8.x-dev
Issue tags: +Needs backport to D7

Looking at old issues...

Can we get a second opinion on whether this is "too much information" for the core docs?

As a note, we don't even have a section in http://drupal.org/developing/api currently about the node access API, so I'm not sure where we could put this in the handbook (not about to write a section just so we can put in this information). I guess it could go into the Examples project, but I still think it belongs here, since this is the only reference we have to the node access API.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Issue tags: -Needs backport to D7

#1: 1027212.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Needs backport to D7

The last submitted patch, 1027212.patch, failed testing.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Apparently, besides a second opinion on the review, this needs a reroll.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Issue summary: View changes
Status: Needs work » Closed (won't fix)

Well I think at this point we can "won't fix" this one.

ragnarkurm’s picture

ragnarkurm commented

As Google search is leading here, more people might be looking for help regarding "too many realms/grants", I added the info here. Encountered the issue with the permissions_by_term module, but the problem is more generic. Created also a core patch to alleviate the problem. The core patch is in the module issue queue since I understand that core maintainers are not willing to optimize NodeGrantDatabaseStorage::access(), and pointing toward using more specific hooks.

https://www.drupal.org/project/permissions_by_term/issues/3380033

The patch in the issue takes care of monster select queries to the node_access table per node, but not for views. It is still a noticeable improvement in terms of speed.