[wp-trac] [WordPress Trac] #36901: Removing wp_die() from wp_allow_comment()

[WordPress Trac]

#36901: Removing wp_die() from wp_allow_comment()
-------------------------------------------------+-------------------------
 Reporter:  websupporter                         |       Owner:
     Type:  enhancement                          |  rachelbaker
 Priority:  normal                               |      Status:  assigned
Component:  Comments                             |   Milestone:  4.7
 Severity:  normal                               |     Version:  4.6
 Keywords:  has-patch needs-testing has-unit-    |  Resolution:
  tests                                          |     Focuses:
-------------------------------------------------+-------------------------

Comment (by websupporter):

 Finally I found the reason why check_comment_flood_db() is not directly
 called:

 >  Make the entire comment flood check pluggable as it can cause load
 problems on large sites.
 https://core.trac.wordpress.org/changeset/5947

 Unfortunatly there is no ticket associated and I couldn't find one
 searching these tickets:
 https://core.trac.wordpress.org/query?version=2.3&resolution=fixed

 So the basic idea seems to be, nine years ago large sites did run into
 problems.

 '''Why did they run into problems?'''
 I do not understand why, is it the database request back then?

 `SELECT comment_date_gmt FROM $wpdb->comments WHERE comment_author_IP =
 '$ip' OR comment_author_email = '$email' ORDER BY comment_date DESC LIMIT
 1`

 Might it already been resolved now?
 `SELECT comment_date_gmt FROM $wpdb->comments WHERE comment_date_gmt >= %s
 AND ( $check_column = %s OR comment_author_email = %s ) ORDER BY
 comment_date_gmt DESC LIMIT 1`

 As far as I understand currently the db columns `comment_author_email` and
 `comment_date_gmt` (besides others) are indexed. Back than it seems like
 only `comment_ID`, `comment_approved` and `comment_post_ID` where indexed
 (see https://core.trac.wordpress.org/browser/branches/2.3/wp-
 admin/includes/schema.php#L53). Maybe this meant the queries took long to
 execute and its ok now?

 '''Why as action hook?'''
 The basic idea of the action seems to be, so large sites can dehook the
 check (and hook into with their own checks).

 Any thoughts on this?

--
Ticket URL: <https://core.trac.wordpress.org/ticket/36901#comment:15>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform