feature: add some exceptions when schedule computing session #1887

minseokey · 2024-02-05T07:14:05Z

This PR resolves #1814
add some exceptions when schedule computing session

When candidates_agent itself is empty.
When requested_architecture is empty.
When the agent's architecture and the requested architecture are different

it is possible to express the error situation in detail

Checklist: (if applicable)

Milestone metadata specifying the target backport version
Mention to the original issue
Installer updates including:
- Fixtures for db schema changes
- New mandatory config options
Update of end-to-end CLI integration tests in ai.backend.test
API server-client counterparts (e.g., manager API -> client SDK)
Test case(s) to:
- Demonstrate the difference of before/after
- Demonstrate the flow of abstract/conceptual models with a concrete implementation
Documentation
- Contents in the docs directory
- docstrings in public interfaces and type annotations

.feature.md -> 1887.feature.md

fregataa

Thanks! I left some comments

fregataa · 2024-02-14T04:48:39Z

src/ai/backend/manager/scheduler/dispatcher.py

+        if requested_architectures is None:
+            raise GenericBadRequest("Requested session has no architecture information")


This cannot happen because requested_architectures is always set, not None

fregataa · 2024-02-14T04:50:16Z

src/ai/backend/manager/scheduler/dispatcher.py

+            if candidate_agents is None:
+                raise InstanceNotAvailable(extra_msg=("No agents are registered with the manager"))


Same here. candidate_agents is always a list, not None

fregataa · 2024-02-14T04:53:10Z

src/ai/backend/manager/scheduler/dispatcher.py

+                        if candidate_agents is None:
+                            raise InstanceNotAvailable(
+                                extra_msg=("No agents are registered with the manager")
+                            )


Same here. candidate_agents cannot be None

fregataa · 2024-02-14T04:53:13Z

src/ai/backend/manager/scheduler/dispatcher.py

@@ -997,7 +1001,7 @@ async def _schedule_multi_node_session(
                                    AgentRow.occupied_slots,
                                ]).where(AgentRow.id == agent.id)
                            )
-                        ).fecthall()[0]
+                        ).fetchall()[0]


👍
Could you test whether this fix works well?

fregataa · 2024-02-16T02:26:52Z

src/ai/backend/manager/scheduler/dispatcher.py

+        if not requested_architectures:
+            raise GenericBadRequest("Requested session has no architecture information")


requested_architectures can be empty when the computing session does not have any sibling computing kernels.
How about checking session's sibling kernels instead of requested_architectures?
like this.

if not sess_ctx.kernels: raise GenericBadRequest(f"Given session does not have any sibling kernel. (id: {sess_ctx.id}")

fregataa · 2024-02-16T02:30:00Z

src/ai/backend/manager/scheduler/dispatcher.py

+            if not candidate_agents:
+                raise InstanceNotAvailable(extra_msg="No agents are registered with the manager")


👍
"No agents are available for scheduling" error msg would be more comprehensible

Could you check the error message again? The sentence "Registered with the manager" can confuse clients

fregataa · 2024-02-16T02:32:00Z

src/ai/backend/manager/scheduler/dispatcher.py

+                        if not candidate_agents:
+                            raise InstanceNotAvailable(
+                                extra_msg="No agents are registered with the manager"
+                            )


fregataa

Looks good!

fregataa · 2024-02-21T06:44:47Z

changes/1887.feature.md

@@ -0,0 +1 @@
+add some exceptions when schedule computing session


This will be logged on release note, so we should consider how users will read this and understand.
Something like "Eager error handling when scheduling compute session" would be readable for readers. We could use chatGPT or ask to other seniors for it !

fregataa

The new change log is good! I left some comments

fregataa · 2024-02-22T05:51:13Z

src/ai/backend/manager/scheduler/dispatcher.py

+            if not candidate_agents:
+                raise InstanceNotAvailable(extra_msg="No agents are registered with the manager")


Could you check the error message again? The sentence "Registered with the manager" can confuse clients

fregataa · 2024-02-22T05:51:16Z

src/ai/backend/manager/scheduler/dispatcher.py

+                        if not candidate_agents:
+                            raise InstanceNotAvailable(
+                                extra_msg="No agents are registered with the manager"
+                            )


src/ai/backend/manager/scheduler/dispatcher.py

…1887) Backported-from: main (24.03) Backported-to: 23.09 Co-authored-by: Joongi Kim <joongi@lablup.com>

minseokey added 2 commits February 5, 2024 06:52

feat: add some exceptions when schedule computing session

99c5eb4

feat: add some exceptions when schedule computing session

0cb6cda

minseokey requested review from achimnol, kyujin-cho and fregataa February 5, 2024 07:14

github-actions bot added the comp:manager Related to Manager component label Feb 5, 2024

github-actions bot assigned minseokey Feb 5, 2024

github-actions bot added size:S 10~30 LoC effort:easy Need to understand only a specific region of codes (good first issue, easy). labels Feb 5, 2024

docs: Rename the news fragment with the PR number

0f16ff8

.feature.md -> 1887.feature.md

minseokey changed the title ~~Feature/better log no agent schedule session~~ feature: add some exceptions when schedule computing session Feb 5, 2024

minseokey added size:S 10~30 LoC and removed size:S 10~30 LoC labels Feb 5, 2024

fregataa reviewed Feb 14, 2024

View reviewed changes

fix: Convert None to not

14ac1dd

minseokey requested a review from fregataa February 15, 2024 08:35

fregataa reviewed Feb 16, 2024

View reviewed changes

fix: check session's sibling kernels instead of requested_architectures

4c286eb

minseokey requested a review from fregataa February 19, 2024 06:59

fregataa reviewed Feb 21, 2024

View reviewed changes

fix: redeem .changes file

9c70e3a

minseokey requested a review from fregataa February 22, 2024 03:53

fregataa reviewed Feb 22, 2024

View reviewed changes

fix: change error message

83a8b53

achimnol reviewed Feb 22, 2024

View reviewed changes

src/ai/backend/manager/scheduler/dispatcher.py Outdated Show resolved Hide resolved

achimnol added 2 commits February 23, 2024 00:52

fix: Session/kernel relationship is not siblings but parent-child.

699cbc0

doc: Elaborate the news fragment

97756f4

achimnol approved these changes Feb 22, 2024

View reviewed changes

achimnol enabled auto-merge February 22, 2024 15:57

achimnol added this pull request to the merge queue Feb 22, 2024

Merged via the queue into main with commit 7831270 Feb 22, 2024
26 checks passed

achimnol deleted the feature/better-log-no-agent-schedule-session branch February 22, 2024 16:04

achimnol added this to the 23.09 milestone Feb 23, 2024

achimnol added a commit that referenced this pull request Feb 23, 2024

feature: add some exceptions when schedule computing session (#1401, #…

911927e

…1887) Backported-from: main (24.03) Backported-to: 23.09 Co-authored-by: Joongi Kim <joongi@lablup.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: add some exceptions when schedule computing session #1887

feature: add some exceptions when schedule computing session #1887

minseokey commented Feb 5, 2024 •

edited

Loading

fregataa left a comment

fregataa Feb 14, 2024

fregataa Feb 14, 2024

fregataa Feb 14, 2024

fregataa Feb 14, 2024

fregataa Feb 16, 2024

fregataa Feb 16, 2024

fregataa Feb 22, 2024

fregataa Feb 16, 2024

fregataa Feb 22, 2024

fregataa left a comment

fregataa Feb 21, 2024

fregataa left a comment

fregataa Feb 22, 2024

fregataa Feb 22, 2024

		if requested_architectures is None:
		raise GenericBadRequest("Requested session has no architecture information")

		if candidate_agents is None:
		raise InstanceNotAvailable(extra_msg=("No agents are registered with the manager"))

		if not requested_architectures:
		raise GenericBadRequest("Requested session has no architecture information")

		if not candidate_agents:
		raise InstanceNotAvailable(extra_msg="No agents are registered with the manager")

		@@ -0,0 +1 @@
		add some exceptions when schedule computing session

feature: add some exceptions when schedule computing session #1887

feature: add some exceptions when schedule computing session #1887

Conversation

minseokey commented Feb 5, 2024 • edited Loading

fregataa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fregataa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fregataa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minseokey commented Feb 5, 2024 •

edited

Loading