The problem

When a user with many comments cancels his account, this could lead to php memory issues.

What are the steps required to reproduce the bug?

Cancel the user account of a User with thousands of comments with a php memory limit of 256M.

What behavior were you expecting?

The account will be deleted and all comments are unpublished/updated as well.

What happened instead?

You get a php memory error and not all comments of the canceled user are deleted/updated.

Reason

The code were the comments are loaded and updated has no chunking or batching mechanic.

$comments = entity_load_multiple_by_properties('comment', ['uid' => $account->id()]);
      foreach ($comments as $comment) {
        $comment->setUnpublished();
        $comment->save();
      }
CommentFileSizeAuthor
#21 interdiff_20_21.txt832 bytesameymudras
#21 3007581-21.patch8.74 KBameymudras
#20 interdiff_10_20.txt1.05 KBameymudras
#20 3007581-20.patch8.68 KBameymudras
#19 3007581-nr-bot.txt1.79 KBneeds-review-queue-bot
#18 3007581-nr-bot.txt1.79 KBneeds-review-queue-bot
#14 3007581-13.patch8.65 KBnikhil_110
#11 3007581-10.patch8.62 KBameymudras
#4 3007581-fix-memory-issues-in-comment-user-cancel-hook.patch8.48 KBdtv_rb

Issue fork drupal-3007581

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

dtv_rb created an issue. See original summary.

dtv_rb’s picture

dtv_rb commented
Issue summary: View changes
cilefen’s picture

cilefen commented
Issue tags: +Performance
dtv_rb’s picture

dtv_rb commented
StatusFileSize
new 3007581-fix-memory-issues-in-comment-user-cancel-hook.patch8.48 KB

Here is a first patch that implements a similar solution like the node module does.

Version: 8.6.1 » 8.6.x-dev

Core issues are now filed against the dev versions where changes will be made. Document the specific release you are using in your issue comment. More information about choosing a version.

Version: 8.6.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Branches prior to 8.8.x are not supported, and Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev
catch’s picture

catch
he/him
Primary language English
commented
Title: Memory issues with comment_user_cancel » comment_user_cancel() doesn't implement a batch
Component: entity system » comment.module
Status: Active » Needs work
Issue tags: +Bug Smash Initiative

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

ameymudras’s picture

ameymudras commented
Version: 9.4.x-dev » 9.5.x-dev
Status: Needs work » Needs review
StatusFileSize
new 3007581-10.patch8.62 KB

Re rolling the above patch to 9.5.x

Status: Needs review » Needs work

The last submitted patch, 11: 3007581-10.patch, failed testing. View results

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

nikhil_110’s picture

nikhil_110 commented
StatusFileSize
new 3007581-13.patch8.65 KB

Re-rolling patch to 9.5.x

nikhil_110’s picture

nikhil_110 commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented

#11 still applied to 9.5 cleanly so #14 reroll was unnecessary also no interdiff was included.

The last submitted patch, 11: 3007581-10.patch, failed testing. View results

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3007581-nr-bot.txt1.79 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
StatusFileSize
new 3007581-nr-bot.txt1.79 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

ameymudras’s picture

ameymudras commented
Status: Needs work » Needs review
StatusFileSize
new 3007581-20.patch8.68 KB
new interdiff_10_20.txt1.05 KB

Removing deprecated code from patch #10 and making it compatible with 10.1.x

ameymudras’s picture

ameymudras commented
StatusFileSize
new 3007581-21.patch8.74 KB
new interdiff_20_21.txt832 bytes

Status: Needs review » Needs work

The last submitted patch, 21: 3007581-21.patch, failed testing. View results

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

samit.310@gmail.com made their first commit to this issue’s fork.

Samit Khulve opened merge request !9444

samitk’s picture

samitk commented
Status: Needs work » Needs review

Hello,

Batch process for comment_user_cancel has been added.

Thanks
Samit K.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs issue summary update, +Needs tests

Seems to the patch from #21 was anything different done?

samitk’s picture

samitk commented
Status: Needs work » Needs review

Hi @smustgrave ,

It has a small difference that i introduced the BatchBuilder class to create batch, also upgrade the code as per Drupal 11 standards like by replacing module_load_include function.

Thanks
Samit K.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Tags are still relevant

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.