AkkomaGang/akkoma

Fork 118

Set cleanup_attachments to true by default #910

Merged

floatingghost merged 1 commit from norm/akkoma:cleanup-attachments-default-true into develop

2025-05-09 16:49:50 +00:00

norm commented

2025-05-06 16:50:07 +00:00

Contributor

Since bcfbfbcff5 (part of
https://akkoma.dev/AkkomaGang/akkoma/pulls/789), attachment cleanup no
longer carries a significant performance overhead.

Most admins are unaware of this option even existing, but may notice an
increase in the size of the uploads directory (or S3 bucket size if used
instead) even if auto-expiring posts are used. This should hopefully
make this problem more manageable.

Since bcfbfbcff594d3b4dc9241ad38df5c1ca5729145 (part of <https://akkoma.dev/AkkomaGang/akkoma/pulls/789>), attachment cleanup no longer carries a significant performance overhead. Most admins are unaware of this option even existing, but may notice an increase in the size of the uploads directory (or S3 bucket size if used instead) even if auto-expiring posts are used. This should hopefully make this problem more manageable.

Oneric approved these changes

2025-05-08 12:21:41 +00:00

Oneric left a comment

Owner

The impact is still noticeable for large instances as shown in your report over in the SQL outlier issue, but it should indeed now be bearable for most.

This still suffers from issues like not being able to clean up attachments which didn’t end up being used in posts and not being able to delete anything after a domain migration since the approach from #818 was rejected and no alternative found.
However even with this in mind, enabling it in its current form is imho likely an improvement for the vast majority and it can still be disabled if necessary, so I think this makes sense

The impact is still noticeable for large instances as shown in your report over in the SQL outlier issue, but it should indeed now be bearable for most. This still suffers from issues like not being able to clean up attachments which didn’t end up being used in posts and not being able to delete anything after a domain migration since the approach from #818 was rejected and no alternative found. However even with this in mind, enabling it in its current form is imho likely an improvement for the vast majority and it can still be disabled if necessary, so I think this makes sense

norm force-pushed cleanup-attachments-default-true from a59b2aa6c9

ci/woodpecker/pr/build-amd64 Pipeline is pending approval

Details

ci/woodpecker/pr/build-arm64 Pipeline is pending approval

Details

ci/woodpecker/pr/docs Pipeline is pending approval

Details

ci/woodpecker/pr/lint Pipeline is pending approval

Details

ci/woodpecker/pr/test/1 Pipeline is pending approval

Details

ci/woodpecker/pr/test/2 Pipeline is pending approval

Details

to 8712e06d27

ci/woodpecker/pr/lint Pipeline was successful

Details

ci/woodpecker/pr/test/2 Pipeline was successful

Details

ci/woodpecker/pr/test/1 Pipeline was successful

Details

ci/woodpecker/pr/build-arm64 Pipeline was successful

Details

ci/woodpecker/pr/build-amd64 Pipeline was successful

Details

ci/woodpecker/pr/docs Pipeline was successful

Details

ci/woodpecker/pull_request_closed/lint Pipeline was successful

Details

ci/woodpecker/pull_request_closed/test/2 Pipeline was successful

Details

ci/woodpecker/pull_request_closed/test/1 Pipeline was successful

Details

ci/woodpecker/pull_request_closed/build-arm64 Pipeline was successful

Details

ci/woodpecker/pull_request_closed/build-amd64 Pipeline was successful

Details

ci/woodpecker/pull_request_closed/docs Pipeline was successful

Details

2025-05-08 22:15:23 +00:00

Compare

norm commented

2025-05-08 22:19:12 +00:00

Author

Contributor

The impact is still noticeable for large instances as shown in your report over in the SQL outlier issue, but it should indeed now be bearable for most.

Reworded the commit to reflect this.

This still suffers from issues like not being able to clean up attachments which didn’t end up being used in posts and not being able to delete anything after a domain migration since the approach from #818 was rejected and no alternative found.
However even with this in mind, enabling it in its current form is imho likely an improvement for the vast majority and it can still be disabled if necessary, so I think this makes sense

There is also the problem of orphaned attachments not being cleaned up if the posts they were in were removed prior to the config being set. One brute force soltion to that I can think of is scanning the uploads dir/bucket for files that aren't referenced and marking those for deletion. But then we'd probably run into the domain migration issue thing. Would like more ideas for that, but I'd imagine that's best discussed in #818 instead.

> The impact is still noticeable for large instances as shown in your report over in the SQL outlier issue, but it should indeed now be bearable for most. Reworded the commit to reflect this. > This still suffers from issues like not being able to clean up attachments which didn’t end up being used in posts and not being able to delete anything after a domain migration since the approach from #818 was rejected and no alternative found. However even with this in mind, enabling it in its current form is imho likely an improvement for the vast majority and it can still be disabled if necessary, so I think this makes sense There is also the problem of orphaned attachments not being cleaned up if the posts they were in were removed prior to the config being set. One brute force soltion to that I can think of is scanning the uploads dir/bucket for files that aren't referenced and marking those for deletion. But then we'd probably run into the domain migration issue thing. Would like more ideas for that, but I'd imagine that's best discussed in #818 instead.