[24.0] Workflow Comment Indexing

[ElectronicBlueberry]

usegalaxy.org has performance issues with workflows which contain Comments.
After having a closer look at the database model, @davelopez and I suspect this could be due to missing indexes.

This PR adds those indexes.

How to test the changes?

(Select all options that apply)

• I've included appropriate automated tests.
• This is a refactoring of components with existing test coverage.
• Instructions for manual testing are as follows:
  1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

• I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

[mvdbeek]

Adding the index makes sense, but can you say more about the performance issues ? Is that in the editor or the list index ? Which endpoint are you seeing this with ? Outside of the editor and workflow export I would hope we don't need to join against the comment model.

[mvdbeek]

Weren't comments part of 23.2 as well ? In which case you want to target 23.2

[ElectronicBlueberry]

Adding the index makes sense, but can you say more about the performance issues ? Is that in the editor or the list index ? Which endpoint are you seeing this with ? Outside of the editor and workflow export I would hope we don't need to join against the comment model.

This is on the editor, specifically the load workflow endpoint

[mvdbeek]

Weren't comments part of 23.2 as well ? In which case you want to target 23.2

I take that back, it's a little unexpected that we'll have a migration against an existing release, I think 24.0 is fine for now as that's not yet out and I think it's not super-critical that the editor load is slow (how slow are we talking btw ?)

[ElectronicBlueberry]

I take that back, it's a little unexpected that we'll have a migration against an existing release, I think 24.0 is fine for now as that's not yet out and I think it's not super-critical that the editor load is slow (how slow are we talking btw ?)

about 10-20 seconds

[mvdbeek]

Can you share a workflow ?

[davelopez]

You can try this one: https://usegalaxy.org/published/workflow?id=174d3de4589911fa

[martenson]

You can try this one: https://usegalaxy.org/published/workflow?id=174d3de4589911fa

ups, that one is 30s for me

[jdavcs]

We need to add an index as a database migration as a separate commit (see #15733 for an example)

[martenson]

@jdavcs do you mean we need one migration commit and one index addition commit per index?

[jdavcs]

we can add multiple indexes in one migration. But, ideally, we need one db migration per commit (i.e. a commit containing a new db revision script).
The index should be added to the model and, explicitly, to the database. The edit in model/__init__.py makes the model change, the revision script changes the database.

[ElectronicBlueberry]

That is what the second commit in this PR does

[jdavcs]

That is what the second commit in this PR does

which, obviously, I missed!

[jdavcs]

Looks good, sorry for missing the second commit! 😄