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 SubmitBatchRequests to use Coroutines #1467

Merged
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
9898ebf
Refactor SubmitBatchRequests to use Coroutines
tristanvuong2021 Feb 8, 2024
ae095fb
Merge branch 'main' into tristanvuong-change-submit-batch-requests-to…
tristanvuong2021 Feb 8, 2024
51faade
lint fix
tristanvuong2021 Feb 8, 2024
aaa1d70
lint fix
tristanvuong2021 Feb 8, 2024
ecf2ca1
Merge branch 'main' into tristanvuong-change-submit-batch-requests-to…
tristanvuong2021 Feb 12, 2024
200bb1b
Flow of lists is returned now.
tristanvuong2021 Feb 12, 2024
896ee81
lint fix
tristanvuong2021 Feb 12, 2024
18ba7ce
Merge branch 'main' into tristanvuong-change-submit-batch-requests-to…
tristanvuong2021 Feb 12, 2024
2a3392b
Clarify method comment
tristanvuong2021 Feb 12, 2024
e013889
Use flow as input
tristanvuong2021 Feb 14, 2024
c4856cd
lint fix
tristanvuong2021 Feb 14, 2024
a65dc67
Merge branch 'main' into tristanvuong-change-submit-batch-requests-to…
tristanvuong2021 Feb 14, 2024
d18475f
Merge branch 'main' into tristanvuong-change-submit-batch-requests-to…
tristanvuong2021 Feb 14, 2024
03a63aa
Merge branch 'main' into tristanvuong-change-submit-batch-requests-to…
tristanvuong2021 Feb 15, 2024
c45e7f9
Merge branch 'main' into tristanvuong-change-submit-batch-requests-to…
tristanvuong2021 Feb 16, 2024
990ae00
Merge branch 'main' into tristanvuong-change-submit-batch-requests-to…
tristanvuong2021 Feb 16, 2024
79442cb
Merge branch 'main' into tristanvuong-change-submit-batch-requests-to…
tristanvuong2021 Feb 16, 2024
7b1387e
Merge branch 'main' into tristanvuong-change-submit-batch-requests-to…
tristanvuong2021 Feb 20, 2024
d659037
Merge branch 'main' into tristanvuong-change-submit-batch-requests-to…
tristanvuong2021 Feb 22, 2024
0162ff0
Merge branch 'main' into tristanvuong-change-submit-batch-requests-to…
tristanvuong2021 Feb 26, 2024
dac02ed
Replace collect with transform
tristanvuong2021 Feb 26, 2024
265ac80
lint fix
tristanvuong2021 Feb 26, 2024
e1bb621
refactor
tristanvuong2021 Feb 26, 2024
72bc131
lint fix
tristanvuong2021 Feb 26, 2024
77b1120
lint fix
tristanvuong2021 Feb 26, 2024
f5b929c
Merge branch 'main' into tristanvuong-change-submit-batch-requests-to…
tristanvuong2021 Feb 27, 2024
a5349ca
Merge branch 'main' into tristanvuong-change-submit-batch-requests-to…
tristanvuong2021 Feb 28, 2024
fde15f8
Merge branch 'main' into tristanvuong-change-submit-batch-requests-to…
tristanvuong2021 Feb 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@

package org.wfanet.measurement.reporting.service.api

import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.Deferred
import kotlinx.coroutines.async
import kotlinx.coroutines.coroutineScope
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.asFlow
import kotlinx.coroutines.flow.flatMapConcat
import kotlinx.coroutines.flow.flow
import kotlinx.coroutines.sync.Semaphore
import kotlinx.coroutines.sync.withPermit

class BatchRequestException(message: String? = null, cause: Throwable? = null) :
Exception(message, cause)
Expand All @@ -47,20 +49,35 @@ fun <T> Flow<T>.chunked(chunkSize: Int): Flow<List<T>> {
}
}

/** Submits multiple RPCs by dividing the input items to batches. */
@OptIn(ExperimentalCoroutinesApi::class) // For `flatMapConcat`.
/**
* Submits multiple RPCs by dividing the input items to batches.
*
* @return [Flow] that emits [List]s containing the results of the multiple RPCs.
*/
suspend fun <ITEM, RESP, RESULT> submitBatchRequests(
items: Flow<ITEM>,
items: Collection<ITEM>,
limit: Int,
callRpc: suspend (List<ITEM>) -> RESP,
parseResponse: (RESP) -> List<RESULT>,
): Flow<RESULT> {
): Flow<List<RESULT>> {
if (limit <= 0) {
throw BatchRequestException(
"Invalid limit",
IllegalArgumentException("The size limit of a batch must be greater than 0."),
)
}

return items.chunked(limit).flatMapConcat { batch -> parseResponse(callRpc(batch)).asFlow() }
// For network requests, the number of concurrent coroutines needs to be capped. To be on the safe
// side, a low number is chosen.
val batchSemaphore = Semaphore(3)
return flow {
coroutineScope {
val deferred: List<Deferred<List<RESULT>>> =
items.chunked(limit).map { batch: List<ITEM> ->
async { batchSemaphore.withPermit { parseResponse(callRpc(batch)) } }
}

deferred.forEach { emit(it.await()) }
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,6 @@ import kotlinx.coroutines.asExecutor
import kotlinx.coroutines.async
import kotlinx.coroutines.awaitAll
import kotlinx.coroutines.coroutineScope
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.asFlow
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.toList
import kotlinx.coroutines.withContext
import org.jetbrains.annotations.BlockingExecutor
Expand Down Expand Up @@ -308,7 +305,7 @@ class MetricsService(
val measurementConsumer: MeasurementConsumer = getMeasurementConsumer(principal)

// Gets all external IDs of primitive reporting sets from the metric list.
val externalPrimitiveReportingSetIds: Flow<String> =
val externalPrimitiveReportingSetIds: List<String> =
internalMetricsList
.flatMap { internalMetric ->
internalMetric.weightedMeasurementsList.flatMap { weightedMeasurement ->
Expand All @@ -318,24 +315,27 @@ class MetricsService(
}
}
.distinct()
.asFlow()

val callBatchGetInternalReportingSetsRpc:
suspend (List<String>) -> BatchGetReportingSetsResponse =
{ items ->
batchGetInternalReportingSets(principal.resourceKey.measurementConsumerId, items)
}

val internalPrimitiveReportingSetMap: Map<String, InternalReportingSet> =
val internalPrimitiveReportingSetMap: Map<String, InternalReportingSet> = buildMap {
submitBatchRequests(
externalPrimitiveReportingSetIds,
BATCH_GET_REPORTING_SETS_LIMIT,
callBatchGetInternalReportingSetsRpc,
) { response: BatchGetReportingSetsResponse ->
response.reportingSetsList
}
.toList()
.associateBy { it.externalReportingSetId }
.collect { reportingSets: List<InternalReportingSet> ->
for (reportingSet in reportingSets) {
computeIfAbsent(reportingSet.externalReportingSetId) { reportingSet }
}
}
}

val dataProviderNames = mutableSetOf<String>()
for (internalPrimitiveReportingSet in internalPrimitiveReportingSetMap.values) {
Expand Down Expand Up @@ -372,14 +372,16 @@ class MetricsService(
batchCreateCmmsMeasurements(principal, items)
}

val cmmsMeasurements: Flow<Measurement> =
val cmmsMeasurements: List<Measurement> =
submitBatchRequests(
cmmsCreateMeasurementRequests.asFlow(),
BATCH_KINGDOM_MEASUREMENTS_LIMIT,
callBatchCreateMeasurementsRpc,
) { response: BatchCreateMeasurementsResponse ->
response.measurementsList
}
cmmsCreateMeasurementRequests,
BATCH_KINGDOM_MEASUREMENTS_LIMIT,
callBatchCreateMeasurementsRpc,
) { response: BatchCreateMeasurementsResponse ->
response.measurementsList
}
.toList()
.flatten()

// Set CMMS measurement IDs.
val callBatchSetCmmsMeasurementIdsRpc:
Expand All @@ -400,7 +402,7 @@ class MetricsService(
) { response: BatchSetCmmsMeasurementIdsResponse ->
response.measurementsList
}
.toList()
.collect {}
}

/** Sets a batch of CMMS [MeasurementIds] to the [InternalMeasurement] table. */
Expand Down Expand Up @@ -800,13 +802,13 @@ class MetricsService(
batchSetInternalMeasurementResults(items, apiAuthenticationKey, principal)
}
submitBatchRequests(
measurementsList.asFlow(),
measurementsList,
BATCH_SET_MEASUREMENT_RESULTS_LIMIT,
callBatchSetInternalMeasurementResultsRpc,
) { response: BatchSetCmmsMeasurementResultsResponse ->
response.measurementsList
}
.toList()
.collect {}

anyUpdate = true
}
Expand All @@ -823,13 +825,13 @@ class MetricsService(
)
}
submitBatchRequests(
measurementsList.asFlow(),
measurementsList,
BATCH_SET_MEASUREMENT_FAILURES_LIMIT,
callBatchSetInternalMeasurementFailuresRpc,
) { response: BatchSetCmmsMeasurementFailuresResponse ->
response.measurementsList
}
.toList()
.collect {}

anyUpdate = true
}
Expand Down Expand Up @@ -926,13 +928,14 @@ class MetricsService(
}

return submitBatchRequests(
measurementNames.asFlow(),
measurementNames,
BATCH_KINGDOM_MEASUREMENTS_LIMIT,
callBatchGetMeasurementsRpc,
) { response: BatchGetMeasurementsResponse ->
response.measurementsList
}
.toList()
.flatten()
}

/** Batch get CMMS measurements. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import java.time.ZonedDateTime
import java.time.temporal.TemporalAdjusters
import java.time.zone.ZoneRulesException
import kotlin.math.min
import kotlinx.coroutines.flow.asFlow
import org.projectnessie.cel.Env
import org.wfanet.measurement.api.v2alpha.DataProvider
import org.wfanet.measurement.api.v2alpha.DataProviderKey
Expand Down Expand Up @@ -673,31 +672,32 @@ class ReportSchedulesService(
while (externalReportingSetIdSet.isNotEmpty()) {
retrievedExternalReportingSetIdSet.addAll(externalReportingSetIdSet)

submitBatchRequests(
externalReportingSetIdSet.asFlow(),
BATCH_GET_REPORTING_SETS_LIMIT,
callRpc,
) { response ->
submitBatchRequests(externalReportingSetIdSet, BATCH_GET_REPORTING_SETS_LIMIT, callRpc) {
response ->
externalReportingSetIdSet.clear()
response.reportingSetsList
}
.collect {
if (it.hasComposite()) {
val lhsExternalReportingSetId = it.composite.lhs.externalReportingSetId
if (lhsExternalReportingSetId.isNotEmpty()) {
if (!retrievedExternalReportingSetIdSet.contains(lhsExternalReportingSetId)) {
externalReportingSetIdSet.add(lhsExternalReportingSetId)
.collect { internalReportingSets: List<InternalReportingSet> ->
for (internalReportingSet in internalReportingSets) {
if (internalReportingSet.hasComposite()) {
val lhsExternalReportingSetId =
internalReportingSet.composite.lhs.externalReportingSetId
if (lhsExternalReportingSetId.isNotEmpty()) {
if (!retrievedExternalReportingSetIdSet.contains(lhsExternalReportingSetId)) {
externalReportingSetIdSet.add(lhsExternalReportingSetId)
}
}
}

val rhsExternalReportingSetId = it.composite.rhs.externalReportingSetId
if (rhsExternalReportingSetId.isNotEmpty()) {
if (!retrievedExternalReportingSetIdSet.contains(rhsExternalReportingSetId)) {
externalReportingSetIdSet.add(rhsExternalReportingSetId)
val rhsExternalReportingSetId =
internalReportingSet.composite.rhs.externalReportingSetId
if (rhsExternalReportingSetId.isNotEmpty()) {
if (!retrievedExternalReportingSetIdSet.contains(rhsExternalReportingSetId)) {
externalReportingSetIdSet.add(rhsExternalReportingSetId)
}
}
}
reportingSets.add(internalReportingSet)
}
reportingSets.add(it)
}
}

Expand Down
Loading
Loading