Skip to content

Cleanup indicies #526

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

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from
Draft

Conversation

mdashlw
Copy link
Contributor

@mdashlw mdashlw commented Apr 27, 2025

Before you begin

  • I understand my contributions may be rejected for any reason
  • I understand my contributions are for the benefit of Derpibooru and/or the Philomena software
  • I understand my contributions are licensed under the GNU AGPLv3
  • I understand all of the above

Requires a total reindex. WIP PR until we decide to do that

  • Removes unused fields
  • Adds updated_at to comments
  • Adds thumbnail_id and spoiler_warning to galleries
  • Removes watcher_ids from galleries
  • Adds scratchpad to images
  • Adds deleted_by_user_id, deleted_by_user, and deletion_reason to posts & comments
  • Adds author to anon posts & comments
  • Simplifies namespaced_tags on images
  • Adds destroyed_content to posts & comments

Closes #517 and #519

@liamwhite
Copy link
Contributor

Should this be marked as draft so it doesn't get merged unintentionally?

@mdashlw mdashlw marked this pull request as draft April 27, 2025 13:43
@mdashlw mdashlw linked an issue Apr 27, 2025 that may be closed by this pull request
@Meow
Copy link
Member

Meow commented Apr 28, 2025

Merging blocked until we're ready to update the boorus, aka until Debian Trixie is released (june-august time window).

@MareStare
Copy link
Contributor

Merging blocked until we're ready to update the boorus, aka until Debian Trixie is released (june-august time window).

Could you elaborate on why this change is blocked by updating the underlying OS?

@Meow
Copy link
Member

Meow commented Apr 28, 2025

It's not blocked by the update itself, but by the logistics required to deploy it. Basically to deploy this and the other indexing change, I'd have to delete all search indices and reindex anew, something that has to be done while the site is offline and will take 45-60 minutes. Since a debian distro version upgrade requires a host reboot and some downtime too, why not just combine the two into one big downtime.

@MareStare
Copy link
Contributor

I see, that makes sense. @mdashlw do you think reindexing is actually required after this PR is merged? I haven't looked into it closely yet, so I'm just asking. What if we merge this PR and don't do a reindex, does it mean the boorus become more broken than they are today, or not? If not, then merging should be harmless

@Meow
Copy link
Member

Meow commented Apr 28, 2025

It would require a reindex lest things will be hilariously broken.

@liamwhite
Copy link
Contributor

do you think reindexing is actually required after this PR is merged?

lest things will be hilariously broken

To be specific, the changes are incompatible (namespaced_tags structure vs tags).

@MareStare MareStare added the requires-reindex The change requires some downtime to delete and recreate indexes in OpenSearch label May 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
requires-reindex The change requires some downtime to delete and recreate indexes in OpenSearch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

author should be indexed on anon comments/posts Index deletion reason on comments/posts
4 participants