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

Revert to using Paperclip's filesystem storage, and fix dangling records in remove_remote #8339

Merged
merged 2 commits into from Aug 21, 2018

Conversation

Projects
None yet
5 participants
@ThibG
Collaborator

ThibG commented Aug 21, 2018

No description provided.

@ThibG

This comment has been minimized.

Collaborator

ThibG commented Aug 21, 2018

Actually, that may be too much of performance hit, #exists? checks for the actual existence of the file afaict.

@ThibG ThibG force-pushed the ThibG:fixes/redownload-remote branch 2 times, most recently from a33998e to 4868b15 Aug 21, 2018

@ThibG

This comment has been minimized.

Collaborator

ThibG commented Aug 21, 2018

Ok, for some reason, there are some media for which #exists? is false while #present? is true.
This shouldn't happen, I don't know why it did, but I updated the worker to destroy the attachment (and thus the record claiming the file exists) regardless of whether the actual file exists.

@ThibG

This comment has been minimized.

Collaborator

ThibG commented Aug 21, 2018

It doesn't solve everything as Paperclip/fog is throwing errors at me.

2018-08-21T09:40:29.463Z 19269 TID--blvp3j WARN: Errno::ENOENT: No such file or directory - getcwd
2018-08-21T09:40:29.474Z 19269 TID--blvp3j WARN: /home/mastodon/live/vendor/bundle/ruby/2.5.0/gems/fog-local-0.5.0/lib/fog/storage/local/models/file.rb:57:in `pwd'
/home/mastodon/live/vendor/bundle/ruby/2.5.0/gems/fog-local-0.5.0/lib/fog/storage/local/models/file.rb:57:in `block in destroy'
/home/mastodon/live/vendor/bundle/ruby/2.5.0/gems/fog-local-0.5.0/lib/fog/storage/local/models/file.rb:48:in `times'
/home/mastodon/live/vendor/bundle/ruby/2.5.0/gems/fog-local-0.5.0/lib/fog/storage/local/models/file.rb:48:in `destroy'
/home/mastodon/live/vendor/bundle/ruby/2.5.0/gems/paperclip-6.0.0/lib/paperclip/storage/fog.rb:135:in `block in flush_deletes'
/home/mastodon/live/vendor/bundle/ruby/2.5.0/gems/paperclip-6.0.0/lib/paperclip/storage/fog.rb:133:in `each'
/home/mastodon/live/vendor/bundle/ruby/2.5.0/gems/paperclip-6.0.0/lib/paperclip/storage/fog.rb:133:in `flush_deletes'
/home/mastodon/live/vendor/bundle/ruby/2.5.0/gems/paperclip-6.0.0/lib/paperclip/attachment.rb:242:in `save'
/home/mastodon/live/vendor/bundle/ruby/2.5.0/gems/paperclip-6.0.0/lib/paperclip/attachment.rb:270:in `destroy'

EDIT: this may be because of fog/fog-local#5
EDIT2: I'm therefore looking at reverting to Paperclip's internal backend for local filestorage instead of fog

@ThibG ThibG changed the title from Fix redownloading remote media to Revert to using Paperclip's filesystem storage, and fix dangling records in remove_remote Aug 21, 2018

@ThibG

This comment has been minimized.

Collaborator

ThibG commented Aug 21, 2018

Switching back to Paperclip's internal filesystem storage backend and changing the worker to also delete dangling records allowed me to clean ~40k attachment records without errors, some were genuine files that needed cleaning, and some of them were dangling files because of previous failed run.
Re-running the task is not populated with dangling records anymore and remote media re-download works as expected.

@ThibG ThibG requested review from ykzts and Gargron Aug 21, 2018

@ykzts

ykzts approved these changes Aug 21, 2018

@ThibG ThibG referenced this pull request Aug 21, 2018

Closed

can't remove_remote #8145

2 of 2 tasks complete
@nightpool

This comment has been minimized.

Collaborator

nightpool commented Aug 21, 2018

why and when did we switch to fog-local originally?

@ThibG

This comment has been minimized.

Collaborator

ThibG commented Aug 21, 2018

@nightpool we switched last year, because we started using fog for other backends, see #5604
EDIT: (as far as I know, only fog-local is affected by this issue)

ThibG added some commits Aug 21, 2018

Revert to using Paperclip's filesystem backend instead of fog-local
fog-local has lots of concurrency issues, causing failure to delete files,
dangling file records, and spurious errors UncacheMediaWorker

@ThibG ThibG force-pushed the ThibG:fixes/redownload-remote branch from 62e9a22 to 5c20e2b Aug 21, 2018

@Gargron

This comment has been minimized.

Member

Gargron commented Aug 21, 2018

How is CircleCI still not finished...

@ThibG

This comment has been minimized.

Collaborator

ThibG commented Aug 21, 2018

@Gargron I re-pushed because it just refused to run last time I pushed.
It's maybe due to the fact that you are out-running your plan by about 500% :x

@Gargron Gargron merged commit f06fa09 into tootsuite:master Aug 21, 2018

11 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: check-i18n Your tests passed on CircleCI!
Details
ci/circleci: install Your tests passed on CircleCI!
Details
ci/circleci: install-ruby2.3 Your tests passed on CircleCI!
Details
ci/circleci: install-ruby2.4 Your tests passed on CircleCI!
Details
ci/circleci: install-ruby2.5 Your tests passed on CircleCI!
Details
ci/circleci: test-ruby2.3 Your tests passed on CircleCI!
Details
ci/circleci: test-ruby2.4 Your tests passed on CircleCI!
Details
ci/circleci: test-ruby2.5 Your tests passed on CircleCI!
Details
ci/circleci: test-webui Your tests passed on CircleCI!
Details
codeclimate All good!
Details
@gllmhyt

This comment has been minimized.

gllmhyt commented Aug 23, 2018

Can you please release with this fix as soon as possible? My server media storage is getting out of hand and will soon have its drive fully loaded and be unusable: unable to run, unable to boot, requiring manual deletion of files where it should have been automatically deleted.

kyori19 added a commit to kyori19/mastodon that referenced this pull request Sep 20, 2018

Revert to using Paperclip's filesystem storage, and fix dangling reco…
…rds in remove_remote (tootsuite#8339)

* Fix uncaching worker

* Revert to using Paperclip's filesystem backend instead of fog-local

fog-local has lots of concurrency issues, causing failure to delete files,
dangling file records, and spurious errors UncacheMediaWorker
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment