Anthony-Nolan / Atlas

A free & open-source Donor Search Algorithm Service
GNU General Public License v3.0
9 stars 5 forks source link

Performance improvements for downloading match prediction results in PersistSearchResults #938

Closed daria-sorokina-da closed 1 year ago

daria-sorokina-da commented 1 year ago

Currently, PersistSearchResults takes quite a long time, because it loads match prediction results sequentially, see: https://github.com/Anthony-Nolan/Atlas/blob/master/Atlas.Common/AzureStorage/Blob/BlobDownloader.cs#L93

foreach (var location in locations)
{
    data[location.Key] = await GetBlobData<T>(containerClient, location.Value);
}

In the search with 10000 donors, there will be 10000 sequential GetBlobData calls.

Let's investigate if: 1) variant 1 - create a new activity for saving individual blobs within atlas-search-results 2) variant 2 - if we can parallelize calls for GetBlobData in multuple threads https://github.com/Anthony-Nolan/Atlas/blob/master/Atlas.Common/AzureStorage/Blob/BlobDownloader.cs#L93

daria-sorokina-da commented 1 year ago

Performance measures: Search 22015ec9-9fc7-4242-bd35-69626159f00b performed 03 Apr 2023 Search step - 17 min Match Predictions step - 1 hour Persist Search Results step - 1 hour

daria-sorokina-da commented 1 year ago

@DmitriyShcherbina could you please prepare a search request for AN Dev env that would have some larger donors count (at least 1000)?

DmitriyShcherbina commented 1 year ago

@DmitriyShcherbina could you please prepare a search request for AN Dev env that would have some larger donors count (at least 1000)?

Here is Search Request, it should return 4963 donors

{ "SearchDonorType": 1, "MatchCriteria": { "DonorMismatchCount": 4, "LocusMismatchCriteria": { "A": 2, "B": 1, "C": 1, "Dpb1": null, "Dqb1": 0, "Drb1": 0 }, "includeBetterMatches": true }, "ScoringCriteria": { "LociToScore": [], "LociToExcludeFromAggregateScore": [] }, "PatientEthnicityCode": null, "PatientRegistryCode": null, "runMatchPrediction": false, "SearchHlaData":{ "A": { "Position1": "*01:DXGWF", "Position2": "*01:DXGWF" }, "B": { "Position1": "*07:DXFTK", "Position2": "*07:DXFTK" }, "C": { "Position1": "*05:DUVRN", "Position2": "*07:BRXNC" }, "DPB1": { "Position1": "*03:FYKD", "Position2": "*04:BYVXE" }, "DQB1": { "Position1": "*03:BMSUA", "Position2": "*06:BSBZX" }, "DRB1": { "Position1": "*15:01:01", "Position2": "*12:JV" } } }

zabeen commented 1 year ago

Testing Notes

@DmitriyShcherbina please re-run the same search parameters ran by @daria-sorokina-da for id 22015ec9-9fc7-4242-bd35-69626159f00b so we can compare the search times. That will also serve as a regression test for search in general. Thanks.

DmitriyShcherbina commented 1 year ago

@zabeen I didn't find any data in AN devatlasstorage related to 22015ec9-9fc7-4242-bd35-69626159f00b in order to retry it. If it was ran in wmda env - I don't have the access to those services to check that request data and retry it.

zabeen commented 1 year ago

@DmitriyShcherbina ok, fair enough, that ID will be used at wmda testing stage instead. Please go ahead and use the search request you shared above instead for AN Dev testing, thanks!

DmitriyShcherbina commented 1 year ago

@zabeen Search result is Ok "ElapsedTime": "00:00:22.6101124", "NumberOfResults": 4969

Should I perform some additional test for this ticket?

zabeen commented 1 year ago

@DmitriyShcherbina that's fine, thanks, I'll move ticket to next stage so we can confirm that match prediction times have gone down.

zabeen commented 1 year ago

@seanmobrien would you, or one of your team, please be able to test this on WMDA-DEV by re-running the same search Daria did above? The goal being to check that the PersistSearchResults time has gone down?

zabeen commented 1 year ago

@seanmobrien to re-run search 22015ec9-9fc7-4242-bd35-69626159f00b

seanmobrien commented 1 year ago

Search id 96759bd1-c7b7-419c-9093-d026d61df5a4 started at 2:47pm CST "MatchingAlgorithmTime": "00:10:28.0594495", "MatchPredictionTime": "01:57:16.0621773", Search Results written to blob: 5:04pm CST (2hrs 17mn run time)

zabeen commented 1 year ago

Testing notes: though it is not clear from re-run test whether the improvement made a significant change, we will be running more performance tests going forward and now have better logging in place thanks to #945 and #956, which will make it easier to identify any remaining performance bottenecks.