Skip to content

Commit

Permalink
Deprecate ambiguous SchedulingMetadata.task_size field (#7901)
Browse files Browse the repository at this point in the history
The `SchedulingMetadata.task_size` field is both lossy (since it is
supposed to contain the "default" task size, but gets overwritten) and
redundant (the executor allegedly uses this for scheduling but in
practice it actually uses EnqueueTaskReservationRequest.task_size).

Deprecate this field and add clearer, not-lossy alternatives. (We'll
keep setting it for some time, probably, because we do have some places
in the code where it's unfortunately used - e.g.
ExecutedActionMetadata.task_size gets set to this value, but probably
should be getting set to the task_size from
EnqueueTaskReservationRequest, which represents its "final" size for
local scheduling
  • Loading branch information
bduffany authored Nov 18, 2024
1 parent 06588f9 commit 369437d
Showing 1 changed file with 20 additions and 16 deletions.
36 changes: 20 additions & 16 deletions proto/scheduler.proto
Original file line number Diff line number Diff line change
Expand Up @@ -130,27 +130,31 @@ message TaskSize {

// Next ID: 9
message SchedulingMetadata {
// Task size used for scheduling purposes, when the scheduler is deciding
// which executors (if any) may execute a task, and also when an executor is
// deciding which task to dequeue. Executors may see a different value of this
// field than what the scheduler sees, depending on measured_task_size or
// predicted_task_size. See documentation of those fields for more info.
TaskSize task_size = 1;
// DEPRECATED. This field has two different meanings:
// 1. The execution server sets this to the task size "estimate" which only
// incorporates the default task size and user-requested size, and not any
// historical information or model predictions. This estimate is now split
// into two separate fields: default_task_size and requested_size.
// 2. Just before enqueueing a task on an executor, the scheduler sets this to
// the "final" task size that should be used by the executor, which
// incorporates the measured size or model-predicated size as applicable.
// In this case, EnqueueTaskReservationRequest.task_size should be used
// instead.
TaskSize task_size = 1 [deprecated = true];

// A default task size based on some hard-coded parameters. For example,
// non-test actions may be given some default task size, test actions
// declaring TEST_SIZE=large in their environment variables may be given a
// predefined number of resources, etc. If no better estimate is available,
// this is the size that is used when scheduling the task on an executor.
TaskSize default_task_size = 13;

// Task size measured from a previous task execution of a similar task, if
// such data is available.
//
// The scheduler may use this size to compute an adjusted `task_size` just
// before enqueueing a task onto an executor, but the adjusted size should not
// exceed the executor's limits.
TaskSize measured_task_size = 7;

// Task size computed via prediction model. This is only necessary when we
// a measured task size is not available.
//
// The scheduler may use this size to compute an adjusted `task_size` just
// before enqueueing a task onto an executor, but the adjusted size should not
// exceed the executor's limits.
// Task size computed via prediction model. This is only necessary when a
// measured task size is not available.
TaskSize predicted_task_size = 8;

// The resources explicitly requested by the user. These will be unset if the
Expand Down

0 comments on commit 369437d

Please sign in to comment.