fix: re-materialize views during procedure commit (re-entrant)#4299
Closed
clockwork-labs-bot wants to merge 2 commits intomasterfrom
Closed
fix: re-materialize views during procedure commit (re-entrant)#4299clockwork-labs-bot wants to merge 2 commits intomasterfrom
clockwork-labs-bot wants to merge 2 commits intomasterfrom
Conversation
When a procedure commits a transaction via procedure_commit_mut_tx, we now check for dirty views (views whose read sets overlap with the transaction's write set) and re-evaluate them re-entrantly before committing. This ensures that views are kept up-to-date when procedures modify data they depend on. Implementation details: - Store call_view/call_view_anon TypedFunc handles and module_def on WasmInstanceEnv for re-entrant access during procedure commits - Factor out evaluate_view_sql as a standalone pub(crate) function from InstanceCommon::run_query_for_view - In procedure_commit_mut_tx, iterate dirty views, put tx back in TxSlot for each view call, call view function re-entrantly via TypedFunc::call_async/now_or_never, process results (BSATN rows or RawSql), and materialize before final commit - Add set_module_def to WasmInstance trait for post-instantiation module definition injection - V8 backend: added TODO for equivalent implementation Fixes #4296
Mirrors the wasmtime re-entrant view evaluation in the V8 backend. During procedure_commit_mut_tx in V8: - Check tx.view_for_update() for dirty views - Get hooks via get_registered_hooks(scope) - For each dirty view, call call_call_view/call_call_view_anon re-entrantly through the V8 scope - Process results (Rows or RawSql) and materialize backing tables - Commit the transaction with view updates included Also adds module_def to JsInstanceEnv (set via set_module_def on the WasmInstance trait) so view definitions are available during the syscall. Part of #4296
Collaborator
|
Closing in favor of #4301 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Procedures that modify tables don't trigger view re-materialization (fixes #4296).
Root Cause
Procedures commit transactions via
procedure_commit_mut_txsyscall while executing inside the wasm/V8 instance. The reducer path callscall_views_with_tx()to re-materialize views before committing, but the procedure path skipped this entirely — views were never updated.Fix: Re-entrant View Evaluation
Both backends (wasmtime and V8) now evaluate dirty views re-entrantly during
procedure_commit_mut_tx, within the same transaction:tx.view_for_update()for views whose read sets overlap with the transaction's writesTypedFunc::call_async+now_or_never, V8:call_call_view/call_call_view_anonthrough the scope)RowsorRawSqlpaths) and materialize backing tables viaupdate_view_tableChanges
wasm_instance_env.rs(wasmtime):call_view,call_view_anon,module_deffields toWasmInstanceEnvprocedure_commit_mut_txwith re-entrant view evaluationprocess_view_result_and_materializehelperv8/syscall/common.rs(V8):procedure_commit_mut_txwith re-entrant view evaluation via hooksprocess_v8_view_resulthelperv8/mod.rs:module_deftoJsInstanceEnv, implementedset_module_defwasmtime_module.rs:WasmInstanceEnvduring instantiationmodule_host_actor.rs:evaluate_view_sqlas standalonepub(crate)functionset_module_deftoWasmInstancetraitdeserialize_view_rowspub(crate)instance_env.rs:finish_anon_tx()andprocedure_last_tx_offsetpub(crate)Closes #4296