Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Replace sessions, kernels's status_history's type map with list #1662

Closed
wants to merge 39 commits into from

Conversation

jopemachine
Copy link
Member

@jopemachine jopemachine commented Oct 27, 2023

Refs #412, follow-up to #480.
Related issue: #1116.

The current implementation saves only the most recent timestamp whenever status information in status_history is updated, and all previous information is deleted.

This PR prevents the loss of timestamp information by changing the data structure of sessions, kernels's status_history to List.

Previous format:

{
  "PENDING": "2024-01-01T00:00:05Z",
  "SCHEDULED": "2024-01-01T00:00:10Z",
  "PREPARING": "2024-01-01T00:00:12Z",
  "RESTARTING": "2024-01-01T00:30:00Z",
  "RUNNING": "2024-01-01T00:30:08Z",  // overwritten if restarted
}

New format:

[
  {"status": "PENDING", "timestamp": "2024-01-01T00:00:05Z"},
  {"status": "SCHEDULED", "timestamp": "2024-01-01T00:00:05Z"},
  {"status": "PREPARING", "timestamp": "2024-01-01T00:00:05Z"},
  {"status": "RUNNING", "timestamp": "2024-01-01T00:00:15Z"},  // preserved
  {"status": "RESTARTING", "timestamp": "2024-01-01T00:30:00Z"},
  {"status": "RUNNING", "timestamp": "2024-01-01T00:30:08Z"},
]

Checklist: (if applicable)

  • Milestone metadata specifying the target backport version
  • API server-client counterparts (e.g., manager API -> client SDK)

📚 Documentation preview 📚: https://sorna--1662.org.readthedocs.build/en/1662/


📚 Documentation preview 📚: https://sorna-ko--1662.org.readthedocs.build/ko/1662/

@github-actions github-actions bot added comp:client Related to Client component comp:manager Related to Manager component size:L 100~500 LoC labels Oct 27, 2023
@jopemachine jopemachine changed the title fix: Change status_history from map to list fix: Change status_history from map to list Oct 27, 2023
@jopemachine jopemachine changed the title fix: Change status_history from map to list fix: Replace status_history's type map with list Oct 27, 2023
@jopemachine jopemachine added the require:db-migration Automatically set when alembic migrations are added or updated label Oct 27, 2023
@github-actions github-actions bot added size:XL 500~ LoC size:L 100~500 LoC and removed size:L 100~500 LoC size:XL 500~ LoC labels Oct 30, 2023
@achimnol achimnol added this to the 24.03 milestone Oct 31, 2023
@jopemachine
Copy link
Member Author

jopemachine commented Nov 3, 2023

Tested manually by writing some temporary client code.

    @api_function
    @pass_ctx_obj
    async def get_status_history(ctx: CLIContext, self):
        """
        Retrieves the status transition history of the compute session.
        """

        try:
            with Session() as session:
                fetch_func = lambda pg_offset, pg_size: session.ComputeSession.paginated_list(fields=(
                    session_fields["status_history"],
                    session_fields["status_history_log"],
                ))
                ctx.output.print_paginated_list(
                    fetch_func,
                    initial_page_offset=0,
                    page_size=20,
                )
        except Exception as e:
            ctx.output.print_error(e)
            sys.exit(ExitCode.FAILURE)

The result is as follows.

You can see status_history via graphene still use the same format, and the status_history_log uses the migrationed format.

스크린샷 2023-11-03 오후 12 10 39

@jopemachine jopemachine marked this pull request as ready for review November 6, 2023 01:59
@jopemachine jopemachine force-pushed the fix/status-history branch 2 times, most recently from f73d02d to a94fc0f Compare January 26, 2024 11:22
@jopemachine jopemachine changed the title fix: Replace status_history's type map with list fix: Replace session, kernel's status_history's type map with list Feb 28, 2024
@jopemachine jopemachine changed the title fix: Replace session, kernel's status_history's type map with list fix: Replace sessions, kernels's status_history's type map with list Feb 28, 2024
@achimnol
Copy link
Member

achimnol commented May 3, 2024

We also need to update clients using the session resource usage API handler's status_history field at:

"status_history": row["status_history"] or {},

as this does not go through the GraphQL's backward-compatible field generation.

@achimnol achimnol modified the milestones: 24.03, 24.09 May 3, 2024
@github-actions github-actions bot added area:docs Documentations comp:common Related to Common component comp:cli Related to CLI component labels May 3, 2024
tests/manager/models/test_utils.py Outdated Show resolved Hide resolved
src/ai/backend/manager/models/session.py Outdated Show resolved Hide resolved
changes/1662.fix.md Outdated Show resolved Hide resolved
@jopemachine jopemachine changed the title fix: Replace sessions, kernels's status_history's type map with list refactor: Replace sessions, kernels's status_history's type map with list May 7, 2024
@jopemachine
Copy link
Member Author

Switching to another PR as we introduce Graphite.
See #2116.

@jopemachine jopemachine closed this May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:docs Documentations comp:cli Related to CLI component comp:client Related to Client component comp:common Related to Common component comp:manager Related to Manager component require:db-migration Automatically set when alembic migrations are added or updated size:XL 500~ LoC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants