Search: Use RUM index.

I think we can do the following

config :pleroma, :database,
  rum_enabled: true

Then create two new tasks in pleroma.database. One of which will enable rum and drop gin, and the other one would drop rum and enable gin. I don't think it is a good idea to force existing installations to install rum considering it does not bring any new functionality, only performance improvements. Also iirc not many hosted postgres db providers have rum in their extension list.

Is there a warning if rum does not exists or it would silently fail?

as I discussed with lain on fedi yesterday, this will ultimately be an optional feature since rum indexes aren't mainline.

@plataformatec do you guys have experience with these kinds of 'optional' migrations?

That's a very good question.

Today we don't really support optional migrations out of the box.

One option is to wrap the whole change function if a conditional check:

if Application.get_env(:pleroma, :database)[:rum_enabled] do
  ...
end

The downside of this approach is that once you run the migration, if you enable run, you can't enable it again.

Another option is to create a task that loads migrations from other directories. For example, you could put those migrations in priv/optional_migrations/rum_enabled/XYZ_do_something.exs. Then, in order to run those migrations, you can do something like this:

Ecto.Migrator.with_repo(repo, fn repo ->
  # The default migration directory
  Ecto.Migrator.run(repo, :up, all: true)

  if Application.get_env(:pleroma, :database)[:rum_enabled] do
    Ecto.Migrator.run(repo, "priv/optional_migrations/rum_enabled", :up, all: true)
  end
end)

Then you put that inside "pleroma.migrate" and then you will always run all of the appropriate migrations depending on the user configuration. The benefit of this approach is that you have full control but you may also end up re-implementing the existing ecto.migrate/ecto.rollback commands. Note also we are using Ecto.Migrator.with_repo/2, which requires Ecto.SQL 3.1.2+ (released this weekend).

Another idea is to send a PR to Ecto.SQL so we support a --migration-path option in Ecto. It should be relatively straight-forward to do so, and we will be happy to provide any guidance. With such flag, then people interested in using those special features can run those commands themselves, but it means they will be the ones doing most of the work.

The other thing you will want to consider is testing. If you start having a bunch of optional migrations, you need to make sure your suite passes on all possible combinations. Luckily this can be done by using multiple environment variables on CI and handling those accordingly.

Thoughts?

The --migration-path idea sounds interesting. Would that take a directory or a migration file as argument?

A directory with all migrations, so it should likely be called --migrations-path. Good point!

I am also for doing --migrations-path, and maybe aliasing pleroma.migrate/pleroma.rollback to

defp migrate(args) do
   Mix.Tasks.Ecto.Migrate.run(args)

  if Pleroma.Config.get([:database, :rum_enabled]),
    do: Mix.Tasks.Ecto.Migrate.run(["--migrations-path priv/optional_migrations/rum_enabled" | args])
end

defp rollback(args) do
  Mix.Tasks.Ecto.Rollback.run(args)

  if Pleroma.Config.get([:database, :rum_enabled]),
    do: Mix.Tasks.Ecto.Rollback.run(["--migrations-path priv/optional_migrations/rum_enabled" | args])
end

for convenience

So I went ahead and implementations --migrations-path here: https://github.com/elixir-ecto/ecto_sql/commit/e839a9a327b632d73533ac8105ba360bc831cf83

Feel free to give it a try and, if it works, we can do a new release.

Btw, you will have to split the option from the argument when calling run, like this:

Mix.Tasks.Ecto.Migrate.run(["--migrations-path", "priv/optional_migrations/rum_enabled" | args])

Other than that everything looks good!

That commit looks like it will work well for us.

added 1 commit

f1e67bdc - Search: Add optional rum indexing / searching.

Compare with previous version

mentioned in merge request !1160 (merged)

added 157 commits

f1e67bdc...e5b34f5e - 156 commits from branch develop
412a3d8a - Merge branch 'develop' of git.pleroma.social:pleroma/pleroma into rum-index

Compare with previous version

added 1 commit

6a8491d9 - CI: Add rum variant testing.

Compare with previous version

added 1 commit

3c0fe26f - CI: Add rum variant testing.

Compare with previous version

added 1 commit

89391a3f - CI: Add rum variant testing.

Compare with previous version

added 1 commit

feb1edf3 - CI: Add rum variant testing.

Compare with previous version

added 1 commit

b79618ef - CI: Add rum variant testing.

Compare with previous version

added 1 commit

a3fc7294 - CI: Add rum variant testing.

Compare with previous version

resolved all discussions

added 3 commits

022e6e4b - RUM: Remove vacuum analyze from migration
8784a7d1 - RUM: Set rum status by the environment.
b986ec49 - CI: Use the correct image with the correct hostname.

Compare with previous version

added 1 commit

ef63cf70 - CI: Use the correct image with the correct hostname.

Compare with previous version

unmarked as a Work In Progress

I didn't wrap the migration in a way that it gets automatically run when the rum setting is set, mainly because I want people to pay attention when adding it and really know what they are doing.

Nice!

merged

mentioned in commit d2dacadb

Search: Use RUM index.

Merged by rinpatch 5 years ago (May 17, 2019 4:35pm UTC) 5 years ago

Activity

Search: Use RUM index.

Merge request reports

Merged by rinpatch 5 years ago (May 17, 2019 4:35pm UTC) 5 years ago

Activity