Index on task instance

[babinos87]

Added index ti_state_incl_start_date on taskinstance table. This index seems to have great positive effect when querying this table in a setup with many millions such rows. Existing index ti_state_lkp is not heavily utilized, as the start_date column (not part of the existing index) needs to be selected, hence a sequential scan will be performed in a very large table. Including the start_date column makes a use of INDEX ONLY SCAN, which improves performance dramatically in such situations.

This change is applicable to PostgreSQL database backends only, as other databases do not support this feature.

---

Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

[boring-cyborg]

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
Here are some useful points:

• Pay attention to the quality of your code (ruff, mypy and type annotations). Our pre-commits will help you with that.
• In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
• Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
• Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
• Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
• Be sure to read the Airflow Coding style.
  Apache Airflow is a community-driven project and together we are making it better 🚀.
  In case of doubts contact the developers at:
  Mailing List: dev@airflow.apache.org
  Slack: https://s.apache.org/airflow-slack

[potiuk]

It looks good, I do not see any bad side effects of it. Anyone else wants to chime in?

[uranusjr]

If the airflow_version part is figured out.

[boring-cyborg]

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

[dstandish]

Hi @babinos87 , I happened to be looking a bit at index utilization and noticed that this one isn't getting used on our cluster. I saw you shared a query that this benefits. Is that emitted from our codebase? Or is it just a custom query that you run? Thanks

[babinos87]

@dstandish this is a custom query, we have some custom code run by the scheduler to find the last successful dag run date, which runs this query and this index helps greatly.

[dstandish]

Thanks @babinos87

In that case, I don't think this belongs in airflow so I will plan to remove it. You can ensure that the index is there yourself as part of your infra process.

Thanks

[babinos87]

@dstandish this would make sense if there was a way to define custom sql schema / commands as part of the installation process. As far as I am aware, there is no such thing, and any manual changes could potentially break on db migration. I think it would be more appropriate to plan some work for this, so anyone could use it to define custom schema without risking of breaking the database between upgrades?

In any case, I don't foresee any trouble caused by this extra index, and it would help if it stays - my use case is quite basic so I think that someone else could benefit from this too.

[potiuk]

I think it would be more appropriate to plan some work for this, so anyone could use it to define custom schema without risking of breaking the database between upgrades? In any case, I don't foresee any trouble caused by this extra index, and it would help if it stays - my use case is quite basic so I think that someone else could benefit from this too.

You are free to propose somethign if you could ask - for example - a PR where you would allow anyone to (for example) plug -in a script that will be used as a step after migration and a way to add it to be able to add after-migration steps, that might be a good contribution that you could do.

As anyone else in the community you can propose PRs there, and it's a nice and isolated one that you are probably one of the best people to test and have need for it, so it would be nice if you could propose one (or find / sponsor/ encourage someone who could contribute it if you do not feel like you have the right skills)

[dstandish]

@dstandish this would make sense if there was a way to define custom sql schema / commands as part of the installation process. As far as I am aware, there is no such thing, and any manual changes could potentially break on db migration. I think it would be more appropriate to plan some work for this, so anyone could use it to define custom schema without risking of breaking the database between upgrades?

In any case, I don't foresee any trouble caused by this extra index, and it would help if it stays - my use case is quite basic so I think that someone else could benefit from this too.

I don't think it's likely to break anything. You can simply name it my_company_index_blah_blah and we won't clash with it. Sure if we drop a column that it uses (unlikely) then you might have a problem. But I think that's I think a risk that you must accept given that you are doing something custom with the scheduler that requires a special index.

Indexes are not free. When you have more indexes, that is more storage used, more work for postgres on each update, and more for the query planner to sort through. It doesn't make sense to force all airflow users in the world to have an index that doesn't help them at all, just so that you can use it. I think we have like 9 indexes or something on task instance? That's getting quite high. We can't only add indexes, we must also try to remove ones that are not useful.

Here's the other thing.... The indexes we include in airflow, you know, they might not be perfect for everyone's cluster. It's possible that some clusters might have an unusual usage pattern and maybe as a result they might need to add special index, even without having custom logic or queries. And even in that case it might not make sense to add such an index to airflow, given that doing so would be a cost for everyone and a benefit perhaps only to one. Right?