Skip to content

Commit 6ed4fff

Browse files
perf(log-ingestor): Reduce compression-submission lock contention with READ COMMITTED and smaller update batches (fixes y-scope#2358). (y-scope#2369)
1 parent 7b94e28 commit 6ed4fff

1 file changed

Lines changed: 101 additions & 48 deletions

File tree

components/log-ingestor/src/ingestion_job_manager/clp_ingestion.rs

Lines changed: 101 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use clp_rust_utils::{
2222
};
2323
use const_format::formatcp;
2424
use non_empty_string::NonEmptyString;
25-
use sqlx::MySqlPool;
25+
use sqlx::{Connection, MySqlPool};
2626
use strum_macros::{AsRefStr, Display, EnumIter, EnumString};
2727
use tokio::sync::mpsc;
2828

@@ -1071,8 +1071,9 @@ impl ClpCompressionState {
10711071
/// * [`anyhow::Error`] if one or more object metadata rows fail to be updated in the DB.
10721072
/// * Forwards [`clp_rust_utils::serde::BrotliMsgpack::serialize`]'s return values on failure.
10731073
/// * Forwards [`sqlx::query::Query::execute`]'s return values on failure.
1074-
/// * Forwards [`sqlx::Pool::begin`]'s return values on failure.
1074+
/// * Forwards [`sqlx::Connection::begin`]'s return values on failure.
10751075
/// * Forwards [`sqlx::Transaction::commit`]'s return values on failure.
1076+
/// * Forwards [`run_read_committed_tx`]'s return values on failure.
10761077
///
10771078
/// # Panics
10781079
///
@@ -1093,56 +1094,61 @@ impl ClpCompressionState {
10931094
}
10941095
};
10951096

1096-
let mut tx = self.db_pool.begin().await?;
1097+
// Run the submission transaction at `READ COMMITTED` to avoid the gap/next-key locks that
1098+
// serialize concurrent submissions under the default `REPEATABLE READ`.
1099+
run_read_committed_tx(self.db_pool.clone(), async |conn| {
1100+
let mut tx = Connection::begin(conn).await?;
10971101

1098-
// Submit compression job
1099-
let result = sqlx::query(COMPRESSION_JOB_SUBMISSION_QUERY)
1100-
.bind(clp_rust_utils::serde::BrotliMsgpack::serialize(&io_config)?)
1101-
.execute(&mut *tx)
1102-
.await?;
1103-
let compression_job_id =
1104-
CompressionJobId::try_from(result.last_insert_id()).map_err(|_| {
1105-
anyhow::anyhow!("The retrieved ID overflows: {}", result.last_insert_id())
1106-
})?;
1107-
1108-
// Update compression job ID for ingested objects.
1109-
// NOTE: We batch the update to avoid hitting the maximum placeholder limit of MySQL. The
1110-
// batch size is chosen to be 10000, which is conservative enough to avoid hitting the limit
1111-
// while also minimizing the number of batches for typical use cases. If the number of
1112-
// placeholders per update changes, we may need to adjust the batch size accordingly.
1113-
for chunk in object_metadata_ids.chunks(10000) {
1114-
let mut query_builder = sqlx::QueryBuilder::<sqlx::MySql>::new(formatcp!(
1115-
r"UPDATE `{table}` ",
1116-
table = INGESTED_S3_OBJECT_METADATA_TABLE_NAME,
1117-
));
1118-
query_builder
1119-
.push("SET `compression_job_id` = ")
1120-
.push_bind(compression_job_id);
1121-
query_builder
1122-
.push(", `status` = ")
1123-
.push_bind(IngestedS3ObjectMetadataStatus::Submitted);
1124-
query_builder.push(" WHERE `id` IN (");
1125-
let mut separated_ids = query_builder.separated(", ");
1126-
for id in chunk {
1127-
separated_ids.push_bind(id);
1128-
}
1129-
query_builder.push(")");
1130-
query_builder
1131-
.push(" AND `status` = ")
1132-
.push_bind(IngestedS3ObjectMetadataStatus::Buffered);
1133-
1134-
let result = query_builder.build().execute(&mut *tx).await?;
1135-
if result.rows_affected()
1136-
!= u64::try_from(chunk.len()).expect("size conversion should always succeed")
1137-
{
1138-
return Err(anyhow::anyhow!(
1139-
"Failed to update compression job ID for some objects."
1102+
// Submit compression job
1103+
let result = sqlx::query(COMPRESSION_JOB_SUBMISSION_QUERY)
1104+
.bind(clp_rust_utils::serde::BrotliMsgpack::serialize(&io_config)?)
1105+
.execute(&mut *tx)
1106+
.await?;
1107+
let compression_job_id =
1108+
CompressionJobId::try_from(result.last_insert_id()).map_err(|_| {
1109+
anyhow::anyhow!("The retrieved ID overflows: {}", result.last_insert_id())
1110+
})?;
1111+
1112+
// Update compression job ID for ingested objects.
1113+
// NOTE: We batch the update to avoid hitting the maximum placeholder limit of MySQL.
1114+
// The batch size of 1000 is conservative enough to avoid the limit while keeping each
1115+
// UPDATE's lock footprint small under concurrency. If the number of placeholders per
1116+
// update changes, we may need to adjust the batch size accordingly.
1117+
for chunk in object_metadata_ids.chunks(1000) {
1118+
let mut query_builder = sqlx::QueryBuilder::<sqlx::MySql>::new(formatcp!(
1119+
r"UPDATE `{table}` ",
1120+
table = INGESTED_S3_OBJECT_METADATA_TABLE_NAME,
11401121
));
1122+
query_builder
1123+
.push("SET `compression_job_id` = ")
1124+
.push_bind(compression_job_id);
1125+
query_builder
1126+
.push(", `status` = ")
1127+
.push_bind(IngestedS3ObjectMetadataStatus::Submitted);
1128+
query_builder.push(" WHERE `id` IN (");
1129+
let mut separated_ids = query_builder.separated(", ");
1130+
for id in chunk {
1131+
separated_ids.push_bind(id);
1132+
}
1133+
query_builder.push(")");
1134+
query_builder
1135+
.push(" AND `status` = ")
1136+
.push_bind(IngestedS3ObjectMetadataStatus::Buffered);
1137+
1138+
let result = query_builder.build().execute(&mut *tx).await?;
1139+
if result.rows_affected()
1140+
!= u64::try_from(chunk.len()).expect("size conversion should always succeed")
1141+
{
1142+
return Err(anyhow::anyhow!(
1143+
"Failed to update compression job ID for some objects."
1144+
));
1145+
}
11411146
}
1142-
}
11431147

1144-
tx.commit().await?;
1145-
Ok(compression_job_id)
1148+
tx.commit().await?;
1149+
Ok(compression_job_id)
1150+
})
1151+
.await
11461152
}
11471153

11481154
/// Waits for the compression job to finish and updates the status of submitted object metadata.
@@ -1509,6 +1515,53 @@ async fn update_job_status(
15091515
Ok(())
15101516
}
15111517

1518+
/// Runs `tx` on a freshly acquired pooled connection whose next transaction uses the `READ
1519+
/// COMMITTED` isolation level.
1520+
///
1521+
/// A `SET TRANSACTION ISOLATION LEVEL READ COMMITTED` statement is issued on the connection before
1522+
/// `tx` runs. Because it omits `SESSION`/`GLOBAL`, it applies only to the next transaction started
1523+
/// on that connection. `tx` is expected to begin exactly one transaction to consume the setting,
1524+
/// and to commit or roll it back itself.
1525+
///
1526+
/// If `tx` returns an error, the connection is detached from the pool and closed rather than being
1527+
/// released back into it. This ensures a failed attempt can never hand a later, unrelated borrower
1528+
/// a connection still carrying the pending isolation change (or a half-open transaction).
1529+
///
1530+
/// # Type Parameters
1531+
///
1532+
/// * `ReturnType` - The return type of `tx`.
1533+
/// * `TransactionType` - The type of `tx`, which is an async function that takes a mutable
1534+
/// reference to the connection.
1535+
///
1536+
/// # Returns
1537+
///
1538+
/// The value returned by `tx` on success.
1539+
///
1540+
/// # Errors
1541+
///
1542+
/// Returns an error if:
1543+
///
1544+
/// * Forwards [`sqlx::Pool::acquire`]'s return values on failure.
1545+
/// * Forwards [`sqlx::query::Query::execute`]'s return values on failure.
1546+
/// * Forwards `tx`'s return values on failure.
1547+
async fn run_read_committed_tx<ReturnType, TransactionType>(
1548+
pool: MySqlPool,
1549+
tx: TransactionType,
1550+
) -> anyhow::Result<ReturnType>
1551+
where
1552+
for<'connection_lifetime> TransactionType:
1553+
AsyncFnOnce(&'connection_lifetime mut sqlx::MySqlConnection) -> anyhow::Result<ReturnType>,
1554+
{
1555+
const SET_READ_COMMITTED: &str = "SET TRANSACTION ISOLATION LEVEL READ COMMITTED";
1556+
let mut conn = pool.acquire().await?;
1557+
sqlx::query(SET_READ_COMMITTED).execute(&mut *conn).await?;
1558+
let result = tx(&mut *conn).await;
1559+
if result.is_err() {
1560+
let _ = conn.detach().close().await;
1561+
}
1562+
result
1563+
}
1564+
15121565
#[cfg(test)]
15131566
mod tests {
15141567
use super::*;

0 commit comments

Comments
 (0)