Implement properly AtomicView::latest_view
#1581
Assignees
Comments
xgreenx
added a commit
that referenced
this issue
Jan 19, 2024
Closes #1549 ## Overview The change extracts the off-chain-related logic from the executor and moves it to the GraphQL off-chain worker. It creates two new concepts - Off-chain and On-chain databases where the GraphQL worker has exclusive ownership of the database and may modify it without intersecting with the On-chain database. ## Challenges caused by the change Delegating updating of the state to something other than `BlockImporter` causes several new problems: - The commitment to the on-chain and off-chain databases is done in different places. The off-chain database may be out of sync with the on-chain database due to race conditions. - The result of the block execution(receipts, statuses) is not stored anywhere and may be lost due to emergency shutdown. We don't want to duplicate on-chain data inside of the off-chain database, so the GraphQL service works with two sources of data, which leads to two problems: - The off-chain database may be out of sync with the on-chain database due to race conditions causing failing requests. - The view of the databases during the GraphQL request may change, causing invalid responses with a mix of old and new data. We had this problem before, but now it is more critical. ## Solutions to the challenges ### Out of sync The change applies two steps to solve this issue. The main one is a new trait for the database: ```rust /// Provides a view of the storage at the given height. /// It guarantees to be atomic, meaning the view is immutable to outside modifications. pub trait AtomicView<View>: Send + Sync { /// Returns the view of the storage at the given `height`. fn view_at(&self, height: BlockHeight) -> StorageResult<View>; /// Returns the view of the storage for the latest block height. fn latest_view(&self) -> View; } ``` Another one to await on the `BlockCommiter` side finishing processing the `ImportResult` by all listeners. The goal of the trait is to provide an immutable read-only view of the database at a specific time. However, this trait has not yet been implemented properly during this PR and will be implemented in the following PRs. The `view_at` requires functionality from #451. We already can implement the `latest_view` method via [`RocksDB::Transaction`](https://github.com/facebook/rocksdb/wiki/Transactions#reading-from-a-transaction), but it is better to do it after merging #1576. Waiting on the `BlockImporter` side is a temporary solution to not escalate the problem. But maybe we can keep it later to guarantee the consistent state of the blockchain. ### Losing result of execution The `AtomicView` trait also solves the issue of losing the state of the execution because it is possible to get a view of the database at a specific block height and execute the block again receiving the same execution result. Waiting inside the `BlockImporter` guarantees that we will not lose more than one `ImportResult`. ### Inconsistent database view within GraphQL requests The GraphQL now has `ReadDatabase`: ```rust pub type OnChainView = Arc<dyn OnChainDatabase>; pub type OffChainView = Arc<dyn OffChainDatabase>; pub struct ReadDatabase { on_chain: Box<dyn AtomicView<OnChainView>>, off_chain: Box<dyn AtomicView<OffChainView>>, } ``` It implements the `view` method that returns the `ReadView` type. The `ReadView` implements all required methods by using internal on-chain view and off-chain view. The `AtomicView` allows us to get the `last_view` of the off-chain database and get the `view_at(off_chain_view.last_height())` of the on-chain database creating a consistent view for both databases at a specific height. The change also adds a `ViewExtension` to the GraphQL that creates a `ReadView` for each request. ```rust /// The extension that adds the `ReadView` to the request context. /// It guarantees that the request works with the one view of the database, /// and external database modification cannot affect the result. struct ViewExtension; #[async_trait::async_trait] impl Extension for ViewExtension { async fn prepare_request( &self, ctx: &ExtensionContext<'_>, request: Request, next: NextPrepareRequest<'_>, ) -> ServerResult<Request> { let database: &ReadDatabase = ctx.data_unchecked(); let view = database.view(); let request = request.data(view); next.run(ctx, request).await } } ``` ## Implementation details - The `ExecutionResult` now also has receipts for the transaction along with its status. The off-chain worker will insert them later into the database, while the `dry_run` can fetch them immediately. - All API requests now work with the `ReadView` instead of the `Database` type. The `ReadDatabase` is only used in one place in the `ViewExtension`. - The `BlockImpoerter::comit_result` now is `async` and awaits for the previous block to be processed by all listeners. The execution of the `execute_and_commit` now runs `verify_and_execute_block` in the spawned task in the `tokio_rayon`. ## Follow up - #1580 - #1581 - #1582 - #1583 - #1584
3 tasks
GoldenPath1109
added a commit
to GoldenPath1109/fuel-core
that referenced
this issue
Sep 7, 2024
Closes FuelLabs/fuel-core#1549 ## Overview The change extracts the off-chain-related logic from the executor and moves it to the GraphQL off-chain worker. It creates two new concepts - Off-chain and On-chain databases where the GraphQL worker has exclusive ownership of the database and may modify it without intersecting with the On-chain database. ## Challenges caused by the change Delegating updating of the state to something other than `BlockImporter` causes several new problems: - The commitment to the on-chain and off-chain databases is done in different places. The off-chain database may be out of sync with the on-chain database due to race conditions. - The result of the block execution(receipts, statuses) is not stored anywhere and may be lost due to emergency shutdown. We don't want to duplicate on-chain data inside of the off-chain database, so the GraphQL service works with two sources of data, which leads to two problems: - The off-chain database may be out of sync with the on-chain database due to race conditions causing failing requests. - The view of the databases during the GraphQL request may change, causing invalid responses with a mix of old and new data. We had this problem before, but now it is more critical. ## Solutions to the challenges ### Out of sync The change applies two steps to solve this issue. The main one is a new trait for the database: ```rust /// Provides a view of the storage at the given height. /// It guarantees to be atomic, meaning the view is immutable to outside modifications. pub trait AtomicView<View>: Send + Sync { /// Returns the view of the storage at the given `height`. fn view_at(&self, height: BlockHeight) -> StorageResult<View>; /// Returns the view of the storage for the latest block height. fn latest_view(&self) -> View; } ``` Another one to await on the `BlockCommiter` side finishing processing the `ImportResult` by all listeners. The goal of the trait is to provide an immutable read-only view of the database at a specific time. However, this trait has not yet been implemented properly during this PR and will be implemented in the following PRs. The `view_at` requires functionality from FuelLabs/fuel-core#451. We already can implement the `latest_view` method via [`RocksDB::Transaction`](https://github.com/facebook/rocksdb/wiki/Transactions#reading-from-a-transaction), but it is better to do it after merging FuelLabs/fuel-core#1576. Waiting on the `BlockImporter` side is a temporary solution to not escalate the problem. But maybe we can keep it later to guarantee the consistent state of the blockchain. ### Losing result of execution The `AtomicView` trait also solves the issue of losing the state of the execution because it is possible to get a view of the database at a specific block height and execute the block again receiving the same execution result. Waiting inside the `BlockImporter` guarantees that we will not lose more than one `ImportResult`. ### Inconsistent database view within GraphQL requests The GraphQL now has `ReadDatabase`: ```rust pub type OnChainView = Arc<dyn OnChainDatabase>; pub type OffChainView = Arc<dyn OffChainDatabase>; pub struct ReadDatabase { on_chain: Box<dyn AtomicView<OnChainView>>, off_chain: Box<dyn AtomicView<OffChainView>>, } ``` It implements the `view` method that returns the `ReadView` type. The `ReadView` implements all required methods by using internal on-chain view and off-chain view. The `AtomicView` allows us to get the `last_view` of the off-chain database and get the `view_at(off_chain_view.last_height())` of the on-chain database creating a consistent view for both databases at a specific height. The change also adds a `ViewExtension` to the GraphQL that creates a `ReadView` for each request. ```rust /// The extension that adds the `ReadView` to the request context. /// It guarantees that the request works with the one view of the database, /// and external database modification cannot affect the result. struct ViewExtension; #[async_trait::async_trait] impl Extension for ViewExtension { async fn prepare_request( &self, ctx: &ExtensionContext<'_>, request: Request, next: NextPrepareRequest<'_>, ) -> ServerResult<Request> { let database: &ReadDatabase = ctx.data_unchecked(); let view = database.view(); let request = request.data(view); next.run(ctx, request).await } } ``` ## Implementation details - The `ExecutionResult` now also has receipts for the transaction along with its status. The off-chain worker will insert them later into the database, while the `dry_run` can fetch them immediately. - All API requests now work with the `ReadView` instead of the `Database` type. The `ReadDatabase` is only used in one place in the `ViewExtension`. - The `BlockImpoerter::comit_result` now is `async` and awaits for the previous block to be processed by all listeners. The execution of the `execute_and_commit` now runs `verify_and_execute_block` in the spawned task in the `tokio_rayon`. ## Follow up - FuelLabs/fuel-core#1580 - FuelLabs/fuel-core#1581 - FuelLabs/fuel-core#1582 - FuelLabs/fuel-core#1583 - FuelLabs/fuel-core#1584
GoldenPath1109
added a commit
to GoldenPath1109/fuel-core
that referenced
this issue
Sep 7, 2024
…994) Closes FuelLabs/fuel-core#1581 The change finally implements an atomic view of the RocksDB database via the snapshot feature. We use a snapshot (if available) each time we read/iterate over the RocksDB. It guarantees a consistent view even if the data is removed/added/modified. `InMemory` database also implements this feature by simply cloning all `BTreeMap`s. In the future, we need to optimize this behavior and cache views - FuelLabs/fuel-core#1993. ## Checklist - [x] New behavior is reflected in tests ### Before requesting review - [x] I have reviewed the code myself - [x] I have created follow-up issues caused by this PR and linked them here
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Implement
AtomicView::latest_viewusing RockDB read transaction that guarantees state consistency. In the case ofInMemorydatabase we can simply create a clone of the database and store it inside of theArc.The change is better to implement after #1576
The text was updated successfully, but these errors were encountered: