[drupal-devel] [task] Pager Queries, Node Access and Performance
JonBob
drupal-devel at drupal.org
Fri Feb 11 20:00:50 UTC 2005
Project: Drupal
Version: cvs
Component: node.module
Category: tasks
Priority: normal
Assigned to: Anonymous
Reported by: pyromanfo
Updated by: JonBob
-Status: active
+Status: patch
Attachment: http://drupal.org/files/issues/node_access.patch (2.91 KB)
Here's a patch that splits the nid=0 check off into a separate query. If
the user has such a grant, things should be really fast since we don't
need any join or even DISTINCT() check. If not, the join is done as
before, but without the "OR nid=0" piece.
This ideally needs performance checking on systems using node access
with nid=0 rows, using it without them, and not using it at all.
JonBob
Previous comments:
------------------------------------------------------------------------
February 1, 2005 - 10:54 : pyromanfo
I've got a Drupal 4.5.2 site running with Taxonomy Access Control. When
you go to view a taxonomy and it tries to do an access check, the SQL
query takes literally minutes to complete. This is on a dual Xeon box,
it's rather beefy and runs two other sites (vBulletin and a hacked up
Postnuke site) just fine. The problem comes from the
taxonomy_select_nodes function
$sql_count = 'SELECT COUNT(DISTINCT(n.nid)) FROM {node} n '.
node_access_join_sql() .' INNER JOIN {term_node} tn ON n.nid = tn.nid
WHERE tn.tid IN ('. $str_tids .') AND n.status = 1 AND '.
node_access_where_sql();
Which ends up looking like
SELECT COUNT(DISTINCT(n.nid)) FROM node n INNER JOIN node_access na ON
(na.nid = 0 OR na.nid = n.nid) INNER JOIN term_node tn0 ON n.nid =
tn0.nid WHERE n.status = 1 AND na.grant_view = 1 AND CONCAT(na.realm,
na.gid) IN ('all0','term_access1') AND tn0.tid IN (9)
This will take several minutes to complete. The query next to it that
selected the nodes themselves took 3 minutes to complete.
Where is the problem exactly? I've seen elsewhere that this is a
problem for node_privacy_byrole as well, which makes sense because
really all the various access control modules are contributing here is
an extra entry in the concat search.
Let me know if you need any details of how the MySQL server is
configured. Here's some data from my drupal database
mysql> select count(nid) from node;
+------------+
| count(nid) |
+------------+
| 10376 |
+------------+
1 row in set (0.00 sec)
mysql> select count(nid) from node_access;
+------------+
| count(nid) |
+------------+
| 41508 |
+------------+
1 row in set (0.00 sec)
Drupal performs very snappily with the node_access stuff disabled, and
otherwise it performs very well. A three minute increase in the
execute time of an SQL query is just strange, there's something odd
going on here I think.
------------------------------------------------------------------------
February 3, 2005 - 16:02 : pyromanfo
Reducing the size of the 'realm' field of node_access to something
manageable like 12 instead of the current 255 speeds up the query
tremendously, still it's rather slow.
I also added a search index on the grant_view permission, since the
view permission is the only permission you'd want to check for more
than one node usually. It did speed it up, however it still take a
minute or so on my little test box. I'm going to tweak with it some
more and see if I can get it performing better.
------------------------------------------------------------------------
February 3, 2005 - 18:12 : killes at www.drop.org
Could you give us an estimate with numbers? Or even better, make
available the database for testing?
------------------------------------------------------------------------
February 4, 2005 - 14:59 : pyromanfo
I can't really make the database available without going through and
setting all the text and titles to something random and the uid's to 2,
I may try that.
I can give you some more numbers tonight though when I start messing
with it again. You want execution times and table sizes? Any other
specific numbers? Should I just dump show status?
------------------------------------------------------------------------
February 4, 2005 - 15:21 : killes at www.drop.org
Averages of execution times before and after would be fine, I think.
The db would be great of course You could simple set all text to "foo"
and email addresses to "root at localhost".
------------------------------------------------------------------------
February 5, 2005 - 12:31 : pyromanfo
I'm running these tests on a similar database I haven't been developing
on. Created from the same source data with the same script though, I
just haven't been hacking it up and it runs on faster hardware.
SELECT COUNT(DISTINCT(n.nid)) FROM node n INNER JOIN node_access na ON
(na.nid = 0 OR na.nid = n.nid) INNER JOIN term_node tn0 ON n.nid =
tn0.nid WHERE n.status = 1 AND na.grant_view = 1 AND CONCAT(na.realm,
na.gid) IN ('all0','term_access1') AND tn0.tid IN (2);
+------------------------+
| COUNT(DISTINCT(n.nid)) |
+------------------------+
| 2993 |
+------------------------+
1 row in set (7 min 4.00 sec)
Then I ran the following queries
alter table node_access modify realm varchar(12) NOT NULL default '';
alter table node_access add index view_indx (nid,gid,realm,grant_view);
Then I ran
mysqlcheck --auto_repair -os
Then the query again
SELECT COUNT(DISTINCT(n.nid)) FROM node n INNER JOIN node_access na ON
(na.nid = 0 OR na.nid = n.nid) INNER JOIN term_node tn0 ON n.nid =
tn0.nid WHERE n.status = 1 AND na.grant_view = 1 AND CONCAT(na.realm,
na.gid) IN ('all0','term_access1') AND tn0.tid IN (2);
+------------------------+
| COUNT(DISTINCT(n.nid)) |
+------------------------+
| 2993 |
+------------------------+
1 row in set (6 min 18.62 sec)
Not as big a speed boost as I thought I was seeing unfortunately.
Here's 'show variables'
mysql> show variables;
+---------------------------------+---------------------------------------------------+
| Variable_name | Value
|
+---------------------------------+---------------------------------------------------+
| back_log | 50
|
| basedir | /usr/local/mysql/
|
| binlog_cache_size | 32768
|
| bulk_insert_buffer_size | 8388608
|
| character_set_client | latin1
|
| character_set_connection | latin1
|
| character_set_database | latin1
|
| character_set_results | latin1
|
| character_set_server | latin1
|
| character_set_system | utf8
|
| character_sets_dir |
/usr/local/mysql/share/mysql/charsets/ |
| collation_connection | latin1_swedish_ci
|
| collation_database | latin1_swedish_ci
|
| collation_server | latin1_swedish_ci
|
| concurrent_insert | ON
|
| connect_timeout | 5
|
| datadir | /usr/local/mysql/var/
|
| date_format | %Y-%m-%d
|
| datetime_format | %Y-%m-%d %H:%i:%s
|
| default_week_format | 0
|
| delay_key_write | ON
|
| delayed_insert_limit | 100
|
| delayed_insert_timeout | 300
|
| delayed_queue_size | 1000
|
| expire_logs_days | 0
|
| flush | OFF
|
| flush_time | 0
|
| ft_boolean_syntax | + -><()~*:""&|
|
| ft_max_word_len | 84
|
| ft_min_word_len | 4
|
| ft_query_expansion_limit | 20
|
| ft_stopword_file | (built-in)
|
| group_concat_max_len | 1024
|
| have_archive | NO
|
| have_bdb | NO
|
| have_compress | YES
|
| have_crypt | YES
|
| have_csv | NO
|
| have_example_engine | NO
|
| have_geometry | YES
|
| have_innodb | YES
|
| have_isam | NO
|
| have_ndbcluster | NO
|
| have_openssl | NO
|
| have_query_cache | YES
|
| have_raid | NO
|
| have_rtree_keys | YES
|
| have_symlink | YES
|
| init_connect |
|
| init_file |
|
| init_slave |
|
| innodb_additional_mem_pool_size | 1048576
|
| innodb_autoextend_increment | 8
|
| innodb_buffer_pool_awe_mem_mb | 0
|
| innodb_buffer_pool_size | 8388608
|
| innodb_data_file_path | ibdata1:10M:autoextend
|
| innodb_data_home_dir |
|
| innodb_fast_shutdown | ON
|
| innodb_file_io_threads | 4
|
| innodb_file_per_table | OFF
|
| innodb_locks_unsafe_for_binlog | OFF
|
| innodb_flush_log_at_trx_commit | 1
|
| innodb_flush_method |
|
| innodb_force_recovery | 0
|
| innodb_lock_wait_timeout | 50
|
| innodb_log_arch_dir |
|
| innodb_log_archive | OFF
|
| innodb_log_buffer_size | 1048576
|
| innodb_log_file_size | 5242880
|
| innodb_log_files_in_group | 2
|
| innodb_log_group_home_dir | ./
|
| innodb_max_dirty_pages_pct | 90
|
| innodb_table_locks | ON
|
| innodb_max_purge_lag | 0
|
| innodb_mirrored_log_groups | 1
|
| innodb_open_files | 300
|
| innodb_thread_concurrency | 8
|
| interactive_timeout | 28800
|
| join_buffer_size | 131072
|
| key_buffer_size | 8388600
|
| key_cache_age_threshold | 300
|
| key_cache_block_size | 1024
|
| key_cache_division_limit | 100
|
| language |
/usr/local/mysql/share/mysql/english/ |
| large_files_support | ON
|
| license | GPL
|
| local_infile | ON
|
| locked_in_memory | OFF
|
| log | OFF
|
| log_bin | OFF
|
| log_error |
|
| log_slave_updates | OFF
|
| log_slow_queries | OFF
|
| log_update | OFF
|
| log_warnings | 1
|
| long_query_time | 10
|
| low_priority_updates | OFF
|
| lower_case_file_system | OFF
|
| lower_case_table_names | 0
|
| max_allowed_packet | 1048576
|
| max_binlog_cache_size | 4294967295
|
| max_binlog_size | 1073741824
|
| max_connect_errors | 10
|
| max_connections | 100
|
| max_delayed_threads | 20
|
| max_error_count | 64
|
| max_heap_table_size | 16777216
|
| max_insert_delayed_threads | 20
|
| max_join_size | 4294967295
|
| max_length_for_sort_data | 1024
|
| max_relay_log_size | 0
|
| max_seeks_for_key | 4294967295
|
| max_sort_length | 1024
|
| max_tmp_tables | 32
|
| max_user_connections | 0
|
| max_write_lock_count | 4294967295
|
| myisam_data_pointer_size | 4
|
| myisam_max_extra_sort_file_size | 2147483648
|
| myisam_max_sort_file_size | 2147483647
|
| myisam_recover_options | OFF
|
| myisam_repair_threads | 1
|
| myisam_sort_buffer_size | 8388608
|
| net_buffer_length | 16384
|
| net_read_timeout | 30
|
| net_retry_count | 10
|
| net_write_timeout | 60
|
| new | OFF
|
| old_passwords | ON
|
| open_files_limit | 1024
|
| pid_file | ********** |
| port | 3306
|
| preload_buffer_size | 32768
|
| protocol_version | 10
|
| query_alloc_block_size | 8192
|
| query_cache_limit | 1048576
|
| query_cache_min_res_unit | 4096
|
| query_cache_size | 0
|
| query_cache_type | ON
|
| query_cache_wlock_invalidate | OFF
|
| query_prealloc_size | 8192
|
| range_alloc_block_size | 2048
|
| read_buffer_size | 131072
|
| read_only | OFF
|
| read_rnd_buffer_size | 262144
|
| relay_log_purge | ON
|
| rpl_recovery_rank | 0
|
| secure_auth | OFF
|
| server_id | 0
|
| skip_external_locking | ON
|
| skip_networking | OFF
|
| skip_show_database | OFF
|
| slave_net_timeout | 3600
|
| slow_launch_time | 2
|
| socket | /tmp/mysql.sock
|
| sort_buffer_size | 2097144
|
| sql_mode |
|
| storage_engine | MyISAM
|
| sync_binlog | 0
|
| sync_frm | ON
|
| system_time_zone | EST
|
| table_cache | 64
|
| table_type | MyISAM
|
| thread_cache_size | 0
|
| thread_stack | 196608
|
| time_format | %H:%i:%s
|
| time_zone | SYSTEM
|
| tmp_table_size | 33554432
|
| tmpdir |
|
| transaction_alloc_block_size | 8192
|
| transaction_prealloc_size | 4096
|
| tx_isolation | REPEATABLE-READ
|
| version | 4.1.7
|
| version_comment | Source distribution
|
| version_compile_machine | i686
|
| version_compile_os | pc-linux
|
| wait_timeout | 28800
|
+---------------------------------+---------------------------------------------------+
176 rows in set (0.03 sec)
I'm attaching my test database which takes several minutes to execute a
similar query
If you want me to run the tests on the test database I can do that,
it's just on a much skimpier machine and it takes quite a bit longer
(Athlon 600, 256MB Ram vs Dual Xeon).
------------------------------------------------------------------------
February 5, 2005 - 12:33 : pyromanfo
Oops, preview removed the test db, here it is.
------------------------------------------------------------------------
February 5, 2005 - 12:38 : pyromanfo
It's too big, it's 4.47 MB, I'm gonna put it up on GWJ and link to it.
Get it here [1]
[1] http://www.gamerswithjobs.com/testdb/drupal_test_db.tar.bz2
------------------------------------------------------------------------
February 5, 2005 - 12:46 : pyromanfo
Attachment: http://drupal.org/files/issues/drupal_test_db.tar.bz2 (734.78 KB)
Nevermind, I left in some sensitive info and forgot to wipe the search
index, now it's much smaller.
------------------------------------------------------------------------
February 7, 2005 - 17:58 : killes at www.drop.org
Thanks, but I think you stripped the DB a bit too much. If I execute the
query it is very fast because it does not return anything...
------------------------------------------------------------------------
February 7, 2005 - 18:12 : killes at www.drop.org
When I use the query from your original post, I get some result. I guess
I will write a script and run some tests over night...
------------------------------------------------------------------------
February 7, 2005 - 18:55 : killes at www.drop.org
Ok, here are some preliminary results:
- The unchanged query from the original post runs nearly 14 minutes on
the hardware redLED lent to us.
- If I shorten the varchar field to 18 from 255, it gets about halfed.
- neither adding an index for the realm nor the grant_view field does
make a change
- Omitting the "na.nid = 0 OR" part makes the query very fast (quarter
of a second)
- SELECT COUNT(DISTINCT(n.nid)) FROM node n INNER JOIN node_access na
ON ( na.nid = n.nid) left jo
in node_access nb ON nb.nid = 0 INNER JOIN term_node tn0 ON n.nid =
tn0.nid WHERE n.status = 1 AND ((n
a.grant_view = 1 AND CONCAT(na.realm, na.gid) IN
('all0','term_access1')) OR (nb.grant_view = 1 AND CON
CAT(nb.realm, nb.gid) IN ('all0','term_access1'))) AND tn0.tid IN (9);
This query is still very much faster (about half a second) than the
original query and gives the same result for your database. I don't
know if it is fully equivalent, though. Note that it is a left join.
------------------------------------------------------------------------
February 8, 2005 - 08:06 : pyromanfo
The na.nid = 0 stuff is basically a catchall, that's sort of a default
permission that overrides anything else.
This is good when you're not using node_access, however when you are
the na.nid=0 stuff probably isn't there, I know Node Privacy By Role
and Taxonomy Access Control both delete the entries for nid=0.
Perhaps it's a good idea to control this another way, by setting a
variable and using that to determine whether or not some sort of
node_access is installed, as it seems using nid = 0 as a default simply
kills performance.
------------------------------------------------------------------------
February 8, 2005 - 08:08 : pyromanfo
Oh, and thanks for taking a good look at it killes, I should've
explained better. The DB I provided, the test DB, has a large number
of nodes in tid=9, the other DB I ran the tests on has alot in tid=2.
------------------------------------------------------------------------
February 8, 2005 - 11:17 : Anonymous
The more I think about replacing nid=0 with a configuration setting the
more it makes sense. The two major node access modules right now have
to have a seperate "enable/disable" setting to deal with nid=0, since
it overrides everything. In fact, it seems like it's only purpose it
to override all other node access settings with a universal default.
Why not instead have a global configuration setting under permissions
that enables or disable node level permissions, then the various node
access modules wouldn't need a seperate disable/enable setting.
Furthermore, this performance problem would disappear, because either
the node_access stuff would be there and you aren't looking for nid=0,
or it isn't and you don't care what's in node_access. It seems it'd
save queries to node access when you're not using node level
permissions, and make it cleaner to implement a node access modifying
module.
------------------------------------------------------------------------
February 8, 2005 - 11:17 : pyromanfo
I hate the site randomly logging you out, that was me above.
------------------------------------------------------------------------
February 8, 2005 - 14:23 : JonBob
"The more I think about replacing nid=0 with a configuration setting the
more it makes sense. The two major node access modules right now have to
have a seperate "enable/disable" setting to deal with nid=0, since it
overrides everything. In fact, it seems like it's only purpose it to
override all other node access settings with a universal default."
This is not the case. I think you're confusing the case of nid=0 with
the case of realm=all.
As an example of the utility of nid=0, consider the scenario given in
the example access module [2]. This module implements a simple system
whereby anyone can view "public" nodes, and only users with a specific
permission can view "private" nodes. This can be rephrased as follows:
anonymous users can only view "public" nodes, while privileged users
can view all nodes. A grant with nid=0 takes care of the second case,
so that the module only needs to deal with granting permission to view
the public nodes as necessary.
Gerhard's query works in the case where every node has at least one
node_access entry with its nid, but I'm afraid it fails in the above
example. A node that is private would not have any corresponding entry,
and so the inner join would fail for it, returning no record. The left
join doesn't even have a chance to be matched.
It would be worth benchmarking the scenario in which the nid=0
possibility is removed from the query, but the node_access table has an
entry added for each node instead of a single nid=0 entry. If this is
faster than the OR in the join, the extra DB storage is probably worth
the change.
[2]
http://drupaldocs.org/api/head/file/contributions/docs/developer/examples/node_access_example.module
------------------------------------------------------------------------
February 8, 2005 - 20:08 : pyromanfo
"As an example of the utility of nid=0, consider the scenario given in
the example access module. This module implements a simple system
whereby anyone can view "public" nodes, and only users with a specific
permission can view "private" nodes. This can be rephrased as follows:
anonymous users can only view "public" nodes, while privileged users
can view all nodes. A grant with nid=0 takes care of the second case,
so that the module only needs to deal with granting permission to view
the public nodes as necessary.
"
I think there's a bug in the module, in the docs in the top shouldn't
the initial SQL statement be adding that permission to gid=1?
Though you're right, I'd forgotten because I hadn't seen any of the
current modules use it.
As for that performance comparison
mysql> SELECT COUNT(DISTINCT(n.nid)) FROM node n INNER JOIN term_node
tn0 ON n.nid = tn0.nid INNER JOIN node_access na ON (na.nid = n.nid)
WHERE n.status = 1 AND na.grant_view = 1 AND CONCAT(na.realm, na.gid)
IN ('all0','term_access1') AND tn0.tid IN (2);
+------------------------+
| COUNT(DISTINCT(n.nid)) |
+------------------------+
| 2993 |
+------------------------+
1 row in set (0.23 sec)
mysql> update node_access set realm='blam' where realm='term_access';
Query OK, 41508 rows affected (5.42 sec)
Rows matched: 41508 Changed: 41508 Warnings: 0
mysql> insert into node_access (nid,gid,realm,grant_view) values
(0,1,'term_access',1);
Query OK, 1 row affected (0.00 sec)
mysql> SELECT COUNT(DISTINCT(n.nid)) FROM node n INNER JOIN term_node
tn0 ON n.nid = tn0.nid INNER JOIN node_access na ON (na.nid = 0 OR
na.nid=n.nid) WHERE n.status = 1 AND na.grant_view = 1 AND
CONCAT(na.realm, na.gid) IN ('all0','term_access1') AND tn0.tid IN (2);
+------------------------+
| COUNT(DISTINCT(n.nid)) |
+------------------------+
| 2993 |
+------------------------+
1 row in set (0.16 sec)
So I don't think it improves performance for the nid=0 case, though it
is suprisingly slow.
------------------------------------------------------------------------
February 8, 2005 - 20:57 : moshe weitzman
perhaps we should split the query into 2 queries in order to avoid the
'na.nid = 0' part of the JOIN. Alternatively, perhaps we could inject
that clause only when needed using the db_rewrite_sql hook. not sure.
------------------------------------------------------------------------
February 9, 2005 - 23:36 : pyromanfo
Splitting it into two queries would make sense I think.
Basically you could have it entirely in node_access_join_sql and
node_access_where_sql. There we can check for the nid=0 case for any
gids and realms. Then if it returns true, we know we don't have to
check anything else, so we return null for the SQL to enter. We could
even store this in a static variable to avoid calling this twice for
join_sql and where_sql
So in the nid=0 case, this should be similarly fast as we're removing a
table from the JOIN in exchange for checking node_access for one nid.
mysql> select count(nid) from node_access where nid=0 and gid=1 and
realm in ('all','term_access','blah');
+------------+
| count(nid) |
+------------+
| 0 |
+------------+
1 row in set (0.37 sec)
SELECT COUNT(DISTINCT(n.nid)) FROM node n INNER JOIN term_node tn0 ON
n.nid = tn0.nid WHERE n.status = 1 AND tn0.tid IN (9);
+------------------------+
| COUNT(DISTINCT(n.nid)) |
+------------------------+
| 4202 |
+------------------------+
1 row in set (0.15 sec)
SELECT COUNT(DISTINCT(n.nid)) FROM node n INNER JOIN node_access na ON
(na.nid = 0 OR na.nid = n.nid) INNER JOIN term_node tn0 ON n.nid =
tn0.nid WHERE n.status = 1 AND na.grant_view = 1 AND CONCAT(na.realm,
na.gid) IN ('all0','blah1') AND tn0.tid IN (9);
+------------------------+
| COUNT(DISTINCT(n.nid)) |
+------------------------+
| 4202 |
+------------------------+
1 row in set (1.97 sec)
It looks to be faster, actually.
Then, when we don't get a result, we check node access with the
'na.nid=0 OR' removed. This is certain to be faster as the test have
shown, taking the query down from 7 minutes to a matter of seconds.
In either case, this method would be faster. Plus, it wouldn't require
any changes to other modules, simply change the node_access_where_sql
and node_access_join_sql functions.
JonBob, would you be okay with that? I can code it if you think you'll
accept the patch.
------------------------------------------------------------------------
February 10, 2005 - 08:38 : Anonymous
Splitting the queries into two is probably the only way to go. However,
I'd like to avoid calling the nid = 0 query if possible. My Drupal
installations that use node access do not use nid = 0 at all. I think
we could store the available (nid = 0) => realm combinations somewhere
and only execute the query if a matching condition is found. Gerhard
------------------------------------------------------------------------
February 10, 2005 - 10:14 : moshe weitzman
that nid=0 is going to be very fast when done all by itself. no need to
introduce any other mechanism IMO. testing will tell us for sure though
------------------------------------------------------------------------
February 10, 2005 - 13:31 : killes at www.drop.org
The nid = 0 query would be executed for every node query on a page call
even if your access module doesn't use nid = 0 at all. Most
implementations do not use nid = 0. If we can avoid a query we should
do so.
--
View: http://drupal.org/node/16558
Edit: http://drupal.org/project/comments/add/16558
More information about the drupal-devel
mailing list