[DISCUSS] indexes for API calls

[Daniel Standish <da...@astronomer.io.INVALID>]

Historically we have added indexes as needed for the performance of airflow
itself and not for the rest API.

Lately we've observed more usage of task instances list endpoint and
specifically filtering on end_date and / or start_date and / or
execution_date.

One line of argument goes that every possible filter in the rest API should
have an index.  But not all users use the API at all, let alone use all
params in the rest API.  Or even *all possible combinations?!*

Another argument would be "this should be the responsibility of the cluster
maintainer", since it's not part of the core operation of airflow
(scheduler / tasks / webserver) and highly dependent on specific use case.

Another thing to consider is sometimes there's an efficient way to get the
TIs you're looking for already with a slight refactor.  E.g. get the dag
runs first, then fetch the TIs using the PK.

Of course adding indexes is a trade off between storage space,
performance overhead maintaining them etc, balanced against reasonable
usage of the REST API.

I'm curious what folks think about this, and whether and when we should add
indexes in OSS that help REST API queries.

Re: [DISCUSS] indexes for API calls

[Jarek Potiuk <ja...@potiuk.com>]

Yes. I agree. Airflow REST API does not have to be super efficient with the
exception of designated calls which are really designed to be efficient. We
might want to speed up some things for the Airflow Ui purposes for example.
And it should always be explicit decision when we add new filtering etc.

But since we know users will have some needs there - we should be explicit
about what they can and cannot do.

I think it's just worth documenting as design decision. And either
explicitly mark those 'designed to be efficient' or those that may be not
efficient. (The latter is probably better - we likely have just a few of
those filters).

Then we should tell the users who need some.of the efficiency might make
other decisios - for example extracting the data separately.inceementaly to
another data source and querying it from there with their own offline
tables and indexes. And we should tell them 'do not modify airflow db to
achieve efficiency - this is not the way to go and will break things''

This way nobody can complain about it - when we explicity state a design
decision and tell the users what they can do on their own to get what they
want (without them.modifying internal DB) - this seems like a complete
solution.

Explicit is better than implicit.


J.

wt., 18 cze 2024, 00:58 użytkownik Daniel Standish
<da...@astronomer.io.invalid> napisał:

> I tinkered with this a bit.  I think the case to add indexes for "REST" API
> may be weaker than even *I* thought (and I thought it was weak / was mostly
> against it).
>
> The reason is cus there are reasons beyond indexing why such API calls are
> slow, and adding indexes may not even help that much.  For example the TI
> list endpoint lists TIs. TIs have a *lot* of information distributed across
> many tables.  We always load all of it.  Sometimes multiple queries to get
> it.  That's just not going to be an efficient way to implement change data
> capture a.k.a. replication.
>
> I think the main responsibility of airflow is to be performant with regard
> to task execution broadly defined.  Anything beyond that is sort of not
> relevant.  And getting data out of the metastore for ancillary purposes is
> outside that line IMO.  The range of possible queries etc are too varied to
> reasonably optimize for.  So it's best left to the person wearing the "dba"
> hat at the organization.
>
> Users can also add endpoints via plugins that query the data more
> efficiently.
>
> That's my take in a nutshell I guess.
>

Re: [DISCUSS] indexes for API calls

[Daniel Standish <da...@astronomer.io.INVALID>]

I tinkered with this a bit.  I think the case to add indexes for "REST" API
may be weaker than even *I* thought (and I thought it was weak / was mostly
against it).

The reason is cus there are reasons beyond indexing why such API calls are
slow, and adding indexes may not even help that much.  For example the TI
list endpoint lists TIs. TIs have a *lot* of information distributed across
many tables.  We always load all of it.  Sometimes multiple queries to get
it.  That's just not going to be an efficient way to implement change data
capture a.k.a. replication.

I think the main responsibility of airflow is to be performant with regard
to task execution broadly defined.  Anything beyond that is sort of not
relevant.  And getting data out of the metastore for ancillary purposes is
outside that line IMO.  The range of possible queries etc are too varied to
reasonably optimize for.  So it's best left to the person wearing the "dba"
hat at the organization.

Users can also add endpoints via plugins that query the data more
efficiently.

That's my take in a nutshell I guess.

Re: [DISCUSS] indexes for API calls

[Daniel Standish <da...@astronomer.io.INVALID>]

Lots of good discussion here.  We should create separate threads for the
questions about (1) whether to keep or drop mysql / mssql / sqlite /
mongodb / just-pipe-to-/dev/null/ and (2) to UUID or not to UUID and (3)
database agnosticism a.k.a. an interface.

But some responses...

Using UUIDS was the proposal how we can bypass the limitation of MYSQL for
> Airflow 2 when we discussed whether to do a "simple" version of team-prefix
> in dag id, or whether we want to mess with adding
> yet-another-field-to-indexes-that-are-already-too-long-for-mysql and it was
> based on the assumption we are going to that for versioning already, so I
> thought that is the plan :). Main reason why I put the multi-team voting on
> hold while we decide on the scope of Airflow 3 to be honest.


Yeah I think that folks *thought* that we would be forced to go UUID by
adding try_number to grain of a "try history" type of table but that turned
out not to be the case so I don't think any of the dag versioning AIPs will
result in a new UUID PK, at least not in 2.x.  We realized we could achieve
the user-facing aims of those AIPs in 2.10 without a big painful migration
and figured best to avoid such a thing unless necessary in 2.x since there
will be migration pain anyway in 3.0 and we'll have more freedom then
anyway (now that it appears the community is coalescing around 3.0
happening).

Also just to be sure, the slowness you observe for listing task instances
> only appears with a certain combination of filters, while some other
> combinations, yielding a comparable result set in terms of 'size' are much
> quicker ? (Just trying to understand if the slowness comes from missing
> index, or also possibly from missing loading options that would cause lazy
> loading at serialization time slowing everything down, I don't see a lot of
> eager loading to prevent those, but maybe the few that we have are enough).


TI has no index on end_date.  If the table is sufficiently large, and you
list TIs within end_date range, it will be slow.

If we would like to add individual indexes for one field we bump into the
> problem that this are less effective when it comes to a filter/sorting over
> multiple different fields....


Yea, so this is sorta why I'd prefer if our position is that our indexing
strategy is to optimize specifically the the core operations of airflow,
mainly airflow-generated queries, or API calls that are operational -- such
as triggering a dag run, or mucking with datasets and the like.  But for
things like "just pulling data out of the db" using whatever filters you
want to use....  that feels more like a gray area that in a perfect world
we would not have to care about.  One compromise approach could could be we
have one indexed column for data takeout -- updated_at.  If every table had
an updated_at column that was indexed then a downstream process could
efficiently track changes and maybe we say "that's as good as you're going
to get" from us re data take out.  Then of course we might encounter
unforced errors when updated_at doesn't work right but that's another
story....

Rather than add these indexes (and every other "performance optimisation"
> measures) we should prove that this action gains benefits, rather than the
> opposite.


It's easy to prove that adding an index on end_date helps a filter on end
date for sufficiently large table.  It's 100%.  But the problem is it's not
100% of users that it helps because most of them don't do this.  This is
the sort of the main issue of this discussion thread: do we add the index
when it has 100% chance of helping (made-up number ...) 5% of people, but
all cost and no benefit for the rest?

Re: [DISCUSS] indexes for API calls

[Andrey Anshin <an...@taragol.is>]

> however it might or might not be affected. Similarly MariaDB, but MySQL does
not seem to have proper UUID support, so we should really use UUID7 rather
that UUID4 for such UUIDs in case we do not want to affect insert performance
on MySQL.

Some NITs here, I guess better to use deterministic UUID, such as v5 or v3
instead of random/timebased based like v1/v4/v7. So if you have all
components + hash you could always generate this UUID on client side,
instead of get it by JOIN or somewhere else

MySQL do not have native implementation, however SQLAlchemy 2 has a
dialect-neutral UUID, that mean if DB backend supports it natively, then it
will use it otherwise CHAR(32):
https://docs.sqlalchemy.org/en/20/core/type_basics.html#sqlalchemy.types.Uuid
, I don't think that is a problem to have it also in SA 14


On Fri, 31 May 2024 at 19:28, Vincent Beck <vi...@apache.org> wrote:

> Interesting thread.
>
> I think what makes this discussion complex is that Airflow makes a lot of
> different queries (API, Scheduler, ...). I think it is even harder to keep
> track of all the different queries Airflow makes and thus, hard to figure
> if such index is needed. Also, Airflow evolves (and rightfully so),
> therefore, one index might have been useful in the past but not anymore due
> to some modifications in the code.
>
> What would help us is a clear contract between the database and Airflow,
> meaning what are all the operations/queries Airflow needs. I know it had
> been mentioned during the last community sync and some folks are against
> but I wanted to reiterate this idea of having an API/interface for any DB
> operation. I dont know the consequences in terms of performance (especially
> for the scheduler) of even feasibility, but I think it might be worth to
> think about it? Especially because that would open some doors such as using
> NoSQL DB engines to run Airflow.
>
> (Sorry if I deviated the conversation a bit).
>
> On 2024/05/31 12:10:11 Andrey Anshin wrote:
> > IMHO, blindy adding new indexes into the `dag_run` and `task_instance`
> > tables will cause additional maintenance costs.
> >
> > There are 8 indexes already exists per each of this tables
> >
> > SELECT
> >     pi.schemaname schema_name,
> >     pi.tablename table_name,
> >     count(*) num
> > FROM pg_indexes pi
> > WHERE pi.schemaname = 'public'
> > GROUP BY 1, 2
> > ORDER BY 3 DESC, 2;
> >
> > And maintenance indexes overall it is not something cheap, every time
> when
> > the row changed/added (INSERT/UPDATE/DELETE/MERGE/COPY) there is a check
> > whether or not it is required to rebalance index tree (B-Tree indexes).
> >
> > If we would like to add individual indexes for one field we bump into the
> > problem that this are less effective when it comes to a filter/sorting
> over
> > multiple different fields (include PK), just because this action required
> > to create Bitmap Scan into the memory (Working Memory in postgres,
> default
> > something like 10 MiB), and in the end COB (Cost Base Optimisation)
> > mechanism more like decide to do sequential scan, as result indexes
> become
> > useless, and just use additional disc space and CPU for maintenance.
> >
> > Rather than add these indexes (and every other "performance optimisation"
> > measures) we should prove that this action gains benefits, rather than
> the
> > opposite. That might be achieved by enable on some prod-like environment
> > logs collection and analyze logs by pgBadger (
> > https://github.com/darold/pgbadger), that might shows different metrics
> > before changes and after the changes
> >
> > This all valid for Postgres, and I have no idea how to calculate measures
> > for MySQL, I hope we will reach an agreement to drop MySQL in Airflow 3.
> >
> > On Fri, 31 May 2024 at 12:10, Pierre Jeambrun <pi...@gmail.com>
> wrote:
> >
> > > Indeed Jarek I feel like this is another point in favor of stick to
> > > "Postgres"
> > >
> > > As mentioned, maybe we were a little reckless when adding all these
> kinds
> > > of filters. If they are not often used and we rarely / never see
> > > performance github issues on those, marking them as 'non optimised but
> here
> > > for convenience' could be a good short term solution. (And maybe
> removing
> > > those filters at some point)
> > >
> > > What people use the API for (replicating the DB, building an UI,
> etc...) do
> > > not really matter I think, Ideally I just think that the API should
> have a
> > > sort of SLA and guarantee a reasonable response time for all endpoints
> > > (through pagination, indexes, etc...). We can still document that some
> > > endpoints are known to be slower for some reason and leave them out of
> this
> > > performance constraint. (Having performance tests could be great
> otherwise
> > > it will be very hard to track).
> > >
> > > Also just to be sure, the slowness you observe for listing task
> instances
> > > only appears with a certain combination of filters, while some other
> > > combinations, yielding a comparable result set in terms of 'size' are
> much
> > > quicker ? (Just trying to understand if the slowness comes from missing
> > > index, or also possibly from missing loading options that would cause
> lazy
> > > loading at serialization time slowing everything down, I don't see a
> lot of
> > > eager loading to prevent those, but maybe the few that we have are
> enough).
> > >
> > > Le ven. 31 mai 2024 à 09:38, Jarek Potiuk <ja...@potiuk.com> a écrit :
> > >
> > > > And to be perfectly honest -  if people (like me) hesitate on
> settling on
> > > > architectural decisions because they are afraid that their changes
> might
> > > > have unintended consequences, because we want to support all the
> > > different
> > > > kinds of databases - this is one more reason we should stick to
> "Postgres
> > > > only".  This is a very good example where we have real slow-downs
> that
> > > > impact the speed the decisions are made (which impacts speed of
> feature
> > > > development).
> > > >
> > > > On Fri, May 31, 2024 at 9:13 AM Jarek Potiuk <ja...@potiuk.com>
> wrote:
> > > >
> > > > > Using UUIDS was the proposal how we can bypass the limitation of
> MYSQL
> > > > for
> > > > > Airflow 2 when we discussed whether to do a "simple" version of
> > > > team-prefix
> > > > > in dag id, or whether we want to mess with adding
> > > > > yet-another-field-to-indexes-that-are-already-too-long-for-mysql
> and it
> > > > was
> > > > > based on the assumption we are going to that for versioning
> already,
> > > so I
> > > > > thought that is the plan :). Main reason why I put the multi-team
> > > voting
> > > > on
> > > > > hold while we decide on the scope of Airflow 3 to be honest.
> > > > >
> > > > >
> > > > >
> > > >
> > >
> https://apache-airflow.slack.com/archives/C06K9Q5G2UA/p1714140873323569?thread_ts=1713607491.495499&cid=C06K9Q5G2UA
> > > > >
> > > > > J.
> > > > >
> > > > >
> > > > > On Fri, May 31, 2024 at 9:04 AM Daniel Standish
> > > > > <da...@astronomer.io.invalid> wrote:
> > > > >
> > > > >> Yes uuid is risky and problematic as primary key. If you do it you
> > > need
> > > > to
> > > > >> do carefully/ sequential.  But I think that we are not going with
> UUID
> > > > pk
> > > > >> on any tables at this time.
> > > > >>
> > > > >> BUT I do want to add a uuid for every TI try that is not PK but
> can be
> > > > >> used
> > > > >> as a more convenient identifier when tying together log events
> related
> > > > to
> > > > >> TIs emitted on different services.
> > > > >>
> > > > >> On Thu, May 30, 2024 at 11:53 PM Jarek Potiuk <ja...@potiuk.com>
> > > wrote:
> > > > >>
> > > > >> > Also if we are speaking about indexes - a bit tangential but I
> know
> > > we
> > > > >> were
> > > > >> > planning to replace some of the primary keys (mainly because of
> > > mysql
> > > > >> > limitations) with synthetic keys for DAG versioning casse where
> we
> > > > >> planned
> > > > >> > to use UUIDs).
> > > > >> >
> > > > >> > We should be very, very careful when doing it because I've
> learned
> > > > >> recently
> > > > >> > that due to UUID randomness the algorithm to rebalance binary
> tree
> > > do
> > > > >> not
> > > > >> > work well with random UUIDs which means that when you do a lot
> of
> > > > >> inserts,
> > > > >> > rebalancing of the trees is happening very often and
> performance of
> > > > >> inserts
> > > > >> > is heavily affected.  Postgres has a dedicated UUID field type
> for
> > > > that
> > > > >> > that produces an optimal index
> > > > >> >
> > > > >> >
> > > > >>
> > > >
> > >
> https://dba.stackexchange.com/questions/102448/how-should-i-index-a-uuid-in-postgres
> > > > >> > - however it might or might not be affected. Similarly MariaDB,
> but
> > > > >> MySQL
> > > > >> > does not seem to have proper UUID support, so we should really
> use
> > > > UUID7
> > > > >> > rather that UUID4 for such UUIDs in case we do not want to
> affect
> > > > insert
> > > > >> > performance on MySQL.
> > > > >> >
> > > > >> > WAT???? Yeah. That is. - I think - rather non-obvious
> side-effect.
> > > > >> >
> > > > >> > This learning I had adds much more strength to two related
> > > > discussions:
> > > > >> >
> > > > >> > 1) This only adds more fire to "let's only stick to Postgres".
> We
> > > can
> > > > >> > optimize the experience for our users way better and move
> faster.
> > > > >> >
> > > > >> > 2) Let's REALLY not put the power of deciding what indexes to
> put on
> > > > the
> > > > >> > database of ours in the hands of our users. There are many
> unobvious
> > > > >> > consequences of adding indexes and while we can aim to learn,
> > > foresee
> > > > >> and
> > > > >> > control them (mostly when we have a working performance test
> suite),
> > > > >> > getting a random user to put a random index on a random set of
> keys
> > > in
> > > > >> > Airflow DB might have a number of unobvious consequences which
> might
> > > > >> give
> > > > >> > our users (and us) a LOT of headache.
> > > > >> >
> > > > >> > J.
> > > > >> >
> > > > >> >
> > > > >> >
> > > > >> >
> > > > >> > On Fri, May 31, 2024 at 8:13 AM Pankaj Koti
> > > > >> > <pa...@astronomer.io.invalid> wrote:
> > > > >> >
> > > > >> > > Addressing one of Pierre's questions:
> > > > >> > >
> > > > >> > > Should I index foreign keys? Is that done by default or
> should I
> > > > >> > explicitly
> > > > >> > > do it?
> > > > >> > >
> > > > >> > > The answer varies depending on the database engine.
> > > > >> > > PostgreSQL and SQLite do not add indexes on foreign keys
> > > > >> > > by default, while MySQL does. Developers should keep this
> > > > >> > > in mind. Many of us (myself included) may have assumed
> > > > >> > > from our university days, indexes are always created for
> > > > >> > > foreign keys, but that's not always the case. :)
> > > > >> > >
> > > > >> > > Best regards,
> > > > >> > >
> > > > >> > > *Pankaj Koti*
> > > > >> > > Senior Software Engineer (Airflow OSS Engineering team)
> > > > >> > > Location: Pune, Maharashtra, India
> > > > >> > > Timezone: Indian Standard Time (IST)
> > > > >> > >
> > > > >> > >
> > > > >> > > On Fri, May 31, 2024 at 11:37 AM Daniel Standish
> > > > >> > > <da...@astronomer.io.invalid> wrote:
> > > > >> > >
> > > > >> > > > I would be in favor of this for sure.  Let's see what others
> > > think
> > > > >> :)
> > > > >> > > >
> > > > >> > > > On Thu, May 30, 2024 at 10:55 PM Jarek Potiuk <
> jarek@potiuk.com
> > > >
> > > > >> > wrote:
> > > > >> > > >
> > > > >> > > > > Simply speaking - let's make "lack of optimisation for
> these
> > > and
> > > > >> > that"
> > > > >> > > > part
> > > > >> > > > > of the API specification.
> > > > >> > > > >
> > > > >> > > > > On Fri, May 31, 2024 at 7:54 AM Jarek Potiuk <
> > > jarek@potiuk.com>
> > > > >> > wrote:
> > > > >> > > > >
> > > > >> > > > > > So let's document as part of the API which queries are
> not
> > > > >> > performant
> > > > >> > > > and
> > > > >> > > > > > suggest users that want to use them to make their
> analytics
> > > > >> queries
> > > > >> > > > > > elsewhere. I'd very much prefer that it's slow "by
> design"
> > > for
> > > > >> > > everyone
> > > > >> > > > > > rather than add option for the user to speed them up
> where
> > > we
> > > > >> > decided
> > > > >> > > > > > not to do it ourselves because we know the consequences.
> > > > >> > > > > >
> > > > >> > > > > > I think the root cause of the problem is `US` adding new
> > > > >> filtering
> > > > >> > > > > options
> > > > >> > > > > > to public API without thinking about and documenting
> > > > >> consequences.
> > > > >> > > > > >
> > > > >> > > > > > J.
> > > > >> > > > > >
> > > > >> > > > > >
> > > > >> > > > > > On Fri, May 31, 2024 at 7:38 AM Daniel Standish
> > > > >> > > > > > <da...@astronomer.io.invalid> wrote:
> > > > >> > > > > >
> > > > >> > > > > >> So I think the notion that *all possibly expensive
> queries*
> > > > >> should
> > > > >> > > > have
> > > > >> > > > > an
> > > > >> > > > > >> index to support them is not a tenable one.  E.g.
> there are
> > > > >> > > something
> > > > >> > > > > like
> > > > >> > > > > >> 5 params on TI list endpoint that don't have an index.
> > > > >> > > > > >>
> > > > >> > > > > >> In contrast with queries from airflow itself, the API
> > > queries
> > > > >> are
> > > > >> > > more
> > > > >> > > > > >> arbitrary -- user can combine in any way.  So it's not
> so
> > > > >> simple
> > > > >> > to
> > > > >> > > > > >> determine just from the endpoint params what indexes
> will
> > > be
> > > > >> > needed.
> > > > >> > > > > And
> > > > >> > > > > >> we're not going to add an index for all possible param
> > > > >> > combinations,
> > > > >> > > > > that
> > > > >> > > > > >> would be silly.  So I think like it or not I think we
> will
> > > > >> > > necessarily
> > > > >> > > > > >> draw
> > > > >> > > > > >> the line at *some* point and say we're trying to cover
> the
> > > > most
> > > > >> > > > likely /
> > > > >> > > > > >> normal / reasonable uses and if your usage pattern is
> an
> > > > >> outlier
> > > > >> > you
> > > > >> > > > > might
> > > > >> > > > > >> need to add an index yourself.  And I don't think
> that's
> > > > >> > > unreasonable
> > > > >> > > > > >>
> > > > >> > > > > >> Maybe at least one conclusion from this is to add an
> index
> > > > for
> > > > >> > each
> > > > >> > > of
> > > > >> > > > > >> start_date, end_date, and updated_at on TI.  Already
> the
> > > > >> storage
> > > > >> > > > > occupied
> > > > >> > > > > >> by indexes on TI is more than the table data size -- I
> > > don't
> > > > >> > > remember
> > > > >> > > > it
> > > > >> > > > > >> might have been like 2x or something.  So then index
> count
> > > > will
> > > > >> > > > increase
> > > > >> > > > > >> from 8 to 11 and overhead increases.  Whether this is a
> > > > >> material
> > > > >> > > > > concern,
> > > > >> > > > > >> I
> > > > >> > > > > >> am honestly not sure -- I am not a db specialist.  But
> it
> > > > does
> > > > >> > feel
> > > > >> > > > icky
> > > > >> > > > > >> to
> > > > >> > > > > >> add all the indexes when they are used by, I suspect, a
> > > very
> > > > >> small
> > > > >> > > > > >> percentage of airflow users overall.
> > > > >> > > > > >>
> > > > >> > > > > >> Part of this for me is also the reason these types of
> > > queries
> > > > >> are
> > > > >> > > run.
> > > > >> > > > > >> Typically it is for analytics purposes, either on the
> fly
> > > or
> > > > as
> > > > >> > part
> > > > >> > > > of
> > > > >> > > > > a
> > > > >> > > > > >> replication process to a DWH or something.  But there
> are
> > > > more
> > > > >> > > > efficient
> > > > >> > > > > >> mechanisms for this than pulling the data through the
> > > > >> webserver,
> > > > >> > > e.g.
> > > > >> > > > > >> read-only replicas, or standard ETL processes, or just
> > > making
> > > > >> > better
> > > > >> > > > use
> > > > >> > > > > >> of
> > > > >> > > > > >> the indexes that are already there.  Do we consider
> > > > >> "replication"
> > > > >> > a
> > > > >> > > > > >> reasonable use of the REST API?
> > > > >> > > > > >>
> > > > >> > > > > >> So I may very well be alone on this. But (1) the API
> is not
> > > > >> > > something
> > > > >> > > > > >> everyone uses.  And (2) even of users of the API,
> probably
> > > > >> only a
> > > > >> > > very
> > > > >> > > > > >> small percentage are using it in a way that needs these
> > > > >> indexes --
> > > > >> > > > > >> otherwise why haven't we seen requests for indexes on
> these
> > > > >> > columns
> > > > >> > > on
> > > > >> > > > > >> github?  So, to me, it doesn't seem crazy to just
> leave it
> > > to
> > > > >> the
> > > > >> > > > user /
> > > > >> > > > > >> platform to add.  But as usual, not a hill to die on.
> > > > >> > > > > >>
> > > > >> > > > > >
> > > > >> > > > >
> > > > >> > > >
> > > > >> > >
> > > > >> >
> > > > >>
> > > > >
> > > >
> > >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@airflow.apache.org
> For additional commands, e-mail: dev-help@airflow.apache.org
>
>

Re: [DISCUSS] indexes for API calls

[Vincent Beck <vi...@apache.org>]

Interesting thread.

I think what makes this discussion complex is that Airflow makes a lot of different queries (API, Scheduler, ...). I think it is even harder to keep track of all the different queries Airflow makes and thus, hard to figure if such index is needed. Also, Airflow evolves (and rightfully so), therefore, one index might have been useful in the past but not anymore due to some modifications in the code.

What would help us is a clear contract between the database and Airflow, meaning what are all the operations/queries Airflow needs. I know it had been mentioned during the last community sync and some folks are against but I wanted to reiterate this idea of having an API/interface for any DB operation. I dont know the consequences in terms of performance (especially for the scheduler) of even feasibility, but I think it might be worth to think about it? Especially because that would open some doors such as using NoSQL DB engines to run Airflow.

(Sorry if I deviated the conversation a bit).

On 2024/05/31 12:10:11 Andrey Anshin wrote:
> IMHO, blindy adding new indexes into the `dag_run` and `task_instance`
> tables will cause additional maintenance costs.
> 
> There are 8 indexes already exists per each of this tables
> 
> SELECT
>     pi.schemaname schema_name,
>     pi.tablename table_name,
>     count(*) num
> FROM pg_indexes pi
> WHERE pi.schemaname = 'public'
> GROUP BY 1, 2
> ORDER BY 3 DESC, 2;
> 
> And maintenance indexes overall it is not something cheap, every time when
> the row changed/added (INSERT/UPDATE/DELETE/MERGE/COPY) there is a check
> whether or not it is required to rebalance index tree (B-Tree indexes).
> 
> If we would like to add individual indexes for one field we bump into the
> problem that this are less effective when it comes to a filter/sorting over
> multiple different fields (include PK), just because this action required
> to create Bitmap Scan into the memory (Working Memory in postgres, default
> something like 10 MiB), and in the end COB (Cost Base Optimisation)
> mechanism more like decide to do sequential scan, as result indexes become
> useless, and just use additional disc space and CPU for maintenance.
> 
> Rather than add these indexes (and every other "performance optimisation"
> measures) we should prove that this action gains benefits, rather than the
> opposite. That might be achieved by enable on some prod-like environment
> logs collection and analyze logs by pgBadger (
> https://github.com/darold/pgbadger), that might shows different metrics
> before changes and after the changes
> 
> This all valid for Postgres, and I have no idea how to calculate measures
> for MySQL, I hope we will reach an agreement to drop MySQL in Airflow 3.
> 
> On Fri, 31 May 2024 at 12:10, Pierre Jeambrun <pi...@gmail.com> wrote:
> 
> > Indeed Jarek I feel like this is another point in favor of stick to
> > "Postgres"
> >
> > As mentioned, maybe we were a little reckless when adding all these kinds
> > of filters. If they are not often used and we rarely / never see
> > performance github issues on those, marking them as 'non optimised but here
> > for convenience' could be a good short term solution. (And maybe removing
> > those filters at some point)
> >
> > What people use the API for (replicating the DB, building an UI, etc...) do
> > not really matter I think, Ideally I just think that the API should have a
> > sort of SLA and guarantee a reasonable response time for all endpoints
> > (through pagination, indexes, etc...). We can still document that some
> > endpoints are known to be slower for some reason and leave them out of this
> > performance constraint. (Having performance tests could be great otherwise
> > it will be very hard to track).
> >
> > Also just to be sure, the slowness you observe for listing task instances
> > only appears with a certain combination of filters, while some other
> > combinations, yielding a comparable result set in terms of 'size' are much
> > quicker ? (Just trying to understand if the slowness comes from missing
> > index, or also possibly from missing loading options that would cause lazy
> > loading at serialization time slowing everything down, I don't see a lot of
> > eager loading to prevent those, but maybe the few that we have are enough).
> >
> > Le ven. 31 mai 2024 à 09:38, Jarek Potiuk <ja...@potiuk.com> a écrit :
> >
> > > And to be perfectly honest -  if people (like me) hesitate on settling on
> > > architectural decisions because they are afraid that their changes might
> > > have unintended consequences, because we want to support all the
> > different
> > > kinds of databases - this is one more reason we should stick to "Postgres
> > > only".  This is a very good example where we have real slow-downs that
> > > impact the speed the decisions are made (which impacts speed of feature
> > > development).
> > >
> > > On Fri, May 31, 2024 at 9:13 AM Jarek Potiuk <ja...@potiuk.com> wrote:
> > >
> > > > Using UUIDS was the proposal how we can bypass the limitation of MYSQL
> > > for
> > > > Airflow 2 when we discussed whether to do a "simple" version of
> > > team-prefix
> > > > in dag id, or whether we want to mess with adding
> > > > yet-another-field-to-indexes-that-are-already-too-long-for-mysql and it
> > > was
> > > > based on the assumption we are going to that for versioning already,
> > so I
> > > > thought that is the plan :). Main reason why I put the multi-team
> > voting
> > > on
> > > > hold while we decide on the scope of Airflow 3 to be honest.
> > > >
> > > >
> > > >
> > >
> > https://apache-airflow.slack.com/archives/C06K9Q5G2UA/p1714140873323569?thread_ts=1713607491.495499&cid=C06K9Q5G2UA
> > > >
> > > > J.
> > > >
> > > >
> > > > On Fri, May 31, 2024 at 9:04 AM Daniel Standish
> > > > <da...@astronomer.io.invalid> wrote:
> > > >
> > > >> Yes uuid is risky and problematic as primary key. If you do it you
> > need
> > > to
> > > >> do carefully/ sequential.  But I think that we are not going with UUID
> > > pk
> > > >> on any tables at this time.
> > > >>
> > > >> BUT I do want to add a uuid for every TI try that is not PK but can be
> > > >> used
> > > >> as a more convenient identifier when tying together log events related
> > > to
> > > >> TIs emitted on different services.
> > > >>
> > > >> On Thu, May 30, 2024 at 11:53 PM Jarek Potiuk <ja...@potiuk.com>
> > wrote:
> > > >>
> > > >> > Also if we are speaking about indexes - a bit tangential but I know
> > we
> > > >> were
> > > >> > planning to replace some of the primary keys (mainly because of
> > mysql
> > > >> > limitations) with synthetic keys for DAG versioning casse where we
> > > >> planned
> > > >> > to use UUIDs).
> > > >> >
> > > >> > We should be very, very careful when doing it because I've learned
> > > >> recently
> > > >> > that due to UUID randomness the algorithm to rebalance binary tree
> > do
> > > >> not
> > > >> > work well with random UUIDs which means that when you do a lot of
> > > >> inserts,
> > > >> > rebalancing of the trees is happening very often and performance of
> > > >> inserts
> > > >> > is heavily affected.  Postgres has a dedicated UUID field type for
> > > that
> > > >> > that produces an optimal index
> > > >> >
> > > >> >
> > > >>
> > >
> > https://dba.stackexchange.com/questions/102448/how-should-i-index-a-uuid-in-postgres
> > > >> > - however it might or might not be affected. Similarly MariaDB, but
> > > >> MySQL
> > > >> > does not seem to have proper UUID support, so we should really use
> > > UUID7
> > > >> > rather that UUID4 for such UUIDs in case we do not want to affect
> > > insert
> > > >> > performance on MySQL.
> > > >> >
> > > >> > WAT???? Yeah. That is. - I think - rather non-obvious side-effect.
> > > >> >
> > > >> > This learning I had adds much more strength to two related
> > > discussions:
> > > >> >
> > > >> > 1) This only adds more fire to "let's only stick to Postgres". We
> > can
> > > >> > optimize the experience for our users way better and move faster.
> > > >> >
> > > >> > 2) Let's REALLY not put the power of deciding what indexes to put on
> > > the
> > > >> > database of ours in the hands of our users. There are many unobvious
> > > >> > consequences of adding indexes and while we can aim to learn,
> > foresee
> > > >> and
> > > >> > control them (mostly when we have a working performance test suite),
> > > >> > getting a random user to put a random index on a random set of keys
> > in
> > > >> > Airflow DB might have a number of unobvious consequences which might
> > > >> give
> > > >> > our users (and us) a LOT of headache.
> > > >> >
> > > >> > J.
> > > >> >
> > > >> >
> > > >> >
> > > >> >
> > > >> > On Fri, May 31, 2024 at 8:13 AM Pankaj Koti
> > > >> > <pa...@astronomer.io.invalid> wrote:
> > > >> >
> > > >> > > Addressing one of Pierre's questions:
> > > >> > >
> > > >> > > Should I index foreign keys? Is that done by default or should I
> > > >> > explicitly
> > > >> > > do it?
> > > >> > >
> > > >> > > The answer varies depending on the database engine.
> > > >> > > PostgreSQL and SQLite do not add indexes on foreign keys
> > > >> > > by default, while MySQL does. Developers should keep this
> > > >> > > in mind. Many of us (myself included) may have assumed
> > > >> > > from our university days, indexes are always created for
> > > >> > > foreign keys, but that's not always the case. :)
> > > >> > >
> > > >> > > Best regards,
> > > >> > >
> > > >> > > *Pankaj Koti*
> > > >> > > Senior Software Engineer (Airflow OSS Engineering team)
> > > >> > > Location: Pune, Maharashtra, India
> > > >> > > Timezone: Indian Standard Time (IST)
> > > >> > >
> > > >> > >
> > > >> > > On Fri, May 31, 2024 at 11:37 AM Daniel Standish
> > > >> > > <da...@astronomer.io.invalid> wrote:
> > > >> > >
> > > >> > > > I would be in favor of this for sure.  Let's see what others
> > think
> > > >> :)
> > > >> > > >
> > > >> > > > On Thu, May 30, 2024 at 10:55 PM Jarek Potiuk <jarek@potiuk.com
> > >
> > > >> > wrote:
> > > >> > > >
> > > >> > > > > Simply speaking - let's make "lack of optimisation for these
> > and
> > > >> > that"
> > > >> > > > part
> > > >> > > > > of the API specification.
> > > >> > > > >
> > > >> > > > > On Fri, May 31, 2024 at 7:54 AM Jarek Potiuk <
> > jarek@potiuk.com>
> > > >> > wrote:
> > > >> > > > >
> > > >> > > > > > So let's document as part of the API which queries are not
> > > >> > performant
> > > >> > > > and
> > > >> > > > > > suggest users that want to use them to make their analytics
> > > >> queries
> > > >> > > > > > elsewhere. I'd very much prefer that it's slow "by design"
> > for
> > > >> > > everyone
> > > >> > > > > > rather than add option for the user to speed them up where
> > we
> > > >> > decided
> > > >> > > > > > not to do it ourselves because we know the consequences.
> > > >> > > > > >
> > > >> > > > > > I think the root cause of the problem is `US` adding new
> > > >> filtering
> > > >> > > > > options
> > > >> > > > > > to public API without thinking about and documenting
> > > >> consequences.
> > > >> > > > > >
> > > >> > > > > > J.
> > > >> > > > > >
> > > >> > > > > >
> > > >> > > > > > On Fri, May 31, 2024 at 7:38 AM Daniel Standish
> > > >> > > > > > <da...@astronomer.io.invalid> wrote:
> > > >> > > > > >
> > > >> > > > > >> So I think the notion that *all possibly expensive queries*
> > > >> should
> > > >> > > > have
> > > >> > > > > an
> > > >> > > > > >> index to support them is not a tenable one.  E.g. there are
> > > >> > > something
> > > >> > > > > like
> > > >> > > > > >> 5 params on TI list endpoint that don't have an index.
> > > >> > > > > >>
> > > >> > > > > >> In contrast with queries from airflow itself, the API
> > queries
> > > >> are
> > > >> > > more
> > > >> > > > > >> arbitrary -- user can combine in any way.  So it's not so
> > > >> simple
> > > >> > to
> > > >> > > > > >> determine just from the endpoint params what indexes will
> > be
> > > >> > needed.
> > > >> > > > > And
> > > >> > > > > >> we're not going to add an index for all possible param
> > > >> > combinations,
> > > >> > > > > that
> > > >> > > > > >> would be silly.  So I think like it or not I think we will
> > > >> > > necessarily
> > > >> > > > > >> draw
> > > >> > > > > >> the line at *some* point and say we're trying to cover the
> > > most
> > > >> > > > likely /
> > > >> > > > > >> normal / reasonable uses and if your usage pattern is an
> > > >> outlier
> > > >> > you
> > > >> > > > > might
> > > >> > > > > >> need to add an index yourself.  And I don't think that's
> > > >> > > unreasonable
> > > >> > > > > >>
> > > >> > > > > >> Maybe at least one conclusion from this is to add an index
> > > for
> > > >> > each
> > > >> > > of
> > > >> > > > > >> start_date, end_date, and updated_at on TI.  Already the
> > > >> storage
> > > >> > > > > occupied
> > > >> > > > > >> by indexes on TI is more than the table data size -- I
> > don't
> > > >> > > remember
> > > >> > > > it
> > > >> > > > > >> might have been like 2x or something.  So then index count
> > > will
> > > >> > > > increase
> > > >> > > > > >> from 8 to 11 and overhead increases.  Whether this is a
> > > >> material
> > > >> > > > > concern,
> > > >> > > > > >> I
> > > >> > > > > >> am honestly not sure -- I am not a db specialist.  But it
> > > does
> > > >> > feel
> > > >> > > > icky
> > > >> > > > > >> to
> > > >> > > > > >> add all the indexes when they are used by, I suspect, a
> > very
> > > >> small
> > > >> > > > > >> percentage of airflow users overall.
> > > >> > > > > >>
> > > >> > > > > >> Part of this for me is also the reason these types of
> > queries
> > > >> are
> > > >> > > run.
> > > >> > > > > >> Typically it is for analytics purposes, either on the fly
> > or
> > > as
> > > >> > part
> > > >> > > > of
> > > >> > > > > a
> > > >> > > > > >> replication process to a DWH or something.  But there are
> > > more
> > > >> > > > efficient
> > > >> > > > > >> mechanisms for this than pulling the data through the
> > > >> webserver,
> > > >> > > e.g.
> > > >> > > > > >> read-only replicas, or standard ETL processes, or just
> > making
> > > >> > better
> > > >> > > > use
> > > >> > > > > >> of
> > > >> > > > > >> the indexes that are already there.  Do we consider
> > > >> "replication"
> > > >> > a
> > > >> > > > > >> reasonable use of the REST API?
> > > >> > > > > >>
> > > >> > > > > >> So I may very well be alone on this. But (1) the API is not
> > > >> > > something
> > > >> > > > > >> everyone uses.  And (2) even of users of the API, probably
> > > >> only a
> > > >> > > very
> > > >> > > > > >> small percentage are using it in a way that needs these
> > > >> indexes --
> > > >> > > > > >> otherwise why haven't we seen requests for indexes on these
> > > >> > columns
> > > >> > > on
> > > >> > > > > >> github?  So, to me, it doesn't seem crazy to just leave it
> > to
> > > >> the
> > > >> > > > user /
> > > >> > > > > >> platform to add.  But as usual, not a hill to die on.
> > > >> > > > > >>
> > > >> > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >> >
> > > >>
> > > >
> > >
> >
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@airflow.apache.org
For additional commands, e-mail: dev-help@airflow.apache.org

Re: [DISCUSS] indexes for API calls

[Andrey Anshin <an...@taragol.is>]

IMHO, blindy adding new indexes into the `dag_run` and `task_instance`
tables will cause additional maintenance costs.

There are 8 indexes already exists per each of this tables

SELECT
    pi.schemaname schema_name,
    pi.tablename table_name,
    count(*) num
FROM pg_indexes pi
WHERE pi.schemaname = 'public'
GROUP BY 1, 2
ORDER BY 3 DESC, 2;

And maintenance indexes overall it is not something cheap, every time when
the row changed/added (INSERT/UPDATE/DELETE/MERGE/COPY) there is a check
whether or not it is required to rebalance index tree (B-Tree indexes).

If we would like to add individual indexes for one field we bump into the
problem that this are less effective when it comes to a filter/sorting over
multiple different fields (include PK), just because this action required
to create Bitmap Scan into the memory (Working Memory in postgres, default
something like 10 MiB), and in the end COB (Cost Base Optimisation)
mechanism more like decide to do sequential scan, as result indexes become
useless, and just use additional disc space and CPU for maintenance.

Rather than add these indexes (and every other "performance optimisation"
measures) we should prove that this action gains benefits, rather than the
opposite. That might be achieved by enable on some prod-like environment
logs collection and analyze logs by pgBadger (
https://github.com/darold/pgbadger), that might shows different metrics
before changes and after the changes

This all valid for Postgres, and I have no idea how to calculate measures
for MySQL, I hope we will reach an agreement to drop MySQL in Airflow 3.

On Fri, 31 May 2024 at 12:10, Pierre Jeambrun <pi...@gmail.com> wrote:

> Indeed Jarek I feel like this is another point in favor of stick to
> "Postgres"
>
> As mentioned, maybe we were a little reckless when adding all these kinds
> of filters. If they are not often used and we rarely / never see
> performance github issues on those, marking them as 'non optimised but here
> for convenience' could be a good short term solution. (And maybe removing
> those filters at some point)
>
> What people use the API for (replicating the DB, building an UI, etc...) do
> not really matter I think, Ideally I just think that the API should have a
> sort of SLA and guarantee a reasonable response time for all endpoints
> (through pagination, indexes, etc...). We can still document that some
> endpoints are known to be slower for some reason and leave them out of this
> performance constraint. (Having performance tests could be great otherwise
> it will be very hard to track).
>
> Also just to be sure, the slowness you observe for listing task instances
> only appears with a certain combination of filters, while some other
> combinations, yielding a comparable result set in terms of 'size' are much
> quicker ? (Just trying to understand if the slowness comes from missing
> index, or also possibly from missing loading options that would cause lazy
> loading at serialization time slowing everything down, I don't see a lot of
> eager loading to prevent those, but maybe the few that we have are enough).
>
> Le ven. 31 mai 2024 à 09:38, Jarek Potiuk <ja...@potiuk.com> a écrit :
>
> > And to be perfectly honest -  if people (like me) hesitate on settling on
> > architectural decisions because they are afraid that their changes might
> > have unintended consequences, because we want to support all the
> different
> > kinds of databases - this is one more reason we should stick to "Postgres
> > only".  This is a very good example where we have real slow-downs that
> > impact the speed the decisions are made (which impacts speed of feature
> > development).
> >
> > On Fri, May 31, 2024 at 9:13 AM Jarek Potiuk <ja...@potiuk.com> wrote:
> >
> > > Using UUIDS was the proposal how we can bypass the limitation of MYSQL
> > for
> > > Airflow 2 when we discussed whether to do a "simple" version of
> > team-prefix
> > > in dag id, or whether we want to mess with adding
> > > yet-another-field-to-indexes-that-are-already-too-long-for-mysql and it
> > was
> > > based on the assumption we are going to that for versioning already,
> so I
> > > thought that is the plan :). Main reason why I put the multi-team
> voting
> > on
> > > hold while we decide on the scope of Airflow 3 to be honest.
> > >
> > >
> > >
> >
> https://apache-airflow.slack.com/archives/C06K9Q5G2UA/p1714140873323569?thread_ts=1713607491.495499&cid=C06K9Q5G2UA
> > >
> > > J.
> > >
> > >
> > > On Fri, May 31, 2024 at 9:04 AM Daniel Standish
> > > <da...@astronomer.io.invalid> wrote:
> > >
> > >> Yes uuid is risky and problematic as primary key. If you do it you
> need
> > to
> > >> do carefully/ sequential.  But I think that we are not going with UUID
> > pk
> > >> on any tables at this time.
> > >>
> > >> BUT I do want to add a uuid for every TI try that is not PK but can be
> > >> used
> > >> as a more convenient identifier when tying together log events related
> > to
> > >> TIs emitted on different services.
> > >>
> > >> On Thu, May 30, 2024 at 11:53 PM Jarek Potiuk <ja...@potiuk.com>
> wrote:
> > >>
> > >> > Also if we are speaking about indexes - a bit tangential but I know
> we
> > >> were
> > >> > planning to replace some of the primary keys (mainly because of
> mysql
> > >> > limitations) with synthetic keys for DAG versioning casse where we
> > >> planned
> > >> > to use UUIDs).
> > >> >
> > >> > We should be very, very careful when doing it because I've learned
> > >> recently
> > >> > that due to UUID randomness the algorithm to rebalance binary tree
> do
> > >> not
> > >> > work well with random UUIDs which means that when you do a lot of
> > >> inserts,
> > >> > rebalancing of the trees is happening very often and performance of
> > >> inserts
> > >> > is heavily affected.  Postgres has a dedicated UUID field type for
> > that
> > >> > that produces an optimal index
> > >> >
> > >> >
> > >>
> >
> https://dba.stackexchange.com/questions/102448/how-should-i-index-a-uuid-in-postgres
> > >> > - however it might or might not be affected. Similarly MariaDB, but
> > >> MySQL
> > >> > does not seem to have proper UUID support, so we should really use
> > UUID7
> > >> > rather that UUID4 for such UUIDs in case we do not want to affect
> > insert
> > >> > performance on MySQL.
> > >> >
> > >> > WAT???? Yeah. That is. - I think - rather non-obvious side-effect.
> > >> >
> > >> > This learning I had adds much more strength to two related
> > discussions:
> > >> >
> > >> > 1) This only adds more fire to "let's only stick to Postgres". We
> can
> > >> > optimize the experience for our users way better and move faster.
> > >> >
> > >> > 2) Let's REALLY not put the power of deciding what indexes to put on
> > the
> > >> > database of ours in the hands of our users. There are many unobvious
> > >> > consequences of adding indexes and while we can aim to learn,
> foresee
> > >> and
> > >> > control them (mostly when we have a working performance test suite),
> > >> > getting a random user to put a random index on a random set of keys
> in
> > >> > Airflow DB might have a number of unobvious consequences which might
> > >> give
> > >> > our users (and us) a LOT of headache.
> > >> >
> > >> > J.
> > >> >
> > >> >
> > >> >
> > >> >
> > >> > On Fri, May 31, 2024 at 8:13 AM Pankaj Koti
> > >> > <pa...@astronomer.io.invalid> wrote:
> > >> >
> > >> > > Addressing one of Pierre's questions:
> > >> > >
> > >> > > Should I index foreign keys? Is that done by default or should I
> > >> > explicitly
> > >> > > do it?
> > >> > >
> > >> > > The answer varies depending on the database engine.
> > >> > > PostgreSQL and SQLite do not add indexes on foreign keys
> > >> > > by default, while MySQL does. Developers should keep this
> > >> > > in mind. Many of us (myself included) may have assumed
> > >> > > from our university days, indexes are always created for
> > >> > > foreign keys, but that's not always the case. :)
> > >> > >
> > >> > > Best regards,
> > >> > >
> > >> > > *Pankaj Koti*
> > >> > > Senior Software Engineer (Airflow OSS Engineering team)
> > >> > > Location: Pune, Maharashtra, India
> > >> > > Timezone: Indian Standard Time (IST)
> > >> > >
> > >> > >
> > >> > > On Fri, May 31, 2024 at 11:37 AM Daniel Standish
> > >> > > <da...@astronomer.io.invalid> wrote:
> > >> > >
> > >> > > > I would be in favor of this for sure.  Let's see what others
> think
> > >> :)
> > >> > > >
> > >> > > > On Thu, May 30, 2024 at 10:55 PM Jarek Potiuk <jarek@potiuk.com
> >
> > >> > wrote:
> > >> > > >
> > >> > > > > Simply speaking - let's make "lack of optimisation for these
> and
> > >> > that"
> > >> > > > part
> > >> > > > > of the API specification.
> > >> > > > >
> > >> > > > > On Fri, May 31, 2024 at 7:54 AM Jarek Potiuk <
> jarek@potiuk.com>
> > >> > wrote:
> > >> > > > >
> > >> > > > > > So let's document as part of the API which queries are not
> > >> > performant
> > >> > > > and
> > >> > > > > > suggest users that want to use them to make their analytics
> > >> queries
> > >> > > > > > elsewhere. I'd very much prefer that it's slow "by design"
> for
> > >> > > everyone
> > >> > > > > > rather than add option for the user to speed them up where
> we
> > >> > decided
> > >> > > > > > not to do it ourselves because we know the consequences.
> > >> > > > > >
> > >> > > > > > I think the root cause of the problem is `US` adding new
> > >> filtering
> > >> > > > > options
> > >> > > > > > to public API without thinking about and documenting
> > >> consequences.
> > >> > > > > >
> > >> > > > > > J.
> > >> > > > > >
> > >> > > > > >
> > >> > > > > > On Fri, May 31, 2024 at 7:38 AM Daniel Standish
> > >> > > > > > <da...@astronomer.io.invalid> wrote:
> > >> > > > > >
> > >> > > > > >> So I think the notion that *all possibly expensive queries*
> > >> should
> > >> > > > have
> > >> > > > > an
> > >> > > > > >> index to support them is not a tenable one.  E.g. there are
> > >> > > something
> > >> > > > > like
> > >> > > > > >> 5 params on TI list endpoint that don't have an index.
> > >> > > > > >>
> > >> > > > > >> In contrast with queries from airflow itself, the API
> queries
> > >> are
> > >> > > more
> > >> > > > > >> arbitrary -- user can combine in any way.  So it's not so
> > >> simple
> > >> > to
> > >> > > > > >> determine just from the endpoint params what indexes will
> be
> > >> > needed.
> > >> > > > > And
> > >> > > > > >> we're not going to add an index for all possible param
> > >> > combinations,
> > >> > > > > that
> > >> > > > > >> would be silly.  So I think like it or not I think we will
> > >> > > necessarily
> > >> > > > > >> draw
> > >> > > > > >> the line at *some* point and say we're trying to cover the
> > most
> > >> > > > likely /
> > >> > > > > >> normal / reasonable uses and if your usage pattern is an
> > >> outlier
> > >> > you
> > >> > > > > might
> > >> > > > > >> need to add an index yourself.  And I don't think that's
> > >> > > unreasonable
> > >> > > > > >>
> > >> > > > > >> Maybe at least one conclusion from this is to add an index
> > for
> > >> > each
> > >> > > of
> > >> > > > > >> start_date, end_date, and updated_at on TI.  Already the
> > >> storage
> > >> > > > > occupied
> > >> > > > > >> by indexes on TI is more than the table data size -- I
> don't
> > >> > > remember
> > >> > > > it
> > >> > > > > >> might have been like 2x or something.  So then index count
> > will
> > >> > > > increase
> > >> > > > > >> from 8 to 11 and overhead increases.  Whether this is a
> > >> material
> > >> > > > > concern,
> > >> > > > > >> I
> > >> > > > > >> am honestly not sure -- I am not a db specialist.  But it
> > does
> > >> > feel
> > >> > > > icky
> > >> > > > > >> to
> > >> > > > > >> add all the indexes when they are used by, I suspect, a
> very
> > >> small
> > >> > > > > >> percentage of airflow users overall.
> > >> > > > > >>
> > >> > > > > >> Part of this for me is also the reason these types of
> queries
> > >> are
> > >> > > run.
> > >> > > > > >> Typically it is for analytics purposes, either on the fly
> or
> > as
> > >> > part
> > >> > > > of
> > >> > > > > a
> > >> > > > > >> replication process to a DWH or something.  But there are
> > more
> > >> > > > efficient
> > >> > > > > >> mechanisms for this than pulling the data through the
> > >> webserver,
> > >> > > e.g.
> > >> > > > > >> read-only replicas, or standard ETL processes, or just
> making
> > >> > better
> > >> > > > use
> > >> > > > > >> of
> > >> > > > > >> the indexes that are already there.  Do we consider
> > >> "replication"
> > >> > a
> > >> > > > > >> reasonable use of the REST API?
> > >> > > > > >>
> > >> > > > > >> So I may very well be alone on this. But (1) the API is not
> > >> > > something
> > >> > > > > >> everyone uses.  And (2) even of users of the API, probably
> > >> only a
> > >> > > very
> > >> > > > > >> small percentage are using it in a way that needs these
> > >> indexes --
> > >> > > > > >> otherwise why haven't we seen requests for indexes on these
> > >> > columns
> > >> > > on
> > >> > > > > >> github?  So, to me, it doesn't seem crazy to just leave it
> to
> > >> the
> > >> > > > user /
> > >> > > > > >> platform to add.  But as usual, not a hill to die on.
> > >> > > > > >>
> > >> > > > > >
> > >> > > > >
> > >> > > >
> > >> > >
> > >> >
> > >>
> > >
> >
>

Re: [DISCUSS] indexes for API calls

[Pierre Jeambrun <pi...@gmail.com>]

Indeed Jarek I feel like this is another point in favor of stick to
"Postgres"

As mentioned, maybe we were a little reckless when adding all these kinds
of filters. If they are not often used and we rarely / never see
performance github issues on those, marking them as 'non optimised but here
for convenience' could be a good short term solution. (And maybe removing
those filters at some point)

What people use the API for (replicating the DB, building an UI, etc...) do
not really matter I think, Ideally I just think that the API should have a
sort of SLA and guarantee a reasonable response time for all endpoints
(through pagination, indexes, etc...). We can still document that some
endpoints are known to be slower for some reason and leave them out of this
performance constraint. (Having performance tests could be great otherwise
it will be very hard to track).

Also just to be sure, the slowness you observe for listing task instances
only appears with a certain combination of filters, while some other
combinations, yielding a comparable result set in terms of 'size' are much
quicker ? (Just trying to understand if the slowness comes from missing
index, or also possibly from missing loading options that would cause lazy
loading at serialization time slowing everything down, I don't see a lot of
eager loading to prevent those, but maybe the few that we have are enough).

Le ven. 31 mai 2024 à 09:38, Jarek Potiuk <ja...@potiuk.com> a écrit :

> And to be perfectly honest -  if people (like me) hesitate on settling on
> architectural decisions because they are afraid that their changes might
> have unintended consequences, because we want to support all the different
> kinds of databases - this is one more reason we should stick to "Postgres
> only".  This is a very good example where we have real slow-downs that
> impact the speed the decisions are made (which impacts speed of feature
> development).
>
> On Fri, May 31, 2024 at 9:13 AM Jarek Potiuk <ja...@potiuk.com> wrote:
>
> > Using UUIDS was the proposal how we can bypass the limitation of MYSQL
> for
> > Airflow 2 when we discussed whether to do a "simple" version of
> team-prefix
> > in dag id, or whether we want to mess with adding
> > yet-another-field-to-indexes-that-are-already-too-long-for-mysql and it
> was
> > based on the assumption we are going to that for versioning already, so I
> > thought that is the plan :). Main reason why I put the multi-team voting
> on
> > hold while we decide on the scope of Airflow 3 to be honest.
> >
> >
> >
> https://apache-airflow.slack.com/archives/C06K9Q5G2UA/p1714140873323569?thread_ts=1713607491.495499&cid=C06K9Q5G2UA
> >
> > J.
> >
> >
> > On Fri, May 31, 2024 at 9:04 AM Daniel Standish
> > <da...@astronomer.io.invalid> wrote:
> >
> >> Yes uuid is risky and problematic as primary key. If you do it you need
> to
> >> do carefully/ sequential.  But I think that we are not going with UUID
> pk
> >> on any tables at this time.
> >>
> >> BUT I do want to add a uuid for every TI try that is not PK but can be
> >> used
> >> as a more convenient identifier when tying together log events related
> to
> >> TIs emitted on different services.
> >>
> >> On Thu, May 30, 2024 at 11:53 PM Jarek Potiuk <ja...@potiuk.com> wrote:
> >>
> >> > Also if we are speaking about indexes - a bit tangential but I know we
> >> were
> >> > planning to replace some of the primary keys (mainly because of mysql
> >> > limitations) with synthetic keys for DAG versioning casse where we
> >> planned
> >> > to use UUIDs).
> >> >
> >> > We should be very, very careful when doing it because I've learned
> >> recently
> >> > that due to UUID randomness the algorithm to rebalance binary tree do
> >> not
> >> > work well with random UUIDs which means that when you do a lot of
> >> inserts,
> >> > rebalancing of the trees is happening very often and performance of
> >> inserts
> >> > is heavily affected.  Postgres has a dedicated UUID field type for
> that
> >> > that produces an optimal index
> >> >
> >> >
> >>
> https://dba.stackexchange.com/questions/102448/how-should-i-index-a-uuid-in-postgres
> >> > - however it might or might not be affected. Similarly MariaDB, but
> >> MySQL
> >> > does not seem to have proper UUID support, so we should really use
> UUID7
> >> > rather that UUID4 for such UUIDs in case we do not want to affect
> insert
> >> > performance on MySQL.
> >> >
> >> > WAT???? Yeah. That is. - I think - rather non-obvious side-effect.
> >> >
> >> > This learning I had adds much more strength to two related
> discussions:
> >> >
> >> > 1) This only adds more fire to "let's only stick to Postgres". We can
> >> > optimize the experience for our users way better and move faster.
> >> >
> >> > 2) Let's REALLY not put the power of deciding what indexes to put on
> the
> >> > database of ours in the hands of our users. There are many unobvious
> >> > consequences of adding indexes and while we can aim to learn, foresee
> >> and
> >> > control them (mostly when we have a working performance test suite),
> >> > getting a random user to put a random index on a random set of keys in
> >> > Airflow DB might have a number of unobvious consequences which might
> >> give
> >> > our users (and us) a LOT of headache.
> >> >
> >> > J.
> >> >
> >> >
> >> >
> >> >
> >> > On Fri, May 31, 2024 at 8:13 AM Pankaj Koti
> >> > <pa...@astronomer.io.invalid> wrote:
> >> >
> >> > > Addressing one of Pierre's questions:
> >> > >
> >> > > Should I index foreign keys? Is that done by default or should I
> >> > explicitly
> >> > > do it?
> >> > >
> >> > > The answer varies depending on the database engine.
> >> > > PostgreSQL and SQLite do not add indexes on foreign keys
> >> > > by default, while MySQL does. Developers should keep this
> >> > > in mind. Many of us (myself included) may have assumed
> >> > > from our university days, indexes are always created for
> >> > > foreign keys, but that's not always the case. :)
> >> > >
> >> > > Best regards,
> >> > >
> >> > > *Pankaj Koti*
> >> > > Senior Software Engineer (Airflow OSS Engineering team)
> >> > > Location: Pune, Maharashtra, India
> >> > > Timezone: Indian Standard Time (IST)
> >> > >
> >> > >
> >> > > On Fri, May 31, 2024 at 11:37 AM Daniel Standish
> >> > > <da...@astronomer.io.invalid> wrote:
> >> > >
> >> > > > I would be in favor of this for sure.  Let's see what others think
> >> :)
> >> > > >
> >> > > > On Thu, May 30, 2024 at 10:55 PM Jarek Potiuk <ja...@potiuk.com>
> >> > wrote:
> >> > > >
> >> > > > > Simply speaking - let's make "lack of optimisation for these and
> >> > that"
> >> > > > part
> >> > > > > of the API specification.
> >> > > > >
> >> > > > > On Fri, May 31, 2024 at 7:54 AM Jarek Potiuk <ja...@potiuk.com>
> >> > wrote:
> >> > > > >
> >> > > > > > So let's document as part of the API which queries are not
> >> > performant
> >> > > > and
> >> > > > > > suggest users that want to use them to make their analytics
> >> queries
> >> > > > > > elsewhere. I'd very much prefer that it's slow "by design" for
> >> > > everyone
> >> > > > > > rather than add option for the user to speed them up where we
> >> > decided
> >> > > > > > not to do it ourselves because we know the consequences.
> >> > > > > >
> >> > > > > > I think the root cause of the problem is `US` adding new
> >> filtering
> >> > > > > options
> >> > > > > > to public API without thinking about and documenting
> >> consequences.
> >> > > > > >
> >> > > > > > J.
> >> > > > > >
> >> > > > > >
> >> > > > > > On Fri, May 31, 2024 at 7:38 AM Daniel Standish
> >> > > > > > <da...@astronomer.io.invalid> wrote:
> >> > > > > >
> >> > > > > >> So I think the notion that *all possibly expensive queries*
> >> should
> >> > > > have
> >> > > > > an
> >> > > > > >> index to support them is not a tenable one.  E.g. there are
> >> > > something
> >> > > > > like
> >> > > > > >> 5 params on TI list endpoint that don't have an index.
> >> > > > > >>
> >> > > > > >> In contrast with queries from airflow itself, the API queries
> >> are
> >> > > more
> >> > > > > >> arbitrary -- user can combine in any way.  So it's not so
> >> simple
> >> > to
> >> > > > > >> determine just from the endpoint params what indexes will be
> >> > needed.
> >> > > > > And
> >> > > > > >> we're not going to add an index for all possible param
> >> > combinations,
> >> > > > > that
> >> > > > > >> would be silly.  So I think like it or not I think we will
> >> > > necessarily
> >> > > > > >> draw
> >> > > > > >> the line at *some* point and say we're trying to cover the
> most
> >> > > > likely /
> >> > > > > >> normal / reasonable uses and if your usage pattern is an
> >> outlier
> >> > you
> >> > > > > might
> >> > > > > >> need to add an index yourself.  And I don't think that's
> >> > > unreasonable
> >> > > > > >>
> >> > > > > >> Maybe at least one conclusion from this is to add an index
> for
> >> > each
> >> > > of
> >> > > > > >> start_date, end_date, and updated_at on TI.  Already the
> >> storage
> >> > > > > occupied
> >> > > > > >> by indexes on TI is more than the table data size -- I don't
> >> > > remember
> >> > > > it
> >> > > > > >> might have been like 2x or something.  So then index count
> will
> >> > > > increase
> >> > > > > >> from 8 to 11 and overhead increases.  Whether this is a
> >> material
> >> > > > > concern,
> >> > > > > >> I
> >> > > > > >> am honestly not sure -- I am not a db specialist.  But it
> does
> >> > feel
> >> > > > icky
> >> > > > > >> to
> >> > > > > >> add all the indexes when they are used by, I suspect, a very
> >> small
> >> > > > > >> percentage of airflow users overall.
> >> > > > > >>
> >> > > > > >> Part of this for me is also the reason these types of queries
> >> are
> >> > > run.
> >> > > > > >> Typically it is for analytics purposes, either on the fly or
> as
> >> > part
> >> > > > of
> >> > > > > a
> >> > > > > >> replication process to a DWH or something.  But there are
> more
> >> > > > efficient
> >> > > > > >> mechanisms for this than pulling the data through the
> >> webserver,
> >> > > e.g.
> >> > > > > >> read-only replicas, or standard ETL processes, or just making
> >> > better
> >> > > > use
> >> > > > > >> of
> >> > > > > >> the indexes that are already there.  Do we consider
> >> "replication"
> >> > a
> >> > > > > >> reasonable use of the REST API?
> >> > > > > >>
> >> > > > > >> So I may very well be alone on this. But (1) the API is not
> >> > > something
> >> > > > > >> everyone uses.  And (2) even of users of the API, probably
> >> only a
> >> > > very
> >> > > > > >> small percentage are using it in a way that needs these
> >> indexes --
> >> > > > > >> otherwise why haven't we seen requests for indexes on these
> >> > columns
> >> > > on
> >> > > > > >> github?  So, to me, it doesn't seem crazy to just leave it to
> >> the
> >> > > > user /
> >> > > > > >> platform to add.  But as usual, not a hill to die on.
> >> > > > > >>
> >> > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> >
>

Re: [DISCUSS] indexes for API calls

[Jarek Potiuk <ja...@potiuk.com>]

And to be perfectly honest -  if people (like me) hesitate on settling on
architectural decisions because they are afraid that their changes might
have unintended consequences, because we want to support all the different
kinds of databases - this is one more reason we should stick to "Postgres
only".  This is a very good example where we have real slow-downs that
impact the speed the decisions are made (which impacts speed of feature
development).

On Fri, May 31, 2024 at 9:13 AM Jarek Potiuk <ja...@potiuk.com> wrote:

> Using UUIDS was the proposal how we can bypass the limitation of MYSQL for
> Airflow 2 when we discussed whether to do a "simple" version of team-prefix
> in dag id, or whether we want to mess with adding
> yet-another-field-to-indexes-that-are-already-too-long-for-mysql and it was
> based on the assumption we are going to that for versioning already, so I
> thought that is the plan :). Main reason why I put the multi-team voting on
> hold while we decide on the scope of Airflow 3 to be honest.
>
>
> https://apache-airflow.slack.com/archives/C06K9Q5G2UA/p1714140873323569?thread_ts=1713607491.495499&cid=C06K9Q5G2UA
>
> J.
>
>
> On Fri, May 31, 2024 at 9:04 AM Daniel Standish
> <da...@astronomer.io.invalid> wrote:
>
>> Yes uuid is risky and problematic as primary key. If you do it you need to
>> do carefully/ sequential.  But I think that we are not going with UUID pk
>> on any tables at this time.
>>
>> BUT I do want to add a uuid for every TI try that is not PK but can be
>> used
>> as a more convenient identifier when tying together log events related to
>> TIs emitted on different services.
>>
>> On Thu, May 30, 2024 at 11:53 PM Jarek Potiuk <ja...@potiuk.com> wrote:
>>
>> > Also if we are speaking about indexes - a bit tangential but I know we
>> were
>> > planning to replace some of the primary keys (mainly because of mysql
>> > limitations) with synthetic keys for DAG versioning casse where we
>> planned
>> > to use UUIDs).
>> >
>> > We should be very, very careful when doing it because I've learned
>> recently
>> > that due to UUID randomness the algorithm to rebalance binary tree do
>> not
>> > work well with random UUIDs which means that when you do a lot of
>> inserts,
>> > rebalancing of the trees is happening very often and performance of
>> inserts
>> > is heavily affected.  Postgres has a dedicated UUID field type for that
>> > that produces an optimal index
>> >
>> >
>> https://dba.stackexchange.com/questions/102448/how-should-i-index-a-uuid-in-postgres
>> > - however it might or might not be affected. Similarly MariaDB, but
>> MySQL
>> > does not seem to have proper UUID support, so we should really use UUID7
>> > rather that UUID4 for such UUIDs in case we do not want to affect insert
>> > performance on MySQL.
>> >
>> > WAT???? Yeah. That is. - I think - rather non-obvious side-effect.
>> >
>> > This learning I had adds much more strength to two related discussions:
>> >
>> > 1) This only adds more fire to "let's only stick to Postgres". We can
>> > optimize the experience for our users way better and move faster.
>> >
>> > 2) Let's REALLY not put the power of deciding what indexes to put on the
>> > database of ours in the hands of our users. There are many unobvious
>> > consequences of adding indexes and while we can aim to learn, foresee
>> and
>> > control them (mostly when we have a working performance test suite),
>> > getting a random user to put a random index on a random set of keys in
>> > Airflow DB might have a number of unobvious consequences which might
>> give
>> > our users (and us) a LOT of headache.
>> >
>> > J.
>> >
>> >
>> >
>> >
>> > On Fri, May 31, 2024 at 8:13 AM Pankaj Koti
>> > <pa...@astronomer.io.invalid> wrote:
>> >
>> > > Addressing one of Pierre's questions:
>> > >
>> > > Should I index foreign keys? Is that done by default or should I
>> > explicitly
>> > > do it?
>> > >
>> > > The answer varies depending on the database engine.
>> > > PostgreSQL and SQLite do not add indexes on foreign keys
>> > > by default, while MySQL does. Developers should keep this
>> > > in mind. Many of us (myself included) may have assumed
>> > > from our university days, indexes are always created for
>> > > foreign keys, but that's not always the case. :)
>> > >
>> > > Best regards,
>> > >
>> > > *Pankaj Koti*
>> > > Senior Software Engineer (Airflow OSS Engineering team)
>> > > Location: Pune, Maharashtra, India
>> > > Timezone: Indian Standard Time (IST)
>> > >
>> > >
>> > > On Fri, May 31, 2024 at 11:37 AM Daniel Standish
>> > > <da...@astronomer.io.invalid> wrote:
>> > >
>> > > > I would be in favor of this for sure.  Let's see what others think
>> :)
>> > > >
>> > > > On Thu, May 30, 2024 at 10:55 PM Jarek Potiuk <ja...@potiuk.com>
>> > wrote:
>> > > >
>> > > > > Simply speaking - let's make "lack of optimisation for these and
>> > that"
>> > > > part
>> > > > > of the API specification.
>> > > > >
>> > > > > On Fri, May 31, 2024 at 7:54 AM Jarek Potiuk <ja...@potiuk.com>
>> > wrote:
>> > > > >
>> > > > > > So let's document as part of the API which queries are not
>> > performant
>> > > > and
>> > > > > > suggest users that want to use them to make their analytics
>> queries
>> > > > > > elsewhere. I'd very much prefer that it's slow "by design" for
>> > > everyone
>> > > > > > rather than add option for the user to speed them up where we
>> > decided
>> > > > > > not to do it ourselves because we know the consequences.
>> > > > > >
>> > > > > > I think the root cause of the problem is `US` adding new
>> filtering
>> > > > > options
>> > > > > > to public API without thinking about and documenting
>> consequences.
>> > > > > >
>> > > > > > J.
>> > > > > >
>> > > > > >
>> > > > > > On Fri, May 31, 2024 at 7:38 AM Daniel Standish
>> > > > > > <da...@astronomer.io.invalid> wrote:
>> > > > > >
>> > > > > >> So I think the notion that *all possibly expensive queries*
>> should
>> > > > have
>> > > > > an
>> > > > > >> index to support them is not a tenable one.  E.g. there are
>> > > something
>> > > > > like
>> > > > > >> 5 params on TI list endpoint that don't have an index.
>> > > > > >>
>> > > > > >> In contrast with queries from airflow itself, the API queries
>> are
>> > > more
>> > > > > >> arbitrary -- user can combine in any way.  So it's not so
>> simple
>> > to
>> > > > > >> determine just from the endpoint params what indexes will be
>> > needed.
>> > > > > And
>> > > > > >> we're not going to add an index for all possible param
>> > combinations,
>> > > > > that
>> > > > > >> would be silly.  So I think like it or not I think we will
>> > > necessarily
>> > > > > >> draw
>> > > > > >> the line at *some* point and say we're trying to cover the most
>> > > > likely /
>> > > > > >> normal / reasonable uses and if your usage pattern is an
>> outlier
>> > you
>> > > > > might
>> > > > > >> need to add an index yourself.  And I don't think that's
>> > > unreasonable
>> > > > > >>
>> > > > > >> Maybe at least one conclusion from this is to add an index for
>> > each
>> > > of
>> > > > > >> start_date, end_date, and updated_at on TI.  Already the
>> storage
>> > > > > occupied
>> > > > > >> by indexes on TI is more than the table data size -- I don't
>> > > remember
>> > > > it
>> > > > > >> might have been like 2x or something.  So then index count will
>> > > > increase
>> > > > > >> from 8 to 11 and overhead increases.  Whether this is a
>> material
>> > > > > concern,
>> > > > > >> I
>> > > > > >> am honestly not sure -- I am not a db specialist.  But it does
>> > feel
>> > > > icky
>> > > > > >> to
>> > > > > >> add all the indexes when they are used by, I suspect, a very
>> small
>> > > > > >> percentage of airflow users overall.
>> > > > > >>
>> > > > > >> Part of this for me is also the reason these types of queries
>> are
>> > > run.
>> > > > > >> Typically it is for analytics purposes, either on the fly or as
>> > part
>> > > > of
>> > > > > a
>> > > > > >> replication process to a DWH or something.  But there are more
>> > > > efficient
>> > > > > >> mechanisms for this than pulling the data through the
>> webserver,
>> > > e.g.
>> > > > > >> read-only replicas, or standard ETL processes, or just making
>> > better
>> > > > use
>> > > > > >> of
>> > > > > >> the indexes that are already there.  Do we consider
>> "replication"
>> > a
>> > > > > >> reasonable use of the REST API?
>> > > > > >>
>> > > > > >> So I may very well be alone on this. But (1) the API is not
>> > > something
>> > > > > >> everyone uses.  And (2) even of users of the API, probably
>> only a
>> > > very
>> > > > > >> small percentage are using it in a way that needs these
>> indexes --
>> > > > > >> otherwise why haven't we seen requests for indexes on these
>> > columns
>> > > on
>> > > > > >> github?  So, to me, it doesn't seem crazy to just leave it to
>> the
>> > > > user /
>> > > > > >> platform to add.  But as usual, not a hill to die on.
>> > > > > >>
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>

Re: [DISCUSS] indexes for API calls

[Jarek Potiuk <ja...@potiuk.com>]

Using UUIDS was the proposal how we can bypass the limitation of MYSQL for
Airflow 2 when we discussed whether to do a "simple" version of team-prefix
in dag id, or whether we want to mess with adding
yet-another-field-to-indexes-that-are-already-too-long-for-mysql and it was
based on the assumption we are going to that for versioning already, so I
thought that is the plan :). Main reason why I put the multi-team voting on
hold while we decide on the scope of Airflow 3 to be honest.

https://apache-airflow.slack.com/archives/C06K9Q5G2UA/p1714140873323569?thread_ts=1713607491.495499&cid=C06K9Q5G2UA

J.


On Fri, May 31, 2024 at 9:04 AM Daniel Standish
<da...@astronomer.io.invalid> wrote:

> Yes uuid is risky and problematic as primary key. If you do it you need to
> do carefully/ sequential.  But I think that we are not going with UUID pk
> on any tables at this time.
>
> BUT I do want to add a uuid for every TI try that is not PK but can be used
> as a more convenient identifier when tying together log events related to
> TIs emitted on different services.
>
> On Thu, May 30, 2024 at 11:53 PM Jarek Potiuk <ja...@potiuk.com> wrote:
>
> > Also if we are speaking about indexes - a bit tangential but I know we
> were
> > planning to replace some of the primary keys (mainly because of mysql
> > limitations) with synthetic keys for DAG versioning casse where we
> planned
> > to use UUIDs).
> >
> > We should be very, very careful when doing it because I've learned
> recently
> > that due to UUID randomness the algorithm to rebalance binary tree do not
> > work well with random UUIDs which means that when you do a lot of
> inserts,
> > rebalancing of the trees is happening very often and performance of
> inserts
> > is heavily affected.  Postgres has a dedicated UUID field type for that
> > that produces an optimal index
> >
> >
> https://dba.stackexchange.com/questions/102448/how-should-i-index-a-uuid-in-postgres
> > - however it might or might not be affected. Similarly MariaDB, but MySQL
> > does not seem to have proper UUID support, so we should really use UUID7
> > rather that UUID4 for such UUIDs in case we do not want to affect insert
> > performance on MySQL.
> >
> > WAT???? Yeah. That is. - I think - rather non-obvious side-effect.
> >
> > This learning I had adds much more strength to two related discussions:
> >
> > 1) This only adds more fire to "let's only stick to Postgres". We can
> > optimize the experience for our users way better and move faster.
> >
> > 2) Let's REALLY not put the power of deciding what indexes to put on the
> > database of ours in the hands of our users. There are many unobvious
> > consequences of adding indexes and while we can aim to learn, foresee and
> > control them (mostly when we have a working performance test suite),
> > getting a random user to put a random index on a random set of keys in
> > Airflow DB might have a number of unobvious consequences which might give
> > our users (and us) a LOT of headache.
> >
> > J.
> >
> >
> >
> >
> > On Fri, May 31, 2024 at 8:13 AM Pankaj Koti
> > <pa...@astronomer.io.invalid> wrote:
> >
> > > Addressing one of Pierre's questions:
> > >
> > > Should I index foreign keys? Is that done by default or should I
> > explicitly
> > > do it?
> > >
> > > The answer varies depending on the database engine.
> > > PostgreSQL and SQLite do not add indexes on foreign keys
> > > by default, while MySQL does. Developers should keep this
> > > in mind. Many of us (myself included) may have assumed
> > > from our university days, indexes are always created for
> > > foreign keys, but that's not always the case. :)
> > >
> > > Best regards,
> > >
> > > *Pankaj Koti*
> > > Senior Software Engineer (Airflow OSS Engineering team)
> > > Location: Pune, Maharashtra, India
> > > Timezone: Indian Standard Time (IST)
> > >
> > >
> > > On Fri, May 31, 2024 at 11:37 AM Daniel Standish
> > > <da...@astronomer.io.invalid> wrote:
> > >
> > > > I would be in favor of this for sure.  Let's see what others think :)
> > > >
> > > > On Thu, May 30, 2024 at 10:55 PM Jarek Potiuk <ja...@potiuk.com>
> > wrote:
> > > >
> > > > > Simply speaking - let's make "lack of optimisation for these and
> > that"
> > > > part
> > > > > of the API specification.
> > > > >
> > > > > On Fri, May 31, 2024 at 7:54 AM Jarek Potiuk <ja...@potiuk.com>
> > wrote:
> > > > >
> > > > > > So let's document as part of the API which queries are not
> > performant
> > > > and
> > > > > > suggest users that want to use them to make their analytics
> queries
> > > > > > elsewhere. I'd very much prefer that it's slow "by design" for
> > > everyone
> > > > > > rather than add option for the user to speed them up where we
> > decided
> > > > > > not to do it ourselves because we know the consequences.
> > > > > >
> > > > > > I think the root cause of the problem is `US` adding new
> filtering
> > > > > options
> > > > > > to public API without thinking about and documenting
> consequences.
> > > > > >
> > > > > > J.
> > > > > >
> > > > > >
> > > > > > On Fri, May 31, 2024 at 7:38 AM Daniel Standish
> > > > > > <da...@astronomer.io.invalid> wrote:
> > > > > >
> > > > > >> So I think the notion that *all possibly expensive queries*
> should
> > > > have
> > > > > an
> > > > > >> index to support them is not a tenable one.  E.g. there are
> > > something
> > > > > like
> > > > > >> 5 params on TI list endpoint that don't have an index.
> > > > > >>
> > > > > >> In contrast with queries from airflow itself, the API queries
> are
> > > more
> > > > > >> arbitrary -- user can combine in any way.  So it's not so simple
> > to
> > > > > >> determine just from the endpoint params what indexes will be
> > needed.
> > > > > And
> > > > > >> we're not going to add an index for all possible param
> > combinations,
> > > > > that
> > > > > >> would be silly.  So I think like it or not I think we will
> > > necessarily
> > > > > >> draw
> > > > > >> the line at *some* point and say we're trying to cover the most
> > > > likely /
> > > > > >> normal / reasonable uses and if your usage pattern is an outlier
> > you
> > > > > might
> > > > > >> need to add an index yourself.  And I don't think that's
> > > unreasonable
> > > > > >>
> > > > > >> Maybe at least one conclusion from this is to add an index for
> > each
> > > of
> > > > > >> start_date, end_date, and updated_at on TI.  Already the storage
> > > > > occupied
> > > > > >> by indexes on TI is more than the table data size -- I don't
> > > remember
> > > > it
> > > > > >> might have been like 2x or something.  So then index count will
> > > > increase
> > > > > >> from 8 to 11 and overhead increases.  Whether this is a material
> > > > > concern,
> > > > > >> I
> > > > > >> am honestly not sure -- I am not a db specialist.  But it does
> > feel
> > > > icky
> > > > > >> to
> > > > > >> add all the indexes when they are used by, I suspect, a very
> small
> > > > > >> percentage of airflow users overall.
> > > > > >>
> > > > > >> Part of this for me is also the reason these types of queries
> are
> > > run.
> > > > > >> Typically it is for analytics purposes, either on the fly or as
> > part
> > > > of
> > > > > a
> > > > > >> replication process to a DWH or something.  But there are more
> > > > efficient
> > > > > >> mechanisms for this than pulling the data through the webserver,
> > > e.g.
> > > > > >> read-only replicas, or standard ETL processes, or just making
> > better
> > > > use
> > > > > >> of
> > > > > >> the indexes that are already there.  Do we consider
> "replication"
> > a
> > > > > >> reasonable use of the REST API?
> > > > > >>
> > > > > >> So I may very well be alone on this. But (1) the API is not
> > > something
> > > > > >> everyone uses.  And (2) even of users of the API, probably only
> a
> > > very
> > > > > >> small percentage are using it in a way that needs these indexes
> --
> > > > > >> otherwise why haven't we seen requests for indexes on these
> > columns
> > > on
> > > > > >> github?  So, to me, it doesn't seem crazy to just leave it to
> the
> > > > user /
> > > > > >> platform to add.  But as usual, not a hill to die on.
> > > > > >>
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: [DISCUSS] indexes for API calls

[Daniel Standish <da...@astronomer.io.INVALID>]

Yes uuid is risky and problematic as primary key. If you do it you need to
do carefully/ sequential.  But I think that we are not going with UUID pk
on any tables at this time.

BUT I do want to add a uuid for every TI try that is not PK but can be used
as a more convenient identifier when tying together log events related to
TIs emitted on different services.

On Thu, May 30, 2024 at 11:53 PM Jarek Potiuk <ja...@potiuk.com> wrote:

> Also if we are speaking about indexes - a bit tangential but I know we were
> planning to replace some of the primary keys (mainly because of mysql
> limitations) with synthetic keys for DAG versioning casse where we planned
> to use UUIDs).
>
> We should be very, very careful when doing it because I've learned recently
> that due to UUID randomness the algorithm to rebalance binary tree do not
> work well with random UUIDs which means that when you do a lot of inserts,
> rebalancing of the trees is happening very often and performance of inserts
> is heavily affected.  Postgres has a dedicated UUID field type for that
> that produces an optimal index
>
> https://dba.stackexchange.com/questions/102448/how-should-i-index-a-uuid-in-postgres
> - however it might or might not be affected. Similarly MariaDB, but MySQL
> does not seem to have proper UUID support, so we should really use UUID7
> rather that UUID4 for such UUIDs in case we do not want to affect insert
> performance on MySQL.
>
> WAT???? Yeah. That is. - I think - rather non-obvious side-effect.
>
> This learning I had adds much more strength to two related discussions:
>
> 1) This only adds more fire to "let's only stick to Postgres". We can
> optimize the experience for our users way better and move faster.
>
> 2) Let's REALLY not put the power of deciding what indexes to put on the
> database of ours in the hands of our users. There are many unobvious
> consequences of adding indexes and while we can aim to learn, foresee and
> control them (mostly when we have a working performance test suite),
> getting a random user to put a random index on a random set of keys in
> Airflow DB might have a number of unobvious consequences which might give
> our users (and us) a LOT of headache.
>
> J.
>
>
>
>
> On Fri, May 31, 2024 at 8:13 AM Pankaj Koti
> <pa...@astronomer.io.invalid> wrote:
>
> > Addressing one of Pierre's questions:
> >
> > Should I index foreign keys? Is that done by default or should I
> explicitly
> > do it?
> >
> > The answer varies depending on the database engine.
> > PostgreSQL and SQLite do not add indexes on foreign keys
> > by default, while MySQL does. Developers should keep this
> > in mind. Many of us (myself included) may have assumed
> > from our university days, indexes are always created for
> > foreign keys, but that's not always the case. :)
> >
> > Best regards,
> >
> > *Pankaj Koti*
> > Senior Software Engineer (Airflow OSS Engineering team)
> > Location: Pune, Maharashtra, India
> > Timezone: Indian Standard Time (IST)
> >
> >
> > On Fri, May 31, 2024 at 11:37 AM Daniel Standish
> > <da...@astronomer.io.invalid> wrote:
> >
> > > I would be in favor of this for sure.  Let's see what others think :)
> > >
> > > On Thu, May 30, 2024 at 10:55 PM Jarek Potiuk <ja...@potiuk.com>
> wrote:
> > >
> > > > Simply speaking - let's make "lack of optimisation for these and
> that"
> > > part
> > > > of the API specification.
> > > >
> > > > On Fri, May 31, 2024 at 7:54 AM Jarek Potiuk <ja...@potiuk.com>
> wrote:
> > > >
> > > > > So let's document as part of the API which queries are not
> performant
> > > and
> > > > > suggest users that want to use them to make their analytics queries
> > > > > elsewhere. I'd very much prefer that it's slow "by design" for
> > everyone
> > > > > rather than add option for the user to speed them up where we
> decided
> > > > > not to do it ourselves because we know the consequences.
> > > > >
> > > > > I think the root cause of the problem is `US` adding new filtering
> > > > options
> > > > > to public API without thinking about and documenting consequences.
> > > > >
> > > > > J.
> > > > >
> > > > >
> > > > > On Fri, May 31, 2024 at 7:38 AM Daniel Standish
> > > > > <da...@astronomer.io.invalid> wrote:
> > > > >
> > > > >> So I think the notion that *all possibly expensive queries* should
> > > have
> > > > an
> > > > >> index to support them is not a tenable one.  E.g. there are
> > something
> > > > like
> > > > >> 5 params on TI list endpoint that don't have an index.
> > > > >>
> > > > >> In contrast with queries from airflow itself, the API queries are
> > more
> > > > >> arbitrary -- user can combine in any way.  So it's not so simple
> to
> > > > >> determine just from the endpoint params what indexes will be
> needed.
> > > > And
> > > > >> we're not going to add an index for all possible param
> combinations,
> > > > that
> > > > >> would be silly.  So I think like it or not I think we will
> > necessarily
> > > > >> draw
> > > > >> the line at *some* point and say we're trying to cover the most
> > > likely /
> > > > >> normal / reasonable uses and if your usage pattern is an outlier
> you
> > > > might
> > > > >> need to add an index yourself.  And I don't think that's
> > unreasonable
> > > > >>
> > > > >> Maybe at least one conclusion from this is to add an index for
> each
> > of
> > > > >> start_date, end_date, and updated_at on TI.  Already the storage
> > > > occupied
> > > > >> by indexes on TI is more than the table data size -- I don't
> > remember
> > > it
> > > > >> might have been like 2x or something.  So then index count will
> > > increase
> > > > >> from 8 to 11 and overhead increases.  Whether this is a material
> > > > concern,
> > > > >> I
> > > > >> am honestly not sure -- I am not a db specialist.  But it does
> feel
> > > icky
> > > > >> to
> > > > >> add all the indexes when they are used by, I suspect, a very small
> > > > >> percentage of airflow users overall.
> > > > >>
> > > > >> Part of this for me is also the reason these types of queries are
> > run.
> > > > >> Typically it is for analytics purposes, either on the fly or as
> part
> > > of
> > > > a
> > > > >> replication process to a DWH or something.  But there are more
> > > efficient
> > > > >> mechanisms for this than pulling the data through the webserver,
> > e.g.
> > > > >> read-only replicas, or standard ETL processes, or just making
> better
> > > use
> > > > >> of
> > > > >> the indexes that are already there.  Do we consider "replication"
> a
> > > > >> reasonable use of the REST API?
> > > > >>
> > > > >> So I may very well be alone on this. But (1) the API is not
> > something
> > > > >> everyone uses.  And (2) even of users of the API, probably only a
> > very
> > > > >> small percentage are using it in a way that needs these indexes --
> > > > >> otherwise why haven't we seen requests for indexes on these
> columns
> > on
> > > > >> github?  So, to me, it doesn't seem crazy to just leave it to the
> > > user /
> > > > >> platform to add.  But as usual, not a hill to die on.
> > > > >>
> > > > >
> > > >
> > >
> >
>

Re: [DISCUSS] indexes for API calls

[Jarek Potiuk <ja...@potiuk.com>]

Also if we are speaking about indexes - a bit tangential but I know we were
planning to replace some of the primary keys (mainly because of mysql
limitations) with synthetic keys for DAG versioning casse where we planned
to use UUIDs).

We should be very, very careful when doing it because I've learned recently
that due to UUID randomness the algorithm to rebalance binary tree do not
work well with random UUIDs which means that when you do a lot of inserts,
rebalancing of the trees is happening very often and performance of inserts
is heavily affected.  Postgres has a dedicated UUID field type for that
that produces an optimal index
https://dba.stackexchange.com/questions/102448/how-should-i-index-a-uuid-in-postgres
- however it might or might not be affected. Similarly MariaDB, but MySQL
does not seem to have proper UUID support, so we should really use UUID7
rather that UUID4 for such UUIDs in case we do not want to affect insert
performance on MySQL.

WAT???? Yeah. That is. - I think - rather non-obvious side-effect.

This learning I had adds much more strength to two related discussions:

1) This only adds more fire to "let's only stick to Postgres". We can
optimize the experience for our users way better and move faster.

2) Let's REALLY not put the power of deciding what indexes to put on the
database of ours in the hands of our users. There are many unobvious
consequences of adding indexes and while we can aim to learn, foresee and
control them (mostly when we have a working performance test suite),
getting a random user to put a random index on a random set of keys in
Airflow DB might have a number of unobvious consequences which might give
our users (and us) a LOT of headache.

J.




On Fri, May 31, 2024 at 8:13 AM Pankaj Koti
<pa...@astronomer.io.invalid> wrote:

> Addressing one of Pierre's questions:
>
> Should I index foreign keys? Is that done by default or should I explicitly
> do it?
>
> The answer varies depending on the database engine.
> PostgreSQL and SQLite do not add indexes on foreign keys
> by default, while MySQL does. Developers should keep this
> in mind. Many of us (myself included) may have assumed
> from our university days, indexes are always created for
> foreign keys, but that's not always the case. :)
>
> Best regards,
>
> *Pankaj Koti*
> Senior Software Engineer (Airflow OSS Engineering team)
> Location: Pune, Maharashtra, India
> Timezone: Indian Standard Time (IST)
>
>
> On Fri, May 31, 2024 at 11:37 AM Daniel Standish
> <da...@astronomer.io.invalid> wrote:
>
> > I would be in favor of this for sure.  Let's see what others think :)
> >
> > On Thu, May 30, 2024 at 10:55 PM Jarek Potiuk <ja...@potiuk.com> wrote:
> >
> > > Simply speaking - let's make "lack of optimisation for these and that"
> > part
> > > of the API specification.
> > >
> > > On Fri, May 31, 2024 at 7:54 AM Jarek Potiuk <ja...@potiuk.com> wrote:
> > >
> > > > So let's document as part of the API which queries are not performant
> > and
> > > > suggest users that want to use them to make their analytics queries
> > > > elsewhere. I'd very much prefer that it's slow "by design" for
> everyone
> > > > rather than add option for the user to speed them up where we decided
> > > > not to do it ourselves because we know the consequences.
> > > >
> > > > I think the root cause of the problem is `US` adding new filtering
> > > options
> > > > to public API without thinking about and documenting consequences.
> > > >
> > > > J.
> > > >
> > > >
> > > > On Fri, May 31, 2024 at 7:38 AM Daniel Standish
> > > > <da...@astronomer.io.invalid> wrote:
> > > >
> > > >> So I think the notion that *all possibly expensive queries* should
> > have
> > > an
> > > >> index to support them is not a tenable one.  E.g. there are
> something
> > > like
> > > >> 5 params on TI list endpoint that don't have an index.
> > > >>
> > > >> In contrast with queries from airflow itself, the API queries are
> more
> > > >> arbitrary -- user can combine in any way.  So it's not so simple to
> > > >> determine just from the endpoint params what indexes will be needed.
> > > And
> > > >> we're not going to add an index for all possible param combinations,
> > > that
> > > >> would be silly.  So I think like it or not I think we will
> necessarily
> > > >> draw
> > > >> the line at *some* point and say we're trying to cover the most
> > likely /
> > > >> normal / reasonable uses and if your usage pattern is an outlier you
> > > might
> > > >> need to add an index yourself.  And I don't think that's
> unreasonable
> > > >>
> > > >> Maybe at least one conclusion from this is to add an index for each
> of
> > > >> start_date, end_date, and updated_at on TI.  Already the storage
> > > occupied
> > > >> by indexes on TI is more than the table data size -- I don't
> remember
> > it
> > > >> might have been like 2x or something.  So then index count will
> > increase
> > > >> from 8 to 11 and overhead increases.  Whether this is a material
> > > concern,
> > > >> I
> > > >> am honestly not sure -- I am not a db specialist.  But it does feel
> > icky
> > > >> to
> > > >> add all the indexes when they are used by, I suspect, a very small
> > > >> percentage of airflow users overall.
> > > >>
> > > >> Part of this for me is also the reason these types of queries are
> run.
> > > >> Typically it is for analytics purposes, either on the fly or as part
> > of
> > > a
> > > >> replication process to a DWH or something.  But there are more
> > efficient
> > > >> mechanisms for this than pulling the data through the webserver,
> e.g.
> > > >> read-only replicas, or standard ETL processes, or just making better
> > use
> > > >> of
> > > >> the indexes that are already there.  Do we consider "replication" a
> > > >> reasonable use of the REST API?
> > > >>
> > > >> So I may very well be alone on this. But (1) the API is not
> something
> > > >> everyone uses.  And (2) even of users of the API, probably only a
> very
> > > >> small percentage are using it in a way that needs these indexes --
> > > >> otherwise why haven't we seen requests for indexes on these columns
> on
> > > >> github?  So, to me, it doesn't seem crazy to just leave it to the
> > user /
> > > >> platform to add.  But as usual, not a hill to die on.
> > > >>
> > > >
> > >
> >
>

Re: [DISCUSS] indexes for API calls

[Pankaj Koti <pa...@astronomer.io.INVALID>]

Addressing one of Pierre's questions:

Should I index foreign keys? Is that done by default or should I explicitly
do it?

The answer varies depending on the database engine.
PostgreSQL and SQLite do not add indexes on foreign keys
by default, while MySQL does. Developers should keep this
in mind. Many of us (myself included) may have assumed
from our university days, indexes are always created for
foreign keys, but that's not always the case. :)

Best regards,

*Pankaj Koti*
Senior Software Engineer (Airflow OSS Engineering team)
Location: Pune, Maharashtra, India
Timezone: Indian Standard Time (IST)


On Fri, May 31, 2024 at 11:37 AM Daniel Standish
<da...@astronomer.io.invalid> wrote:

> I would be in favor of this for sure.  Let's see what others think :)
>
> On Thu, May 30, 2024 at 10:55 PM Jarek Potiuk <ja...@potiuk.com> wrote:
>
> > Simply speaking - let's make "lack of optimisation for these and that"
> part
> > of the API specification.
> >
> > On Fri, May 31, 2024 at 7:54 AM Jarek Potiuk <ja...@potiuk.com> wrote:
> >
> > > So let's document as part of the API which queries are not performant
> and
> > > suggest users that want to use them to make their analytics queries
> > > elsewhere. I'd very much prefer that it's slow "by design" for everyone
> > > rather than add option for the user to speed them up where we decided
> > > not to do it ourselves because we know the consequences.
> > >
> > > I think the root cause of the problem is `US` adding new filtering
> > options
> > > to public API without thinking about and documenting consequences.
> > >
> > > J.
> > >
> > >
> > > On Fri, May 31, 2024 at 7:38 AM Daniel Standish
> > > <da...@astronomer.io.invalid> wrote:
> > >
> > >> So I think the notion that *all possibly expensive queries* should
> have
> > an
> > >> index to support them is not a tenable one.  E.g. there are something
> > like
> > >> 5 params on TI list endpoint that don't have an index.
> > >>
> > >> In contrast with queries from airflow itself, the API queries are more
> > >> arbitrary -- user can combine in any way.  So it's not so simple to
> > >> determine just from the endpoint params what indexes will be needed.
> > And
> > >> we're not going to add an index for all possible param combinations,
> > that
> > >> would be silly.  So I think like it or not I think we will necessarily
> > >> draw
> > >> the line at *some* point and say we're trying to cover the most
> likely /
> > >> normal / reasonable uses and if your usage pattern is an outlier you
> > might
> > >> need to add an index yourself.  And I don't think that's unreasonable
> > >>
> > >> Maybe at least one conclusion from this is to add an index for each of
> > >> start_date, end_date, and updated_at on TI.  Already the storage
> > occupied
> > >> by indexes on TI is more than the table data size -- I don't remember
> it
> > >> might have been like 2x or something.  So then index count will
> increase
> > >> from 8 to 11 and overhead increases.  Whether this is a material
> > concern,
> > >> I
> > >> am honestly not sure -- I am not a db specialist.  But it does feel
> icky
> > >> to
> > >> add all the indexes when they are used by, I suspect, a very small
> > >> percentage of airflow users overall.
> > >>
> > >> Part of this for me is also the reason these types of queries are run.
> > >> Typically it is for analytics purposes, either on the fly or as part
> of
> > a
> > >> replication process to a DWH or something.  But there are more
> efficient
> > >> mechanisms for this than pulling the data through the webserver, e.g.
> > >> read-only replicas, or standard ETL processes, or just making better
> use
> > >> of
> > >> the indexes that are already there.  Do we consider "replication" a
> > >> reasonable use of the REST API?
> > >>
> > >> So I may very well be alone on this. But (1) the API is not something
> > >> everyone uses.  And (2) even of users of the API, probably only a very
> > >> small percentage are using it in a way that needs these indexes --
> > >> otherwise why haven't we seen requests for indexes on these columns on
> > >> github?  So, to me, it doesn't seem crazy to just leave it to the
> user /
> > >> platform to add.  But as usual, not a hill to die on.
> > >>
> > >
> >
>

Re: [DISCUSS] indexes for API calls

[Daniel Standish <da...@astronomer.io.INVALID>]

I would be in favor of this for sure.  Let's see what others think :)

On Thu, May 30, 2024 at 10:55 PM Jarek Potiuk <ja...@potiuk.com> wrote:

> Simply speaking - let's make "lack of optimisation for these and that" part
> of the API specification.
>
> On Fri, May 31, 2024 at 7:54 AM Jarek Potiuk <ja...@potiuk.com> wrote:
>
> > So let's document as part of the API which queries are not performant and
> > suggest users that want to use them to make their analytics queries
> > elsewhere. I'd very much prefer that it's slow "by design" for everyone
> > rather than add option for the user to speed them up where we decided
> > not to do it ourselves because we know the consequences.
> >
> > I think the root cause of the problem is `US` adding new filtering
> options
> > to public API without thinking about and documenting consequences.
> >
> > J.
> >
> >
> > On Fri, May 31, 2024 at 7:38 AM Daniel Standish
> > <da...@astronomer.io.invalid> wrote:
> >
> >> So I think the notion that *all possibly expensive queries* should have
> an
> >> index to support them is not a tenable one.  E.g. there are something
> like
> >> 5 params on TI list endpoint that don't have an index.
> >>
> >> In contrast with queries from airflow itself, the API queries are more
> >> arbitrary -- user can combine in any way.  So it's not so simple to
> >> determine just from the endpoint params what indexes will be needed.
> And
> >> we're not going to add an index for all possible param combinations,
> that
> >> would be silly.  So I think like it or not I think we will necessarily
> >> draw
> >> the line at *some* point and say we're trying to cover the most likely /
> >> normal / reasonable uses and if your usage pattern is an outlier you
> might
> >> need to add an index yourself.  And I don't think that's unreasonable
> >>
> >> Maybe at least one conclusion from this is to add an index for each of
> >> start_date, end_date, and updated_at on TI.  Already the storage
> occupied
> >> by indexes on TI is more than the table data size -- I don't remember it
> >> might have been like 2x or something.  So then index count will increase
> >> from 8 to 11 and overhead increases.  Whether this is a material
> concern,
> >> I
> >> am honestly not sure -- I am not a db specialist.  But it does feel icky
> >> to
> >> add all the indexes when they are used by, I suspect, a very small
> >> percentage of airflow users overall.
> >>
> >> Part of this for me is also the reason these types of queries are run.
> >> Typically it is for analytics purposes, either on the fly or as part of
> a
> >> replication process to a DWH or something.  But there are more efficient
> >> mechanisms for this than pulling the data through the webserver, e.g.
> >> read-only replicas, or standard ETL processes, or just making better use
> >> of
> >> the indexes that are already there.  Do we consider "replication" a
> >> reasonable use of the REST API?
> >>
> >> So I may very well be alone on this. But (1) the API is not something
> >> everyone uses.  And (2) even of users of the API, probably only a very
> >> small percentage are using it in a way that needs these indexes --
> >> otherwise why haven't we seen requests for indexes on these columns on
> >> github?  So, to me, it doesn't seem crazy to just leave it to the user /
> >> platform to add.  But as usual, not a hill to die on.
> >>
> >
>

Re: [DISCUSS] indexes for API calls

[Jarek Potiuk <ja...@potiuk.com>]

Simply speaking - let's make "lack of optimisation for these and that" part
of the API specification.

On Fri, May 31, 2024 at 7:54 AM Jarek Potiuk <ja...@potiuk.com> wrote:

> So let's document as part of the API which queries are not performant and
> suggest users that want to use them to make their analytics queries
> elsewhere. I'd very much prefer that it's slow "by design" for everyone
> rather than add option for the user to speed them up where we decided
> not to do it ourselves because we know the consequences.
>
> I think the root cause of the problem is `US` adding new filtering options
> to public API without thinking about and documenting consequences.
>
> J.
>
>
> On Fri, May 31, 2024 at 7:38 AM Daniel Standish
> <da...@astronomer.io.invalid> wrote:
>
>> So I think the notion that *all possibly expensive queries* should have an
>> index to support them is not a tenable one.  E.g. there are something like
>> 5 params on TI list endpoint that don't have an index.
>>
>> In contrast with queries from airflow itself, the API queries are more
>> arbitrary -- user can combine in any way.  So it's not so simple to
>> determine just from the endpoint params what indexes will be needed.  And
>> we're not going to add an index for all possible param combinations, that
>> would be silly.  So I think like it or not I think we will necessarily
>> draw
>> the line at *some* point and say we're trying to cover the most likely /
>> normal / reasonable uses and if your usage pattern is an outlier you might
>> need to add an index yourself.  And I don't think that's unreasonable
>>
>> Maybe at least one conclusion from this is to add an index for each of
>> start_date, end_date, and updated_at on TI.  Already the storage occupied
>> by indexes on TI is more than the table data size -- I don't remember it
>> might have been like 2x or something.  So then index count will increase
>> from 8 to 11 and overhead increases.  Whether this is a material concern,
>> I
>> am honestly not sure -- I am not a db specialist.  But it does feel icky
>> to
>> add all the indexes when they are used by, I suspect, a very small
>> percentage of airflow users overall.
>>
>> Part of this for me is also the reason these types of queries are run.
>> Typically it is for analytics purposes, either on the fly or as part of a
>> replication process to a DWH or something.  But there are more efficient
>> mechanisms for this than pulling the data through the webserver, e.g.
>> read-only replicas, or standard ETL processes, or just making better use
>> of
>> the indexes that are already there.  Do we consider "replication" a
>> reasonable use of the REST API?
>>
>> So I may very well be alone on this. But (1) the API is not something
>> everyone uses.  And (2) even of users of the API, probably only a very
>> small percentage are using it in a way that needs these indexes --
>> otherwise why haven't we seen requests for indexes on these columns on
>> github?  So, to me, it doesn't seem crazy to just leave it to the user /
>> platform to add.  But as usual, not a hill to die on.
>>
>

Re: [DISCUSS] indexes for API calls

[Jarek Potiuk <ja...@potiuk.com>]

So let's document as part of the API which queries are not performant and
suggest users that want to use them to make their analytics queries
elsewhere. I'd very much prefer that it's slow "by design" for everyone
rather than add option for the user to speed them up where we decided
not to do it ourselves because we know the consequences.

I think the root cause of the problem is `US` adding new filtering options
to public API without thinking about and documenting consequences.

J.


On Fri, May 31, 2024 at 7:38 AM Daniel Standish
<da...@astronomer.io.invalid> wrote:

> So I think the notion that *all possibly expensive queries* should have an
> index to support them is not a tenable one.  E.g. there are something like
> 5 params on TI list endpoint that don't have an index.
>
> In contrast with queries from airflow itself, the API queries are more
> arbitrary -- user can combine in any way.  So it's not so simple to
> determine just from the endpoint params what indexes will be needed.  And
> we're not going to add an index for all possible param combinations, that
> would be silly.  So I think like it or not I think we will necessarily draw
> the line at *some* point and say we're trying to cover the most likely /
> normal / reasonable uses and if your usage pattern is an outlier you might
> need to add an index yourself.  And I don't think that's unreasonable
>
> Maybe at least one conclusion from this is to add an index for each of
> start_date, end_date, and updated_at on TI.  Already the storage occupied
> by indexes on TI is more than the table data size -- I don't remember it
> might have been like 2x or something.  So then index count will increase
> from 8 to 11 and overhead increases.  Whether this is a material concern, I
> am honestly not sure -- I am not a db specialist.  But it does feel icky to
> add all the indexes when they are used by, I suspect, a very small
> percentage of airflow users overall.
>
> Part of this for me is also the reason these types of queries are run.
> Typically it is for analytics purposes, either on the fly or as part of a
> replication process to a DWH or something.  But there are more efficient
> mechanisms for this than pulling the data through the webserver, e.g.
> read-only replicas, or standard ETL processes, or just making better use of
> the indexes that are already there.  Do we consider "replication" a
> reasonable use of the REST API?
>
> So I may very well be alone on this. But (1) the API is not something
> everyone uses.  And (2) even of users of the API, probably only a very
> small percentage are using it in a way that needs these indexes --
> otherwise why haven't we seen requests for indexes on these columns on
> github?  So, to me, it doesn't seem crazy to just leave it to the user /
> platform to add.  But as usual, not a hill to die on.
>

Re: [DISCUSS] indexes for API calls

[Daniel Standish <da...@astronomer.io.INVALID>]

So I think the notion that *all possibly expensive queries* should have an
index to support them is not a tenable one.  E.g. there are something like
5 params on TI list endpoint that don't have an index.

In contrast with queries from airflow itself, the API queries are more
arbitrary -- user can combine in any way.  So it's not so simple to
determine just from the endpoint params what indexes will be needed.  And
we're not going to add an index for all possible param combinations, that
would be silly.  So I think like it or not I think we will necessarily draw
the line at *some* point and say we're trying to cover the most likely /
normal / reasonable uses and if your usage pattern is an outlier you might
need to add an index yourself.  And I don't think that's unreasonable

Maybe at least one conclusion from this is to add an index for each of
start_date, end_date, and updated_at on TI.  Already the storage occupied
by indexes on TI is more than the table data size -- I don't remember it
might have been like 2x or something.  So then index count will increase
from 8 to 11 and overhead increases.  Whether this is a material concern, I
am honestly not sure -- I am not a db specialist.  But it does feel icky to
add all the indexes when they are used by, I suspect, a very small
percentage of airflow users overall.

Part of this for me is also the reason these types of queries are run.
Typically it is for analytics purposes, either on the fly or as part of a
replication process to a DWH or something.  But there are more efficient
mechanisms for this than pulling the data through the webserver, e.g.
read-only replicas, or standard ETL processes, or just making better use of
the indexes that are already there.  Do we consider "replication" a
reasonable use of the REST API?

So I may very well be alone on this. But (1) the API is not something
everyone uses.  And (2) even of users of the API, probably only a very
small percentage are using it in a way that needs these indexes --
otherwise why haven't we seen requests for indexes on these columns on
github?  So, to me, it doesn't seem crazy to just leave it to the user /
platform to add.  But as usual, not a hill to die on.

Re: [DISCUSS] indexes for API calls

[Jarek Potiuk <ja...@potiuk.com>]

The API is public, it **should** behave well regardless of local
customizations. We have automated DB maintenance and we "promise" to our
users it will work and we explicitly tell them "do not touch airflow DB as
you might break things".

Now - If we change the narration now and tell them "if you want to use a
particular APIS you will have to touch the DB and potentially break things"
- this is turning that completely upside - down and opens up for a
floodgate of things people would like to modify in their databases.

It would be an absolute nightmare to support our users if we say "yeah,
please modify the DB as you think is right". Most of them will not know
what modifications they did. In plenty of cases people who did it will be
already gone and absent from the project and there will be absolutely no
trace of what they did (because they just applied an index on a live DB one
day). This means that migration, failovers, switching clouds etc. - all
those scenarios will start breaking for such users. For me this is pretty
much no-go.

If anything - I think we should carefully choose what filtering criteria we
add to the API. We could introduce the rule that whenever we add something
there, a serious analysis on:

* whether this filtering is really needed
* does it need to be indexed for speed (maybe we can get away without it if
it's a secondary or tertiary criteria, and maybe we can change the
filtering criteria to make it so).
* what impact such index will have (size, rebalancing the trees, slowing
down inserts etc.)

We could actually somewhat automate it and raise a flag for any such change
and make sure those questions are answered before we merge such PR.

I'd rather gate such changes until answers to all those questions are known
than give the users freedom to modify Airflow metadata DB,

It also boils down to the performance "benchmark/baseline" and reproducible
performance tests discussion. This is the goal of AIP-59 (
https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-59).
If we had it, we could make sure any API change introduces acceptable
performance overhead (disk size, memory, cpu, etc all should be included).

J.

Re: [DISCUSS] indexes for API calls

[Pierre Jeambrun <pi...@gmail.com>]

Thank you for starting this discussion.

At first I would say that databases should be indexed to achieve good
performances against standard queries / use cases.

The Rest API does not do any crazy things/querying (as I recall). Listing,
filtering, ordering and searching against our main tables should not bring
performance issues. I would qualify that as a 'normal' usage. (in
opposition to a special edge case for the rest API)

Also this would be beneficial beyond the web server because the CLI,
Internal RPC, maybe the scheduler or even BI tools directly plugged on the
metadata database would have better performances. (not mentioning the UI of
course which is quite important to our users)

The  API is being more and more advertised to your users as the 'official'
way to interact with the airflow database, if we want that to be adopted
widely and prevent people from accessing the db directly, having a
performant API will be a requirement.

If some endpoints are too specific and there are better ways of getting
that same information, deprecating and later on removing them all together
could be a solution. In case they would require too many specific indexes
to work properly compared to its alternate solution. (or take the
opportunity of airflow 3 and remove them directly)

>  Another argument would be "this should be the responsibility of the
cluster maintainer", since it's not part of the core operation of airflow

I do not know if this implies leaving the customisation up to the cluster
manager, but letting people interfere with the db schema scares me.
(through customisation or something) I think it should remain a private
implementation detail. But maybe we can imagine a configurable option like
`[database] optimize_db_for_read_access` or something similar that would
apply an extra batch of indexing and people can choose whether or not they
want it ?

Defining an index strategy for the db could be beneficial in the long run.
(both for the API, core and help contributors as well). If I write a new
piece of code accessing the db, how do I know if it is indexed ? How do I
know if I should add an index ? Should I index foreign keys, is that done
by default or should I explicitly do it ?

Just throwing out a few ideas, curious to know what others think.
Best




Le mer. 29 mai 2024 à 22:00, Daniel Standish
<da...@astronomer.io.invalid> a écrit :

> Historically we have added indexes as needed for the performance of airflow
> itself and not for the rest API.
>
> Lately we've observed more usage of task instances list endpoint and
> specifically filtering on end_date and / or start_date and / or
> execution_date.
>
> One line of argument goes that every possible filter in the rest API should
> have an index.  But not all users use the API at all, let alone use all
> params in the rest API.  Or even *all possible combinations?!*
>
> Another argument would be "this should be the responsibility of the cluster
> maintainer", since it's not part of the core operation of airflow
> (scheduler / tasks / webserver) and highly dependent on specific use case.
>
> Another thing to consider is sometimes there's an efficient way to get the
> TIs you're looking for already with a slight refactor.  E.g. get the dag
> runs first, then fetch the TIs using the PK.
>
> Of course adding indexes is a trade off between storage space,
> performance overhead maintaining them etc, balanced against reasonable
> usage of the REST API.
>
> I'm curious what folks think about this, and whether and when we should add
> indexes in OSS that help REST API queries.
>