You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The discussions in #735 and #841 are two symptoms of the same architectural problem: we do not have a single boundary that owns blocking SQLite execution, durable commit, in-memory NetworkState publication, and post-commit side effects.
Today EventProcessor::process_logs opens a transaction, processes logs one by one, calls database helpers during the batch, updates last_processed_block, and commits only at the end. During that window:
database helpers mutate the shared watched state before commit
notification behavior depends on whether a path uses silent modify_state or send_modify
In #735, the problem shows up as a correctness/coherence issue:
shared in-memory state is updated during batch processing
state publication is not cleanly tied to commit
side effects can escape before commit
In #841, the same problem shows up as an execution-model issue:
async code is still directly responsible for blocking SQLite writes
so we need ad hoc spawn_blocking fixes at individual call sites
The send_modify / modify_state split and the extra spawn_blocking calls are not the root cause. They are consequences of the same missing store boundary.
What a correct design needs
A single store boundary should own all of the following:
where blocking SQLite work runs
the atomic unit of persistence
publication of committed in-memory state
post-commit side effects
The invariant should be simple:
before commit, changes are local only
after commit, durable state and published in-memory state advance together
side effects happen only from committed state
Two coherent directions
1. Keep batch-level atomicity
In this model:
EventProcessor parses logs into batch-local domain changes
later logs validate against a batch-local staged NetworkState
NetworkDatabase owns the transaction internally
committed state is published exactly once, after commit
This preserves the current batch/block-style processing model, but it is inherently more complex because later events in a batch need to see earlier uncommitted changes.
2. Simplify to event-by-event commits
In this model:
each event validates against committed state
each event commits in its own transaction
NetworkState is updated only after commit
side effects run only after commit
This is much simpler, but it requires replacing the block-only progress cursor with a finer checkpoint such as (block_number, transaction_index, log_index). With only last_processed_block, replay after a crash can re-apply non-idempotent events.
Recommendation
Unless we have measured evidence that one transaction per event is too slow, the simpler event-by-event model looks like the better default.
The current design appears to be optimized around reducing transaction count, but I have not found evidence that this was measured and shown to be a real bottleneck. What is already clear is that the current approach has real correctness and maintenance costs.
This issue is about fixing that ownership model. Tactical changes like adjusting send_modify behavior or adding more spawn_blocking calls may be useful mitigations, but they do not solve the underlying problem.
Problem
The discussions in #735 and #841 are two symptoms of the same architectural problem: we do not have a single boundary that owns blocking SQLite execution, durable commit, in-memory
NetworkStatepublication, and post-commit side effects.Today
EventProcessor::process_logsopens a transaction, processes logs one by one, calls database helpers during the batch, updateslast_processed_block, and commits only at the end. During that window:modify_stateorsend_modifyspawn_blockingworkaroundThat makes the system hard to reason about because there is no simple answer to these questions:
watch<NetworkState>?Why #735 and #841 are the same issue
In #735, the problem shows up as a correctness/coherence issue:
In #841, the same problem shows up as an execution-model issue:
spawn_blockingfixes at individual call sitesThe
send_modify/modify_statesplit and the extraspawn_blockingcalls are not the root cause. They are consequences of the same missing store boundary.What a correct design needs
A single store boundary should own all of the following:
The invariant should be simple:
Two coherent directions
1. Keep batch-level atomicity
In this model:
EventProcessorparses logs into batch-local domain changesNetworkStateNetworkDatabaseowns the transaction internallyThis preserves the current batch/block-style processing model, but it is inherently more complex because later events in a batch need to see earlier uncommitted changes.
2. Simplify to event-by-event commits
In this model:
NetworkStateis updated only after commitThis is much simpler, but it requires replacing the block-only progress cursor with a finer checkpoint such as
(block_number, transaction_index, log_index). With onlylast_processed_block, replay after a crash can re-apply non-idempotent events.Recommendation
Unless we have measured evidence that one transaction per event is too slow, the simpler event-by-event model looks like the better default.
The current design appears to be optimized around reducing transaction count, but I have not found evidence that this was measured and shown to be a real bottleneck. What is already clear is that the current approach has real correctness and maintenance costs.
This issue is about fixing that ownership model. Tactical changes like adjusting
send_modifybehavior or adding morespawn_blockingcalls may be useful mitigations, but they do not solve the underlying problem.