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

perf: improve wait for results performances by reducing the size of the request and making multiple at the same time #537

Merged
merged 1 commit into from
Aug 20, 2024

Conversation

aneojgurhem
Copy link
Contributor

No description provided.

Copy link

github-actions bot commented Aug 19, 2024

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
1174 920 78% 0% 🟢

New Files

No new covered files...

Modified Files

No covered modified files...

updated for commit: db2a4c4 by action🐍

Comment on lines +116 to +177
while (await streamingCall.ResponseStream.MoveNext(cancellationToken))
{
var resp = streamingCall.ResponseStream.Current;
if (resp.UpdateCase == EventSubscriptionResponse.UpdateOneofCase.ResultStatusUpdate &&
resultsNotFound.Contains(resp.ResultStatusUpdate.ResultId))
{
if (resp.ResultStatusUpdate.Status == ResultStatus.Completed)
{
SessionId = sessionId,
ReturnedEvents =
resultsNotFound.Remove(resp.ResultStatusUpdate.ResultId);
if (!resultsNotFound.Any())
{
EventsEnum.ResultStatusUpdate,
EventsEnum.NewResult,
},
ResultsFilters = new Filters
{
Or =
{
resultsNotFound.Select(ResultsFilter),
},
},
},
cancellationToken: cancellationToken);
try
{
while (await streamingCall.ResponseStream.MoveNext(cancellationToken))
{
var resp = streamingCall.ResponseStream.Current;
if (resp.UpdateCase == EventSubscriptionResponse.UpdateOneofCase.ResultStatusUpdate && resultsNotFound.Contains(resp.ResultStatusUpdate.ResultId))
{
if (resp.ResultStatusUpdate.Status == ResultStatus.Completed)
{
resultsNotFound.Remove(resp.ResultStatusUpdate.ResultId);
if (!resultsNotFound.Any())
{
break;
}
}
break;
}
}

if (resp.ResultStatusUpdate.Status == ResultStatus.Aborted)
{
throw new ResultAbortedException($"Result {resp.ResultStatusUpdate.ResultId} has been aborted");
}
}
if (resp.ResultStatusUpdate.Status == ResultStatus.Aborted)
{
throw new ResultAbortedException($"Result {resp.ResultStatusUpdate.ResultId} has been aborted");
}
}

if (resp.UpdateCase == EventSubscriptionResponse.UpdateOneofCase.NewResult && resultsNotFound.Contains(resp.NewResult.ResultId))
{
if (resp.NewResult.Status == ResultStatus.Completed)
{
resultsNotFound.Remove(resp.NewResult.ResultId);
if (!resultsNotFound.Any())
{
break;
}
}
if (resp.UpdateCase == EventSubscriptionResponse.UpdateOneofCase.NewResult &&
resultsNotFound.Contains(resp.NewResult.ResultId))
{
if (resp.NewResult.Status == ResultStatus.Completed)
{
resultsNotFound.Remove(resp.NewResult.ResultId);
if (!resultsNotFound.Any())
{
break;
}
}

if (resp.NewResult.Status == ResultStatus.Aborted)
{
throw new ResultAbortedException($"Result {resp.NewResult.ResultId} has been aborted");
}
}
}
}
catch (OperationCanceledException)
{
}
catch (RpcException)
{
}
}
}
if (resp.NewResult.Status == ResultStatus.Aborted)
{
throw new ResultAbortedException($"Result {resp.NewResult.ResultId} has been aborted");
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can handle both cases without repetition by extracting the "status" and "resultId" first depending on the event type, then do the checks and throw if necessary

…he request and making multiple at the same time
@aneojgurhem aneojgurhem merged commit 8b67c7d into main Aug 20, 2024
61 checks passed
@aneojgurhem aneojgurhem deleted the jg/wait branch August 20, 2024 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants