I noticed our key_value_expirable table growing and growing. The vast majority of entries were honeypot_time_restriction with an "expire" of "2147483647" the column default.

In honeypot.module, shouldn't the calls to
\Drupal::service('keyvalue.expirable')->get('honeypot_time_restriction')->set($identifier, REQUEST_TIME, 3600*24)
be
\Drupal::service('keyvalue.expirable')->get('honeypot_time_restriction')->setWithExpire($identifier, REQUEST_TIME, 3600*24)
??

CommentFileSizeAuthor
#13 2931611-12-honeypot-keyvalueexpire.patch1.4 KBgeerlingguy
#2 2931611-honeypot-keyvalueexpire.patch1.34 KBkrlucas

Comments

krlucas created an issue. See original summary.

krlucas’s picture

krlucas commented
Title: Honeypot key value entries never expire » Honeypot key value entries never expire (effectively)
StatusFileSize
new 2931611-honeypot-keyvalueexpire.patch1.34 KB

Here's a starting patch
...without tests nor any update hook to clean up the existing entries.

j.b’s picture

j.b commented

Hello, this issue causes our database to keep growing in size considerably.

geerlingguy’s picture

geerlingguy commented

Link to the docs for setWithExpire. This looks like a bit of an oversight when I converted the code from D7!

geerlingguy’s picture

geerlingguy commented
Status: Active » Needs review
geerlingguy’s picture

geerlingguy commented

Just noting for myself that the default value for honeypot_expire in D7 was 300 seconds. The patch in #2 sets the expiry for one day... any thoughts on what the best sane default would be? 5 minutes is probably pretty short and many (most) sites don't even run cron every 5 minutes, so the keys wouldn't be expired then anyways...

darkstartom’s picture

darkstartom commented

I'm having the same problem. Is it safe to manually remove the honeypot_time_restriction rows from the key_value_expire table?

nitnac23’s picture

nitnac23 commented

I have been emptying the key_value_expire table manually once a month to get rid of this issue. @DarkstarTom it is fine to empty the table it stores private and shared temp store. For example, you may lose unsaved changes to a view or form.

geerlingguy’s picture

geerlingguy commented
Status: Needs review » Needs work

Ideally I'd like a test added. Additionally, an update hook that clears out any existing entries, maybe?

nick_vh’s picture

nick_vh
he/him
Location Ghent
commented

We faced this issue with more than 3 million rows. Clearing the rows caused the database to completely fail and needed a reboot. I agree a update hook is required before this can go in. But that said, any new installs using this module are exposed to this bug. Therefor I suggest you get this in the dev version and work on 2 new issues, 1 is a test + 1 is a update hook.

j.b’s picture

j.b commented

I had 12 million of entries related to honeypot_time_restriction in my database and my website has been online for 1.5 years.
I had to put in place a batch process which gets executed at night for a total duration of 5 hours to clean up my database gradually over a period of 1 week.

geerlingguy’s picture

geerlingguy commented
Status: Needs work » Reviewed & tested by the community

Makes sense; let's stop the bleeding, and then I'll open a follow-up ticket to add the update hook.

geerlingguy’s picture

geerlingguy commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new 2931611-12-honeypot-keyvalueexpire.patch1.4 KB

Had to update the patch.

  • geerlingguy committed 7be9671 on 8.x-1.x 
    Issue #2931611 by geerlingguy, krlucas: Honeypot key value entries never...
geerlingguy’s picture

geerlingguy commented
Status: Needs review » Fixed
geerlingguy’s picture

geerlingguy commented

Created follow-up: #2997609: Provide ways to clean up old honeypot_time_restriction values.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.