Skip to content

PIR: Add support for multiple profile queries in PirScan and PirOptOut #6413

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

Merged
merged 6 commits into from
Jul 21, 2025

Conversation

landomen
Copy link
Contributor

@landomen landomen commented Jul 17, 2025

Task/Issue URL: https://app.asana.com/1/137249556945/project/1210594645151737/task/1210750227652612?focus=true

Description

Adds support for running the initial PIR scan and PIR opt-out scan on multiple stored profiles instead of the first stored profile. See the task for more details and a full tech spec.

Steps to test this PR

1 profile (current behavior)

  • Enter a new profile and trigger scan - scan runs for 1 profile
  • Go to opt-out scan and trigger it for any broker - scan runs for 1 profile

Multiple profiles (new behavior)

  • Clear data and trigger scan without any input - scan runs for 3 hardcoded profiles
  • Go to opt-out scan and trigger it for any broker - scan runs for all 3 profiles (assuming that the profiles were found for that broker)

UI changes

No UI Changes

@landomen landomen requested a review from karlenDimla July 17, 2025 13:58
@karlenDimla karlenDimla self-assigned this Jul 18, 2025
Comment on lines +24 to +38
val partSize = this.size / parts
val remainder = this.size % parts

val result = mutableListOf<List<T>>()
var startIndex = 0

for (i in 0 until parts) {
val currentPartSize = partSize + if (i < remainder) 1 else 0
val endIndex = startIndex + currentPartSize

result.add(this.subList(startIndex, endIndex))
startIndex = endIndex
}

return result
Copy link
Contributor Author

@landomen landomen Jul 21, 2025

Choose a reason for hiding this comment

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

I noticed that the previous implementation would not use all 20 runners in some cases. Example:

  • list size = 21
  • parts (max web views) = 20
  • result = 10 lists of 2 elements + 1 list of 1 element = 11 runners

The new implementation results in 1 list of 2 elements + 19 lists of 1 element = 20 runners

Comment on lines +226 to +241
// Load opt-out steps jsons for each broker
val brokerOptOutStepsJsons = brokers.mapNotNull { broker ->
repository.getBrokerOptOutSteps(broker)?.let { broker to it }
}

// Map broker steps with their associated profile queries
val allSteps = profileQueries.map { profileQuery ->
brokerOptOutStepsJsons.map { (broker, stepsJson) ->
brokerStepsParser.parseStep(broker, stepsJson, profileQuery.id)
}.flatten().map { step -> profileQuery to step }
}.flatten()

maxWebViewCount = if (brokerSteps.size <= MAX_DETACHED_WEBVIEW_COUNT) {
brokerSteps.size
} else {
MAX_DETACHED_WEBVIEW_COUNT
}
maxWebViewCount = minOf(allSteps.size, MAX_DETACHED_WEBVIEW_COUNT)

// Assign steps to runners based on the maximum number of WebViews we can use
val stepsPerRunner = allSteps.splitIntoParts(maxWebViewCount)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We now split the work between the maximum number of runners to max utilization.

@landomen landomen requested a review from karlenDimla July 21, 2025 06:57
@landomen
Copy link
Contributor Author

@karlenDimla I've updated the runner logic to run steps in parallel on maximum number of runners. The PR is ready for another review.

Copy link
Contributor

@karlenDimla karlenDimla left a comment

Choose a reason for hiding this comment

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

Tested and works as expected! Great work!

@landomen landomen merged commit 5caf9d2 into develop Jul 21, 2025
20 checks passed
@landomen landomen deleted the feature/domen/pir_scan_multiple_profile_queries branch July 21, 2025 12:59
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