Skip to content

Commit e239027

Browse files
ldanilekConvex, Inc.
authored andcommitted
delete TransactionIngredients (#28022)
the struct `TransactionIngredients` is only ever used to immediately construct a `Transaction`. let's skip that step, just construct Transaction directly. GitOrigin-RevId: d99f49901cbeb725f8c67f91d383dcb1d6e53bd5
1 parent cb7b237 commit e239027

File tree

2 files changed

+43
-69
lines changed

2 files changed

+43
-69
lines changed

crates/function_runner/src/in_memory_indexes.rs

Lines changed: 41 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -87,65 +87,42 @@ use crate::{
8787
FunctionWrites,
8888
};
8989

90-
/// Struct from which you can create a [Transaction].
91-
/// TODO: delete this in favor of using Transaction directly.
92-
pub struct TransactionIngredients<RT: Runtime> {
93-
pub ts: RepeatableTimestamp,
94-
pub identity: Identity,
95-
pub existing_writes: FunctionWrites,
96-
pub rt: RT,
97-
pub table_registry: TableRegistry,
98-
pub index_registry: IndexRegistry,
99-
pub table_count_snapshot: Arc<dyn TableCountSnapshot>,
100-
pub database_index_snapshot: DatabaseIndexSnapshot,
101-
pub text_index_snapshot: Arc<dyn TransactionTextSnapshot>,
102-
pub retention_validator: Arc<dyn RetentionValidator>,
103-
pub virtual_system_mapping: VirtualSystemMapping,
104-
pub usage_tracker: FunctionUsageTracker,
105-
}
106-
107-
impl<RT: Runtime> TryFrom<TransactionIngredients<RT>> for Transaction<RT> {
108-
type Error = anyhow::Error;
109-
110-
fn try_from(
111-
TransactionIngredients {
112-
ts,
113-
identity,
114-
existing_writes,
115-
rt,
116-
table_registry,
117-
index_registry,
118-
table_count_snapshot,
119-
database_index_snapshot,
120-
text_index_snapshot,
121-
retention_validator,
122-
virtual_system_mapping,
123-
usage_tracker,
124-
}: TransactionIngredients<RT>,
125-
) -> Result<Self, Self::Error> {
126-
let id_generator = TransactionIdGenerator::new(&rt)?;
127-
// The transaction timestamp might be few minutes behind if the backend
128-
// has been idle. Make sure creation time is always recent. Existing writes to
129-
// the transaction will advance next_creation_time in `merge_writes` below.
130-
let creation_time = CreationTime::try_from(cmp::max(*ts, rt.generate_timestamp()?))?;
131-
let transaction_index =
132-
TransactionIndex::new(index_registry, database_index_snapshot, text_index_snapshot);
133-
let mut tx = Transaction::new(
134-
identity,
135-
id_generator,
136-
creation_time,
137-
transaction_index,
138-
table_registry,
139-
table_count_snapshot,
140-
rt.clone(),
141-
usage_tracker,
142-
retention_validator,
143-
virtual_system_mapping,
144-
);
145-
let updates = existing_writes.updates;
146-
tx.merge_writes(updates, existing_writes.generated_ids)?;
147-
Ok(tx)
148-
}
90+
fn make_transaction<RT: Runtime>(
91+
ts: RepeatableTimestamp,
92+
identity: Identity,
93+
existing_writes: FunctionWrites,
94+
rt: RT,
95+
table_registry: TableRegistry,
96+
index_registry: IndexRegistry,
97+
table_count_snapshot: Arc<dyn TableCountSnapshot>,
98+
database_index_snapshot: DatabaseIndexSnapshot,
99+
text_index_snapshot: Arc<dyn TransactionTextSnapshot>,
100+
retention_validator: Arc<dyn RetentionValidator>,
101+
virtual_system_mapping: VirtualSystemMapping,
102+
usage_tracker: FunctionUsageTracker,
103+
) -> anyhow::Result<Transaction<RT>> {
104+
let id_generator = TransactionIdGenerator::new(&rt)?;
105+
// The transaction timestamp might be few minutes behind if the backend
106+
// has been idle. Make sure creation time is always recent. Existing writes to
107+
// the transaction will advance next_creation_time in `merge_writes` below.
108+
let creation_time = CreationTime::try_from(cmp::max(*ts, rt.generate_timestamp()?))?;
109+
let transaction_index =
110+
TransactionIndex::new(index_registry, database_index_snapshot, text_index_snapshot);
111+
let mut tx = Transaction::new(
112+
identity,
113+
id_generator,
114+
creation_time,
115+
transaction_index,
116+
table_registry,
117+
table_count_snapshot,
118+
rt.clone(),
119+
usage_tracker,
120+
retention_validator,
121+
virtual_system_mapping,
122+
);
123+
let updates = existing_writes.updates;
124+
tx.merge_writes(updates, existing_writes.generated_ids)?;
125+
Ok(tx)
149126
}
150127

151128
#[derive(Hash, PartialEq, Eq, Clone, Debug)]
@@ -367,7 +344,7 @@ impl<RT: Runtime> InMemoryIndexCache<RT> {
367344
text_index_snapshot: Arc<dyn TransactionTextSnapshot>,
368345
usage_tracker: FunctionUsageTracker,
369346
retention_validator: Arc<dyn RetentionValidator>,
370-
) -> anyhow::Result<TransactionIngredients<RT>> {
347+
) -> anyhow::Result<Transaction<RT>> {
371348
let _timer = begin_tx_timer();
372349
for (index_id, last_modified) in &in_memory_index_last_modified {
373350
anyhow::ensure!(
@@ -390,21 +367,20 @@ impl<RT: Runtime> InMemoryIndexCache<RT> {
390367
bootstrap_metadata,
391368
)
392369
.await?;
393-
let transaction_ingredients = TransactionIngredients {
370+
make_transaction(
394371
ts,
395372
identity,
396373
existing_writes,
397-
rt: self.rt.clone(),
374+
self.rt.clone(),
398375
table_registry,
399376
index_registry,
400377
table_count_snapshot,
401378
database_index_snapshot,
402379
text_index_snapshot,
403380
retention_validator,
404-
virtual_system_mapping: virtual_system_mapping(),
381+
virtual_system_mapping(),
405382
usage_tracker,
406-
};
407-
Ok(transaction_ingredients)
383+
)
408384
}
409385
}
410386

crates/function_runner/src/server.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ impl<RT: Runtime, S: StorageForInstance<RT>> FunctionRunnerCore<RT, S> {
309309
retention_validator: Arc<dyn RetentionValidator>,
310310
) -> anyhow::Result<Transaction<RT>> {
311311
let usage_tracker = FunctionUsageTracker::new();
312-
let transaction_ingredients = self
312+
let transaction = self
313313
.index_cache
314314
.begin_tx(
315315
identity.clone(),
@@ -325,7 +325,6 @@ impl<RT: Runtime, S: StorageForInstance<RT>> FunctionRunnerCore<RT, S> {
325325
retention_validator,
326326
)
327327
.await?;
328-
let transaction = transaction_ingredients.try_into()?;
329328
Ok(transaction)
330329
}
331330

@@ -371,7 +370,7 @@ impl<RT: Runtime, S: StorageForInstance<RT>> FunctionRunnerCore<RT, S> {
371370
Arc::new(FollowerRetentionManager::new(self.rt.clone(), reader.clone()).await?)
372371
},
373372
};
374-
let transaction_ingredients = self
373+
let mut transaction = self
375374
.index_cache
376375
.begin_tx(
377376
identity.clone(),
@@ -387,7 +386,6 @@ impl<RT: Runtime, S: StorageForInstance<RT>> FunctionRunnerCore<RT, S> {
387386
retention_validator,
388387
)
389388
.await?;
390-
let mut transaction = transaction_ingredients.try_into()?;
391389
let storage = self
392390
.storage
393391
.storage_for_instance(&mut transaction, StorageUseCase::Files)

0 commit comments

Comments
 (0)