Skip to content

Conversation

@navneet0693
Copy link
Contributor

@navneet0693 navneet0693 commented Sep 21, 2020

Problem

I started working on the issue https://www.drupal.org/project/social/issues/3171563 and I created a solution here: #1986. But even this solution was leading to memory outage no matter if the batch size is 10 or 10000. So, I decided to take a different approach to solve the issue.

Solution

  1. Use database queries instead of Entity APIs to solve the problem reported here: https://www.drupal.org/project/social/issues/3162557
  2. Split the updates into two updates.
  3. First update hook should delete non-existent Activity IDs from activitiy_notification_status table.
  4. Second update hook should delete the Activity entities which doesn't have any value in "field_activity_entity".

Issue tracker

https://www.drupal.org/project/social/issues/3171563

How to test

  • Using version <8.5 of Open Social.
  • Upgrade to 8.5.
  • Check for no memory outage.

Screenshots

N.A

Release notes

In a past release we made a update hook to delete old activities that had no more related entities. This update hook was not working properly for bigger websites, so we changed the logic to make it less performance heavy.

Change Record

N.A

Translations

N.A

@navneet0693 navneet0693 added needs work: maintainer input This pull request needs input from a tech lead prio: high backport: needed This pull request requires a backport to an additionally supported minor version status: needs review This pull request is waiting for a requested review team: bananas type: bug Fixes a bug in Open Social type: refactoring Updates code for improved maintenance without changing its functionality type: performance Improve Open Social performance labels Sep 21, 2020
@navneet0693 navneet0693 added this to the 8.6 milestone Sep 21, 2020
@navneet0693 navneet0693 force-pushed the issue/3171563-memory-outage-activity-creator-2 branch from 3ba0612 to 15577b5 Compare September 21, 2020 11:08
@robertragas
Copy link
Collaborator

@navneet0693
1 question about the change, not sure if i'm misreading it, but in the old situation when the activity does not exist, it would delete the activity_notification_status, like you have in your first update hook

In the second part it would delete the whole activity is the related_entity does not exist, in the old one it would again also delete the activity_notification_status, but in yours it only deletes the activity. Is this not needed anymore?

…y_notification_status table if activity is deleted during batch.
@navneet0693
Copy link
Contributor Author

@navneet0693
1 question about the change, not sure if i'm misreading it, but in the old situation when the activity does not exist, it would delete the activity_notification_status, like you have in your first update hook

In the second part it would delete the whole activity is the related_entity does not exist, in the old one it would again also delete the activity_notification_status, but in yours it only deletes the activity. Is this not needed anymore?

Thanks for pointing out Robert. I have fixed it last commit :-)

@robertragas
Copy link
Collaborator

You made the last change and it goes a lot faster. Still some timeouts on Pantheon, but that's due to their plan being to small.
Will merge it.

@robertragas robertragas removed needs work: maintainer input This pull request needs input from a tech lead status: needs review This pull request is waiting for a requested review labels Sep 30, 2020
@robertragas robertragas merged commit aeb6776 into 10.0.x Sep 30, 2020
robertragas added a commit that referenced this pull request Sep 30, 2020
…-activity-creator-2

Issue #3171563 by navneet0693: Moved logic from activity_creator_update_8802 to hook_post_update_name and refactored.
robertragas added a commit that referenced this pull request Sep 30, 2020
…-activity-creator-2

Issue #3171563 by navneet0693: Moved logic from activity_creator_update_8802 to hook_post_update_name and refactored.
@robertragas robertragas added backport: verified This pull request has been back ported to an older minor version and removed backport: needed This pull request requires a backport to an additionally supported minor version labels Sep 30, 2020
@Kingdutch Kingdutch deleted the issue/3171563-memory-outage-activity-creator-2 branch September 30, 2021 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport: verified This pull request has been back ported to an older minor version prio: high type: bug Fixes a bug in Open Social type: performance Improve Open Social performance type: refactoring Updates code for improved maintenance without changing its functionality

Development

Successfully merging this pull request may close these issues.

3 participants