Thanks Matthias  a lot  for the comments and guidance.

>Can we reorganize the draft? Right now, we have some (for RescaleEvent, 
>Required/AcquiredParallelism) schema defined in the "Proposed Changes" section 
>and some other schema under "Public Interfaces". It would be nice to have this 
>more organized. Just as a suggestion: In the end the proposed changes should 
>list 
>the different REST endpoints you want to introduce (including the 
>corresponding 
>schemas for request and response).

Thanks for the comments. I’ve made some structural adjustments and content 
modifications to help clarify certain issues. Since some questions are still 
under discussion, I plan to update the document once we reach a consensus.

> I'm also wondering whether it would make sense to focus on the REST 
> endpoints in this FLIP and put the UI work in a separate FLIP. WDYT? 
> Decreasing 
> the scope would probably help handling the required changes. 

Nice proposal ! That will help us better focus our attention and concentrate on 
the main target.

> Have you considered adding the onChange event timestamp for a rescale event 
> as well? We introduced a separation of the job requirements change event
> and the actual rescale execution in FLIP-461 [1]. It might be worth
> documenting the time when a change was monitored for the first time that
> triggered the rescale. WDYT?

Thank you very much for your comments.As described in the document, the 
original design had many abstract concepts that were relatively simple. The 
information recorded for each rescale event mainly reflected the current state, 
rather than the entire historical state of the adaptive scheduler throughout 
the rescale process.
IIUC, when we add the onChange event timestamp for rescale events, each rescale 
event will capture the state changes of the adaptive scheduler at various 
stages (these states would form a collection similar to the set of all possible 
states of the adaptive scheduler) along with the corresponding timestamps. 
These states would essentially form an ordered collection. Compared to the 
original design, this would undoubtedly make internal information more 
transparent, which I think is very meaningful.
If I misunderstood, please correct me.

>You're mentioning "comments" as a field of the RescaleEvent in your proposal. 
>What's the use-case here? Where are these comments from? (update) A brief talk 
>with Yuepeng on that topic revealed that the field is supposed to be used for 
>errors that occurred during the rescale operation. My take on that one: 
>- We might want to reconsider the field name in that case (maybe 
>errors_during_rescale?). "comments" seems to be quite generic. 
> - Additionally, shouldn't we make this a list of errors rather than a String 
> field? 

Sorry for not fully explaining the purpose of this field earlier. The intention 
behind adding this field is to highlight special circumstances during a 
rescale, such as exceptions caused by insufficient resources or situations 
where a new request overrides the current one, forcing us to abandon the 
ongoing rescale operation and reallocate resources based on the new request. In 
such cases, it would be valuable to record what exactly happened internally. 
That said, I completely agree with the point that "comments" might seem overly 
broad.
Maybe we need to address a few key questions here:

1. Is it necessary to retain a "comments"-like field to capture such 
information?
2. Should error-related information also be retained?
3. If retained, where should the "comments" (if applicable) and error 
information fields be located?
    - Should they be tied to a specific adaptive-scheduler-state window?
    - Or should they be directly included in the rescale event message?
4. Should these fields be implemented as lists?

For the first and second items, I prefer to retain both types of information 
because they can provide transparent details to the user.
For the third and fourth items, I prefer to associate these two fields as lists 
with specific adaptive-scheduler state windows. This approach could help 
organize the information better and link it to relevant events in the rescale 
process.

I would greatly appreciate hearing more suggestions and ideas!

> - How certain are we that we can associate errors to the actual rescale
> operation and rather than the error being caused by something else?

In my limited reading, it seems that only exceptions related to resource 
shortage during parallelism allocation can be classified into this type of 
exception. Other exceptions can be categorized outside this scope.

> But there is no "attempt number" mentioned in the RescaleEvent schema. 

Please let me try to add to this question:
- Each rescale requirement corresponds to a resourceRequirementsEpochID. 
- For the same rescale requirement (resourceRequirementsEpochID), multiple 
rescale attempts may occur to here. We treat each rescale under the current 
resource requirements as a rescale attempt. That is, for each rescale attempt 
on the same rescale requirement (resourceRequirementsEpochID), the rescale 
attempt id will increment by 1. When the resourceRequirementsEpochID is 
changed, the rescale attempt will be reset.
- Based on these two points, whenever a rescale attempt happens, the rescaleID 
will increment globally.

I hope this may clarify the logic and approach. Please let me know what’s your 
opinion.

> Additionally, what is the ID based on? Do we start from 0 and just increment?
> Or do we want to have a mechanism that ensures that the IDs are also
> unique/monotonically increasing after JobManager failovers?

To be honest, I expect initially it to increment from 0 every time a job is 
started since the rescale ID is not strongly tied to the state. If we can 
ensure that the ID remains unique and increase monotonically even after a 
JobManager failover, this would make the rescale ID more consistent, right? If 
so, that makes sense to me. Please let me know your thoughts.

> For the parallelism schema: I might be misreading the draft here but you're 
> proposing to use the subtask 
> name as the ID to refer to the JobVertex? That the name might become quite 
> long. What about using the 
> JobVertexID here. That would be also more aligned to how the parallelism is 
> represented by the 
> /jobs/<job-id>/resource-requirements endpoint. If we want to add the task 
> name for readability purposes, we 
> can still add this one as a taskName field to the 
> Required/AcquiredParallelism schema.

Good proposal! I did consider using JobVertexID, but for ease of use, I ended 
up changing it to taskName. I'm wondering if we could use both JobVertexID and 
taskName. Could we provide a parameter to control the maximum length of 
taskName? In this way, we could ensure the uniqueness of JobVertexID while 
maintaining a certain level of readability for taskName. WDYTA?

> Status field: 
> - What is the meaning of "TRYING"? I guess, we're more or less using the 
> AdaptiveScheduler states here, aren't we? Can't we align/stick to the
> naming that's defined in the AdaptiveScheduler state?

As mentioned above, this type of state in the design is used to describe the 
current status of a rescale event. It is not equivalent to the state of the 
adaptive scheduler; rather, it represents a window just after receiving an 
update request but before reaching the next defined state.

> Can't we align/stick to the naming that's defined in the AdaptiveScheduler 
> state?

I previously conducted some research on the states of the AdaptiveScheduler 
[1]. Plase let me first try to define the necessary conditions for a Rescale 
Event:

- The process starting from WaitingForResources / CreatingExecutionGraph and 
ending at Executing / FINISHED is considered one rescale.
- If the process starts from WaitingForResources / CreatingExecutionGraph and 
ends at Executing, this can be considered a successful rescale.
- If the process starts from WaitingForResources / CreatingExecutionGraph and 
transitions directly to FINISHED for other reasons, it can be considered a 
failed rescale.
Based on these definitions, the adaptive-scheduler states that can be included 
in the rescale process seem to be Created, CreatingExecutionGraph, 
WaitingForResources, Executing, StopWithSavepoint, and Restarting.

If everything is aligned and we treat the AdaptiveScheduler states as 
equivalent to the RescaleEvent states, some of the states might seem a bit odd. 
For example, after a successful rescale, the final state of the rescale event 
would be Executing(as same as adaptive scheduler). Would we consider this 
situation reasonable? If so, Aligning the states would make sense to me.
Additionally, if the above definition holds, each rescale would need a "final 
state" or “current state” field to indicate its status.

Maybe it’s important to define and reach consensus on the rescale event and its 
start and end boundaries. 
Please correct me if I’m wrong. Thank you very much!


> Do we really need a new REST endpoint for the configuration? Can't we get 
> the provided information already from the existing configuration endpoint?

Thank you for your comments. Sorry for not finding a suitable existing REST 
interface to reuse. 
But there’s a part of ‘/jobs/:jobid/config‘ that could be reused at the backend 
side.
Another reason for introducing a new interface is to directly[2] retrieve the 
latest values from the Adaptive Scheduler, as we cannot guarantee that the 
configuration values used in the Adaptive Scheduler are always the ones 
specified in the configuration file. For example, some parameter values may be 
overwritten, which could change them. WDYTA?

> For the summary endpoint: I see similarities to the checkpoint summary
> here. Not sure whether you already considered that but would it make sense
> to align the field names in some way to have a consistent look-and-feel?
> I'm also wondering whether it makes sense to align the schema to have
> something like the latest rescale, failed rescale, ...

Thank you for the reminder. Yes, I have considered it. For example, I tried to 
align the request information in the REST interface with the checkpoint 
summary, as seen in this document[2]. However, I haven't strictly followed the 
same interface style and fields as the checkpoint REST interface, and instead 
used a simpler list (sorry, I should have shared these considerations in the 
draft earlier to give developers more context). If needed, I think it’s a great 
choice.

By the way, perhaps we should also consider the archive feature for the rescale 
history, to support viewing this information on the history server.

Thank you  very much.


[1] 
https://drive.google.com/file/d/1FE17cAfbUXypYBp0soaVLu5oqn39BoKI/view?usp=sharing
[2]https://docs.google.com/document/d/1WrLBkSkYe2tBQ3j66gKHFr2OB0d1HuHKDrRVr6B8nkM/edit?disco=AAABSsVvI6Q




On 2024/12/02 08:59:16 Matthias Pohl wrote:
> Hi Yuepeng,
> thanks for the proposal. Having a way to see the history of rescales is a
> nice feature, I guess. I went over the draft and have a few questions:
> 
> Can we reorganize the draft? Right now, we have some (for RescaleEvent,
> Required/AcquiredParallelism) schema defined in the "Proposed Changes"
> section and some other schema under "Public Interfaces". It would be nice
> to have this more organized.
> Just as a suggestion: In the end the proposed changes should list the
> different REST endpoints you want to introduce (including the corresponding
> schemas for request and response).
> ---
> I'm also wondering whether it would make sense to focus on the REST
> endpoints in this FLIP and put the UI work in a separate FLIP. WDYT?
> Decreasing the scope would probably help handling the required changes.
> ---
> Have you considered adding the onChange event timestamp for a rescale event
> as well? We introduced a separation of the job requirements change event
> and the actual rescale execution in FLIP-461 [1]. It might be worth
> documenting the time when a change was monitored for the first time that
> triggered the rescale. WDYT?
> ---
> You're mentioning "comments" as a field of the RescaleEvent in your
> proposal. What's the use-case here? Where are these comments from?
> 
> (update)
> A brief talk with Yuepeng on that topic revealed that the field is supposed
> to be used for errors that occurred during the rescale operation. My take
> on that one:
> - We might want to reconsider the field name in that case (maybe
> errors_during_rescale?). "comments" seems to be quite generic.
> - Additionally, shouldn't we make this a list of errors rather than a
> String field?
> - How certain are we that we can associate errors to the actual rescale
> operation and rather than the error being caused by something else?
> ---
> In the schema of the RescaleEvent you describe the three different
> ID/numbers in the following way:
> 
> > The ‘id’ is automatically incremental, The rescaleAttemptId is generated
> > based on one specified resource-requirement and the attempt number is
> > generated based on rescaleAttemptId.
> 
>  But there is no "attempt number" mentioned in the RescaleEvent schema.
> Additionally, what is the ID based on? Do we start from 0 and just
> increment? Or do we want to have a mechanism that ensures that the IDs are
> also unique/monotonically increasing after JobManager failovers?
> ---
> For the parallelism schema: I might be misreading the draft here but you're
> proposing to use the subtask name as the ID to refer to the JobVertex? That
> the name might become quite long. What about using the JobVertexID here.
> That would be also more aligned to how the parallelism is represented by
> the /jobs/<job-id>/resource-requirements endpoint. If we want to add the
> task name for readability purposes, we can still add this one as a taskName
> field to the Required/AcquiredParallelism schema.
> ---
> Status field:
> - What is the meaning of "TRYING"? I guess, we're more or less using the
> AdaptiveScheduler states here, aren't we? Can't we align/stick to the
> naming that's defined in the AdaptiveScheduler state?
> ---
> Do we really need a new REST endpoint for the configuration? Can't we get
> the provided information already from the existing configuration endpoint?
> That said, I still find it useful to have a config tab in the UI at the end.
> ---
> For the summary endpoint: I see similarities to the checkpoint summary
> here. Not sure whether you already considered that but would it make sense
> to align the field names in some way to have a consistent look-and-feel?
> I'm also wondering whether it makes sense to align the schema to have
> something like latest rescale, failed rescale, ...
> 
> Best,
> Matthias
> 
> [1]
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-461%3A+Synchronize+rescaling+with+checkpoint+creation+to+minimize+reprocessing+for+the+AdaptiveScheduler
> 
> On Mon, Nov 25, 2024 at 11:24 AM yuanfeng hu <yuanf...@apache.org> wrote:
> 
> > +1, I think this feature is very useful for adaptive scheduler.
> >
> > Yuepeng Pan <panyuep...@apache.org> 于2024年11月22日周五 18:38写道:
> >
> > > Hi community,
> > >
> > >
> > >
> > >
> > > Currently, the Adaptive Scheduler already supports the REST API
> > >
> > > to manually adjust[1] the parallelism of jobs, which enhances the
> > >
> > > functionality of the Adaptive Scheduler.
> > >
> > > However, Adaptive Scheduler doesn't support displaying or tracing the
> > > rescale history yet[2].
> > >
> > > This makes it inconvenient for users/devs to quickly obtain some internal
> > >
> > > information about the rescale history of the Adaptive Scheduler.
> > >
> > > And showing the history of rescale events of AdaptiveScheduler in the web
> > >
> > > UI is very useful for users to make the next step for jobs.
> > >
> > >
> > >
> > >
> > > Therefore, I created the FLIP-487[3] doc to support
> > >
> > > 'Show history of rescales in Web UI for AdaptiveScheduler'.
> > >
> > > Please refer to the google document[3] for more details
> > >
> > > about the proposed design and implementation.
> > >
> > >
> > >
> > >
> > > Looking forward to any feedback and opinions on this proposal.
> > >
> > >
> > >
> > >
> > > [1]
> > >
> > https://cwiki.apache.org/confluence/display/FLINK/FLIP-291%3A+Externalized+Declarative+Resource+Management
> > >
> > > [2] https://issues.apache.org/jira/browse/FLINK-22258
> > >
> > > [3]
> > >
> > https://docs.google.com/document/d/1WrLBkSkYe2tBQ3j66gKHFr2OB0d1HuHKDrRVr6B8nkM/edit?tab=t.0
> > >
> > >
> > >
> > >
> > > Thank you very much.
> > >
> > >
> > >
> > >
> > > Best,
> > >
> > > Regards.
> > >
> > > Yuepeng Pan
> >
> >
> >
> > --
> > Best,
> > Yuanfeng
> >
> 

Reply via email to