Mailing-List: contact commits-help@airflow.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@airflow.incubator.apache.org
Date: Fri, 6 May 2016 13:30:12 +0000 (UTC)
From: "Bence Nagy (JIRA)" <jira@apache.org>
To: commits@airflow.incubator.apache.org
Message-ID: <JIRA.12965590.1462541287000.128210.1462541412892@Atlassian.JIRA>
In-Reply-To: <JIRA.12965590.1462541287000@Atlassian.JIRA>
References: <JIRA.12965590.1462541287000@Atlassian.JIRA> <JIRA.12965590.1462541287208@arcas>
Subject: [jira] [Updated] (AIRFLOW-57) Rename concurrency configuration
 variables to be more clear
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
archived-at: Fri, 06 May 2016 13:30:19 -0000


     [ https://issues.apache.org/jira/browse/AIRFLOW-57?page=3Dcom.atlassia=
n.jira.plugin.system.issuetabpanels:all-tabpanel ]

Bence Nagy updated AIRFLOW-57:
------------------------------
    Description:=20
Currently the following config variables exists for controlling parallel ex=
ecution limits:

{code}
# The amount of parallelism as a setting to the executor. This defines
# the max number of task instances that should run simultaneously
# on this airflow installation
parallelism =3D 32

# The number of task instances allowed to run concurrently by the scheduler
dag_concurrency =3D 16

# When not using pools, tasks are run in the "default pool",
# whose size is guided by this config element
non_pooled_task_slot_count =3D 128

# The maximum number of active DAG runs per DAG
max_active_runs_per_dag =3D 16
{code}

Let's go through these one by one:

{{parallelism}}: not a very descriptive name, considering that all the abov=
e settings are for parallelism. The description says it sets the maximum ta=
sk instances for the airflow installation, which is a bit ambiguous =E2=80=
=94 if I have two hosts running airflow workers, I'd have airflow installed=
 on two hosts, so that should be two installations, but based on context 'p=
er installation' here means 'per Airflow state database'. I'd name this {{m=
ax_active_tasks}}.

{{dag_concurrency}}: Despite the name based on the comment this is actually=
 the task concurrency, and it's per worker. I'd name this {{max_active_task=
s_for_worker}} ({{per_worker}} would suggest that it's a global setting for=
 workers, but I think you can have workers with different values set for th=
is).

{{non_pooled_task_slot_count}}: This is a weird one. I'm going to pass on s=
uggesting a name for it because I just can't think of any reason this confi=
g variable should exist. We already have a global task instance limit, and =
we have pools to limit access to certain resources =E2=80=94 in what case w=
ould someone want to limit access to everything other than certain resource=
s? So, yeah, skipping this one. In case this was needed only due to how poo=
ls are implemented, I'd suggest setting the limit to {{sys.maxsize}} and ju=
st removing the config variable.

{{max_active_runs_per_dag}}: This one's kinda alright, but since it seems t=
o be just a default value for the matching {{DAG}} kwarg, it might be nice =
to reflect that in the name, something like {{default_max_active_runs_for_d=
ags}}

So let's move on to the {{DAG}} kwargs:

{{concurrency}}: Again, having a general name like this, coupled with the f=
act that concurrency is used for something different elsewhere makes this p=
retty confusing. I'd call this {{max_active_tasks}}.

{{max_active_runs}}: This one sounds alright to me.

So. If people agree that this is something that should be fixed, I think it=
'd be nice to get this in the 1.7.1 release, especially considering that it=
 should be really easy to make the change backwards compatible.

  was:
Currently the following config variables exists for controlling parallel ex=
ecution limits:

{code}
# The amount of parallelism as a setting to the executor. This defines
# the max number of task instances that should run simultaneously
# on this airflow installation
parallelism =3D 32

# The number of task instances allowed to run concurrently by the scheduler
dag_concurrency =3D 16

# When not using pools, tasks are run in the "default pool",
# whose size is guided by this config element
non_pooled_task_slot_count =3D 128

# The maximum number of active DAG runs per DAG
max_active_runs_per_dag =3D 16
{code}

Let's go through these one by one:

{{parallelism}}: not a very descriptive name, considering that all the abov=
e settings are for parallelism. The description says it sets the maximum ta=
sk instances for the airflow installation, which is a bit ambiguous =E2=80=
=94 if I have two hosts running airflow workers, I'd have airflow installed=
 on two hosts, so that should be two installations, but based on context 'p=
er installation' here means 'per Airflow state database'. I'd name this {{m=
ax_active_tasks}}.

{{dag_concurrency}}: Despite the name based on the comment this is actually=
 the task concurrency, and it's per worker. I'd name this {{max_active_task=
s_for_worker}} ({{per_worker}} would suggest that it's a global setting for=
 workers, but I think you can have workers with different values set for th=
is).

{{non_pooled_task_slot_count}}: This is a weird one. I'm going to pass on s=
uggesting a name for it because I just can't think of any reason this confi=
g variable should exist. We already have a global task instance limit, and =
we have pools to limit access to certain resources =E2=80=94 in what case w=
ould someone want to limit access to everything other than certain resource=
s? So, yeah, skipping this one.

{{max_active_runs_per_dag}}: This one's kinda alright, but since it seems t=
o be just a default value for the matching {{DAG}} kwarg, it might be nice =
to reflect that in the name, something like {{default_max_active_runs_for_d=
ags}}

So let's move on to the {{DAG}} kwargs:

{{concurrency}}: Again, having a general name like this, coupled with the f=
act that concurrency is used for something different elsewhere makes this p=
retty confusing. I'd call this {{max_active_tasks}}.

{{max_active_runs}}: This one sounds alright to me.

So. If people agree that this is something that should be fixed, I think it=
'd be nice to get this in the 1.7.1 release, especially considering that it=
 should be really easy to make the change backwards compatible.


> Rename concurrency configuration variables to be more clear
> -----------------------------------------------------------
>
>                 Key: AIRFLOW-57
>                 URL: https://issues.apache.org/jira/browse/AIRFLOW-57
>             Project: Apache Airflow
>          Issue Type: Improvement
>    Affects Versions: Airflow 1.7.0
>            Reporter: Bence Nagy
>            Priority: Minor
>              Labels: newbie
>
> Currently the following config variables exists for controlling parallel =
execution limits:
> {code}
> # The amount of parallelism as a setting to the executor. This defines
> # the max number of task instances that should run simultaneously
> # on this airflow installation
> parallelism =3D 32
> # The number of task instances allowed to run concurrently by the schedul=
er
> dag_concurrency =3D 16
> # When not using pools, tasks are run in the "default pool",
> # whose size is guided by this config element
> non_pooled_task_slot_count =3D 128
> # The maximum number of active DAG runs per DAG
> max_active_runs_per_dag =3D 16
> {code}
> Let's go through these one by one:
> {{parallelism}}: not a very descriptive name, considering that all the ab=
ove settings are for parallelism. The description says it sets the maximum =
task instances for the airflow installation, which is a bit ambiguous =E2=
=80=94 if I have two hosts running airflow workers, I'd have airflow instal=
led on two hosts, so that should be two installations, but based on context=
 'per installation' here means 'per Airflow state database'. I'd name this =
{{max_active_tasks}}.
> {{dag_concurrency}}: Despite the name based on the comment this is actual=
ly the task concurrency, and it's per worker. I'd name this {{max_active_ta=
sks_for_worker}} ({{per_worker}} would suggest that it's a global setting f=
or workers, but I think you can have workers with different values set for =
this).
> {{non_pooled_task_slot_count}}: This is a weird one. I'm going to pass on=
 suggesting a name for it because I just can't think of any reason this con=
fig variable should exist. We already have a global task instance limit, an=
d we have pools to limit access to certain resources =E2=80=94 in what case=
 would someone want to limit access to everything other than certain resour=
ces? So, yeah, skipping this one. In case this was needed only due to how p=
ools are implemented, I'd suggest setting the limit to {{sys.maxsize}} and =
just removing the config variable.
> {{max_active_runs_per_dag}}: This one's kinda alright, but since it seems=
 to be just a default value for the matching {{DAG}} kwarg, it might be nic=
e to reflect that in the name, something like {{default_max_active_runs_for=
_dags}}
> So let's move on to the {{DAG}} kwargs:
> {{concurrency}}: Again, having a general name like this, coupled with the=
 fact that concurrency is used for something different elsewhere makes this=
 pretty confusing. I'd call this {{max_active_tasks}}.
> {{max_active_runs}}: This one sounds alright to me.
> So. If people agree that this is something that should be fixed, I think =
it'd be nice to get this in the 1.7.1 release, especially considering that =
it should be really easy to make the change backwards compatible.


--
This message was sent by Atlassian JIRA
(v6.3.4#6332)