Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#7977] Update AttachmentController#show_as_html to do attachment masking in the background #7979

Merged
merged 2 commits into from
Oct 26, 2023

Conversation

gbp
Copy link
Member

@gbp gbp commented Oct 25, 2023

Relevant issue(s)

Fixes #7977

What does this do?

This updates AttachmentController#show_as_html to do attachment masking in the background.

Why was this needed?

Our "view attachments in the browser" feature, or "view as HTML", is doing a lot of processing inline in the web processes and this sometimes causes memory issues on the server and results in oom-kill running.

Implementation notes

Uses the same logic as the #show action to check an attachment is masked before proceeding with the action. This means it will time outs after 5 seconds and then redirect the AttachmentMaskController to wait for the job to complete before rendering a view saying the attachment is now viewable.

@gbp gbp force-pushed the 7977-view-as-html-masking-in-background branch from 775b0a6 to 0d07b7a Compare October 25, 2023 07:03
@gbp gbp changed the title [#7977] [#7977] Update AttachmentController#show_as_html to do processing in the background Oct 25, 2023
@gbp gbp changed the title [#7977] Update AttachmentController#show_as_html to do processing in the background [#7977] Update Attachment "view as html" to do masking in the background Oct 25, 2023
@gbp gbp changed the title [#7977] Update Attachment "view as html" to do masking in the background [#7977] Update AttachmentController#show_as_html to do attachment masking in the background Oct 25, 2023
@gbp gbp force-pushed the 7977-view-as-html-masking-in-background branch from 42e1f8f to 1113772 Compare October 25, 2023 09:12
@gbp gbp marked this pull request as ready for review October 25, 2023 09:12
@gbp gbp requested a review from garethrees October 25, 2023 10:08
gbp added 2 commits October 25, 2023 11:52
Add new `ensure_masked` callback to the `AttachmentController`.
Currently just used by the `show` action but will be used for
`show_as_html` soon.
This updates `AttachmentController#show_as_html` to do attachment
masking in the background.

Our "view attachments in the browser" feature, or "view as HTML", is
doing a lot of processing inline in the web processes and this sometimes
causes memory issues on the server and results in oom-kill running.

Uses the same logic as the `#show` action to check an attachment is
masked before proceeding with the action. This means it will time outs
after 5 seconds and then redirect the `AttachmentMaskController` to wait
for the job to complete before rendering a view saying the attachment is
now viewable.

Fixes #7977
@gbp gbp force-pushed the 7977-view-as-html-masking-in-background branch from 1113772 to 7e6db46 Compare October 25, 2023 10:53
@gbp
Copy link
Member Author

gbp commented Oct 25, 2023

Testing on staging and this works but I have changed the implementation slightly. Because we're not downloading files we don't need to render the done action at all, and can just redirect the to show as HTML action.

@gbp
Copy link
Member Author

gbp commented Oct 25, 2023

This is up on staging for testing, I have added a sleep 10 line to the job to simulate a slow job to show the redirect to the attachment mask controller.

In order to ensure attachments need to be masked, in the staging Rails console run: FoiAttachment.update_all(masked_at: nil)

@gbp
Copy link
Member Author

gbp commented Oct 25, 2023

The reason for this PR is that we saw Apache had been stopped on the WDTK production server. This was due to oom-kill running:
Oct 24 15:58:49 mstrwdtkhexa systemd[1]: apache2.service: Failed with result 'oom-kill'

Prior to this happening these processes were taking the most memory:

process, memory
1721111, 12%
1721824, 11%
1720565, 10%
1721559, 10%
1721971, 10%
1799539, 10%

Looking at each:

  • 1721111 jumped from 896448 KB to 2926488 KB when processing an AttachmentController#show_as_html request at 15:57:23 but wasn't stuck processing when oom-kill ran
  • 1721824 was at 787944 KB before starting processing an FoiAttachmentMaskJob job inline (not sidekiq) for an AttachmentController#show_as_html request at 15:53:39
  • 1720565 jumped from 1199208 KB to 2639880 KB when processing an AttachmentController#show_as_html request at 15:57:14 but wasn't stuck processing when oom-kill ran
  • 1721559 was at 856708 KB before starting processing an FoiAttachmentMaskJob job inline (not sidekiq) for an AttachmentController#show_as_html request at 15:53:34
  • 1721971 was at 706764 KB before starting processing an FoiAttachmentMaskJob job inline (not sidekiq) for an AttachmentController#show_as_html request at 15:56:07
  • 1799539 was at 316636 KB before starting processing an FoiAttachmentMaskJob job inline (not sidekiq) for an AttachmentController#show_as_html request at 15:53:37

Copy link
Member

@garethrees garethrees left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed on a call together. Looks great!

@gbp gbp merged commit 53fa606 into develop Oct 26, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update "view as HTML" to do processing into the background - masking
2 participants