Skip to content

Commit

Permalink
fix: cancelling session from sessions service cancel tasks and abort …
Browse files Browse the repository at this point in the history
…results from the session (#789)

# Motivation

Previously, cancelling a session from sessions service was only updating
the session status, leaving all its submitted and running tasks in
autoscaling count. This PR removes tasks of a cancelled session from the
autoscaling count by cancelling of the tasks and results from the
session.

# Description

When calling the Cancel RPC from the session service, we also change
task and results statuses in the cancelled session.

# Testing

On an ArmoniK instance, this new implementation was used and confirmed
to change the statuses.

# Impact

- It will reduce the computing resources consummed because the submitted
tasks from a cancelled session will change status to cancelling and not
count for autoscaling.
- If previously cancelling tasks are not removed from the queue during
downscaling, when a new session is submitted and upscaling occurs,
cancelling tasks will be removed first from the queue storage before new
tasks will be processed (at equal priority).

# Checklist

- [x] My code adheres to the coding and style guidelines of the project.
- [x] I have performed a self-review of my code.
- [ ] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have made corresponding changes to the documentation.
- [x] I have thoroughly tested my modifications and added tests when
necessary.
- [x] Tests pass locally and in the CI.
- [x] I have assessed the performance impact of my modifications.
  • Loading branch information
aneojgurhem authored Nov 4, 2024
2 parents 8f6a2f9 + 67b465f commit c956937
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 3 deletions.
23 changes: 23 additions & 0 deletions Common/src/Storage/ResultTableExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,29 @@ await resultTable.UpdateManyResults(result => result.OwnerTaskId == ownerTaskId,
ownerTaskId);
}

/// <summary>
/// Abort the results of the given session
/// </summary>
/// <param name="resultTable">Interface to manage results</param>
/// <param name="sessionId">id of the session containing the results</param>
/// <param name="cancellationToken">Token used to cancel the execution of the method</param>
/// <returns>
/// Task representing the asynchronous execution of the method
/// </returns>
public static async Task AbortSessionResults(this IResultTable resultTable,
string sessionId,
CancellationToken cancellationToken = default)
{
await resultTable.UpdateManyResults(result => result.SessionId == sessionId && result.Status == ResultStatus.Created,
new UpdateDefinition<Result>().Set(data => data.Status,
ResultStatus.Aborted),
cancellationToken)
.ConfigureAwait(false);

resultTable.Logger.LogDebug("Abort results from {session}",
sessionId);
}


/// <summary>
/// Updates in bulk results
Expand Down
13 changes: 10 additions & 3 deletions Common/src/gRPC/Services/GrpcSessionsService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,18 @@ public override async Task<CancelSessionResponse> CancelSession(CancelSessionReq
using var measure = meter_.CountAndTime();
try
{
var tasks = taskTable_.CancelSessionAsync(request.SessionId,
context.CancellationToken);
var results = resultTable_.AbortSessionResults(request.SessionId,
context.CancellationToken);
var sessions = sessionTable_.CancelSessionAsync(request.SessionId,
context.CancellationToken);

await tasks.ConfigureAwait(false);
await results.ConfigureAwait(false);
return new CancelSessionResponse
{
Session = (await sessionTable_.CancelSessionAsync(request.SessionId,
context.CancellationToken)
.ConfigureAwait(false)).ToGrpcSessionRaw(),
Session = (await sessions.ConfigureAwait(false)).ToGrpcSessionRaw(),
};
}
catch (SessionNotFoundException e)
Expand Down

0 comments on commit c956937

Please sign in to comment.