Revert vid optimization (#3046)

* Revert "core, graph, store: Adjust vids of cached entities to reflect db changes"

This reverts commit 29cc7aa.

* Revert "store: Don't implement FromEntityData for Entity"

This reverts commit d8a8e36.

* Revert "graph, store: Include vid in entity modifications use in ClampRangeQuery"

This reverts commit 4396f8a.

* Revert "store: Simplify update_entity in relational test"

This reverts commit d8a89d0.

* Revert "core, graph: Track EntityVersion not Entity in the EntityCache"

This reverts commit 2289b27.

* Revert "all: Add an EntityVersion struct that contains an Entity and its vid"

This reverts commit adaab9f.

* store: Add 'allow(dead_code)' because of new Rust version

Author: evaporei

chain/ethereum/src/network_indexer/network_indexer.rs

@@ -311,7 +311,6 @@ fn load_parent_block_from_store(context: &Context, block_ptr: BlockPtr) -> Block
         .and_then(move |block| {
             future::result(
                 block
-                    .data
                     .get("parent")
                     .ok_or_else(move || {
                         anyhow!("block {} has no parent", block_ptr_for_missing_parent,)

core/src/subgraph/instance_manager.rs

@@ -4,7 +4,6 @@ use atomic_refcell::AtomicRefCell;
 use fail::fail_point;
 use graph::blockchain::{BlockchainKind, DataSource};
 use graph::data::store::scalar::Bytes;
-use graph::data::store::EntityVersion;
 use graph::data::subgraph::{UnifiedMappingApiVersion, MAX_SPEC_VERSION};
 use graph::prelude::TryStreamExt;
 use graph::prelude::{SubgraphInstanceManager as SubgraphInstanceManagerTrait, *};
@@ -73,7 +72,7 @@ struct IndexingState<T: RuntimeHostBuilder<C>, C: Blockchain> {
     instance: SubgraphInstance<C, T>,
     instances: SharedInstanceKeepAliveMap,
     filter: C::TriggerFilter,
-    entity_lfu_cache: LfuCache<EntityKey, Option<EntityVersion>>,
+    entity_lfu_cache: LfuCache<EntityKey, Option<Entity>>,
 }
 
 struct IndexingContext<T: RuntimeHostBuilder<C>, C: Blockchain> {
@@ -1067,7 +1066,7 @@ async fn process_block<T: RuntimeHostBuilder<C>, C: Blockchain>(
         data_sources,
         deterministic_errors,
     ) {
-        Ok(vid_map) => {
+        Ok(_) => {
             // For subgraphs with `nonFatalErrors` feature disabled, we consider
             // any error as fatal.
             //
@@ -1095,10 +1094,6 @@ async fn process_block<T: RuntimeHostBuilder<C>, C: Blockchain>(
                 return Err(BlockProcessingError::Canceled);
             }
 
-            // Adjust the vids of cached entities because inserts and
-            // updates will have changed them
-            ctx.state.entity_lfu_cache.update_vids(vid_map);
-
             Ok(needs_restart)
         }
 

graph/src/components/store.rs

@@ -553,7 +553,7 @@ impl<'a> FromIterator<&'a EntityModification> for StoreEvent {
             .map(|op| {
                 use self::EntityModification::*;
                 match op {
-                    Insert { key, .. } | Overwrite { key, .. } | Remove { key, .. } => {
+                    Insert { key, .. } | Overwrite { key, .. } | Remove { key } => {
                         EntityChange::for_data(key.clone())
                     }
                 }
@@ -1034,7 +1034,7 @@ pub trait WritableStore: Send + Sync + 'static {
     async fn supports_proof_of_indexing(&self) -> Result<bool, StoreError>;
 
     /// Looks up an entity using the given store key at the latest block.
-    fn get(&self, key: &EntityKey) -> Result<Option<EntityVersion>, StoreError>;
+    fn get(&self, key: &EntityKey) -> Result<Option<Entity>, StoreError>;
 
     /// Transact the entity changes from a single block atomically into the store, and update the
     /// subgraph block pointer to `block_ptr_to`, and update the firehose cursor to `firehose_cursor`
@@ -1048,14 +1048,14 @@ pub trait WritableStore: Send + Sync + 'static {
         stopwatch: StopwatchMetrics,
         data_sources: Vec<StoredDynamicDataSource>,
         deterministic_errors: Vec<SubgraphError>,
-    ) -> Result<Vec<(EntityKey, Vid)>, StoreError>;
+    ) -> Result<(), StoreError>;
 
     /// Look up multiple entities as of the latest block. Returns a map of
     /// entities by type.
     fn get_many(
         &self,
         ids_for_type: BTreeMap<&EntityType, Vec<&str>>,
-    ) -> Result<BTreeMap<EntityType, Vec<EntityVersion>>, StoreError>;
+    ) -> Result<BTreeMap<EntityType, Vec<Entity>>, StoreError>;
 
     /// The deployment `id` finished syncing, mark it as synced in the database
     /// and promote it to the current version in the subgraphs where it was the
@@ -1101,7 +1101,7 @@ mock! {
         fn get_many_mock<'a>(
             &self,
             _ids_for_type: BTreeMap<&'a EntityType, Vec<&'a str>>,
-        ) -> Result<BTreeMap<EntityType, Vec<EntityVersion>>, StoreError>;
+        ) -> Result<BTreeMap<EntityType, Vec<Entity>>, StoreError>;
     }
 }
 
@@ -1224,7 +1224,7 @@ impl WritableStore for MockStore {
         unimplemented!()
     }
 
-    fn get(&self, _: &EntityKey) -> Result<Option<EntityVersion>, StoreError> {
+    fn get(&self, _: &EntityKey) -> Result<Option<Entity>, StoreError> {
         unimplemented!()
     }
 
@@ -1236,14 +1236,14 @@ impl WritableStore for MockStore {
         _: StopwatchMetrics,
         _: Vec<StoredDynamicDataSource>,
         _: Vec<SubgraphError>,
-    ) -> Result<Vec<(EntityKey, Vid)>, StoreError> {
+    ) -> Result<(), StoreError> {
         unimplemented!()
     }
 
     fn get_many(
         &self,
         ids_for_type: BTreeMap<&EntityType, Vec<&str>>,
-    ) -> Result<BTreeMap<EntityType, Vec<EntityVersion>>, StoreError> {
+    ) -> Result<BTreeMap<EntityType, Vec<Entity>>, StoreError> {
         self.get_many_mock(ids_for_type)
     }
 
@@ -1457,20 +1457,16 @@ pub enum EntityModification {
     /// Insert the entity
     Insert { key: EntityKey, data: Entity },
     /// Update the entity by overwriting it
-    Overwrite {
-        key: EntityKey,
-        data: Entity,
-        prev_vid: Vid,
-    },
+    Overwrite { key: EntityKey, data: Entity },
     /// Remove the entity
-    Remove { key: EntityKey, prev_vid: Vid },
+    Remove { key: EntityKey },
 }
 
 impl EntityModification {
     pub fn entity_key(&self) -> &EntityKey {
         use EntityModification::*;
         match self {
-            Insert { key, .. } | Overwrite { key, .. } | Remove { key, .. } => key,
+            Insert { key, .. } | Overwrite { key, .. } | Remove { key } => key,
         }
     }
 
@@ -1533,7 +1529,7 @@ impl EntityOp {
 pub struct EntityCache {
     /// The state of entities in the store. An entry of `None`
     /// means that the entity is not present in the store
-    current: LfuCache<EntityKey, Option<EntityVersion>>,
+    current: LfuCache<EntityKey, Option<Entity>>,
 
     /// The accumulated changes to an entity.
     updates: HashMap<EntityKey, EntityOp>,
@@ -1562,7 +1558,7 @@ impl Debug for EntityCache {
 pub struct ModificationsAndCache {
     pub modifications: Vec<EntityModification>,
     pub data_sources: Vec<StoredDynamicDataSource>,
-    pub entity_lfu_cache: LfuCache<EntityKey, Option<EntityVersion>>,
+    pub entity_lfu_cache: LfuCache<EntityKey, Option<Entity>>,
 }
 
 impl EntityCache {
@@ -1579,7 +1575,7 @@ impl EntityCache {
 
     pub fn with_current(
         store: Arc<dyn WritableStore>,
-        current: LfuCache<EntityKey, Option<EntityVersion>>,
+        current: LfuCache<EntityKey, Option<Entity>>,
     ) -> EntityCache {
         EntityCache {
             current,
@@ -1615,10 +1611,7 @@ impl EntityCache {
 
     pub fn get(&mut self, key: &EntityKey) -> Result<Option<Entity>, QueryExecutionError> {
         // Get the current entity, apply any updates from `updates`, then from `handler_updates`.
-        let mut entity = self
-            .current
-            .get_entity(&*self.store, key)?
-            .map(|ev| ev.data);
+        let mut entity = self.current.get_entity(&*self.store, key)?;
         if let Some(op) = self.updates.get(key).cloned() {
             entity = op.apply_to(entity)
         }
@@ -1708,14 +1701,14 @@ impl EntityCache {
         }
 
         for (subgraph_id, keys) in missing_by_subgraph {
-            for (entity_type, evs) in self.store.get_many(keys)? {
-                for ev in evs {
+            for (entity_type, entities) in self.store.get_many(keys)? {
+                for entity in entities {
                     let key = EntityKey {
                         subgraph_id: subgraph_id.clone(),
                         entity_type: entity_type.clone(),
-                        entity_id: ev.data.id().unwrap(),
+                        entity_id: entity.id().unwrap(),
                     };
-                    self.current.insert(key, Some(ev));
+                    self.current.insert(key, Some(entity));
                 }
             }
         }
@@ -1730,47 +1723,33 @@ impl EntityCache {
                     // Merging with an empty entity removes null fields.
                     let mut data = Entity::new();
                     data.merge_remove_null_fields(updates);
-                    let ev = EntityVersion::new(data.clone(), None);
-                    self.current.insert(key.clone(), Some(ev));
+                    self.current.insert(key.clone(), Some(data.clone()));
                     Some(Insert { key, data })
                 }
                 // Entity may have been changed
                 (Some(current), EntityOp::Update(updates)) => {
-                    let mut data = current.data.clone();
+                    let mut data = current.clone();
                     data.merge_remove_null_fields(updates);
-                    let ev = EntityVersion::new(data.clone(), current.vid);
-                    self.current.insert(key.clone(), Some(ev));
-                    if current.data != data {
-                        Some(Overwrite {
-                            key,
-                            data,
-                            prev_vid: current.vid,
-                        })
+                    self.current.insert(key.clone(), Some(data.clone()));
+                    if current != data {
+                        Some(Overwrite { key, data })
                     } else {
                         None
                     }
                 }
                 // Entity was removed and then updated, so it will be overwritten
                 (Some(current), EntityOp::Overwrite(data)) => {
-                    let ev = EntityVersion::new(data.clone(), current.vid);
-                    self.current.insert(key.clone(), Some(ev));
-                    if current.data != data {
-                        Some(Overwrite {
-                            key,
-                            data,
-                            prev_vid: current.vid,
-                        })
+                    self.current.insert(key.clone(), Some(data.clone()));
+                    if current != data {
+                        Some(Overwrite { key, data })
                     } else {
                         None
                     }
                 }
                 // Existing entity was deleted
-                (Some(current), EntityOp::Remove) => {
+                (Some(_), EntityOp::Remove) => {
                     self.current.insert(key.clone(), None);
-                    Some(Remove {
-                        key,
-                        prev_vid: current.vid,
-                    })
+                    Some(Remove { key })
                 }
                 // Entity was deleted, but it doesn't exist in the store
                 (None, EntityOp::Remove) => None,
@@ -1787,39 +1766,26 @@ impl EntityCache {
     }
 }
 
-impl LfuCache<EntityKey, Option<EntityVersion>> {
+impl LfuCache<EntityKey, Option<Entity>> {
     // Helper for cached lookup of an entity.
     fn get_entity(
         &mut self,
         store: &(impl WritableStore + ?Sized),
         key: &EntityKey,
-    ) -> Result<Option<EntityVersion>, QueryExecutionError> {
+    ) -> Result<Option<Entity>, QueryExecutionError> {
         match self.get(key) {
             None => {
                 let mut entity = store.get(key)?;
                 if let Some(entity) = &mut entity {
                     // `__typename` is for queries not for mappings.
-                    entity.data.remove("__typename");
+                    entity.remove("__typename");
                 }
                 self.insert(key.clone(), entity.clone());
                 Ok(entity)
             }
             Some(data) => Ok(data.to_owned()),
         }
     }
-
-    /// Update the `vid` of cached entities to reflect changes made in the
-    /// database. When entities stay cached across insert/update operations,
-    /// their vid in the database changes as a result of these operations
-    /// and needs to be updated
-    pub fn update_vids(&mut self, vid_map: Vec<(EntityKey, Vid)>) {
-        for (key, vid) in vid_map {
-            assert!(vid.is_some());
-            if let Some(Some(x)) = self.get_mut(key) {
-                x.vid = vid;
-            }
-        }
-    }
 }
 
 /// Determines which columns should be selected in a table.

graph/src/components/subgraph/instance.rs

@@ -1,5 +1,4 @@
 use crate::blockchain::Blockchain;
-use crate::data::store::EntityVersion;
 use crate::prelude::*;
 use crate::util::lfu_cache::LfuCache;
 use crate::{components::store::WritableStore, data::subgraph::schema::SubgraphError};
@@ -28,7 +27,7 @@ pub struct BlockState<C: Blockchain> {
 impl<C: Blockchain> BlockState<C> {
     pub fn new(
         store: Arc<dyn WritableStore>,
-        lfu_cache: LfuCache<EntityKey, Option<EntityVersion>>,
+        lfu_cache: LfuCache<EntityKey, Option<Entity>>,
     ) -> Self {
         BlockState {
             entity_cache: EntityCache::with_current(store, lfu_cache),

graph/src/data/store/mod.rs

@@ -8,14 +8,14 @@ use itertools::Itertools;
 use serde::de;
 use serde::{Deserialize, Serialize};
 use stable_hash::prelude::*;
+use std::convert::TryFrom;
 use std::fmt;
 use std::iter::FromIterator;
 use std::str::FromStr;
 use std::{
     borrow::Cow,
     collections::{BTreeMap, HashMap},
 };
-use std::{convert::TryFrom, num::NonZeroU64};
 use strum::AsStaticRef as _;
 use strum_macros::AsStaticStr;
 
@@ -622,33 +622,6 @@ impl CacheWeight for Entity {
     }
 }
 
-pub type Vid = Option<NonZeroU64>;
-
-#[derive(Clone, Default, Debug)]
-pub struct EntityVersion {
-    pub data: Entity,
-    /// The `vid` of the entity if it exists in the store
-    pub vid: Vid,
-}
-
-impl EntityVersion {
-    pub fn new(data: Entity, vid: Vid) -> Self {
-        EntityVersion { data, vid }
-    }
-}
-
-impl From<EntityVersion> for Entity {
-    fn from(ev: EntityVersion) -> Self {
-        ev.data
-    }
-}
-
-impl CacheWeight for EntityVersion {
-    fn indirect_weight(&self) -> usize {
-        self.data.weight()
-    }
-}
-
 /// A value that can (maybe) be converted to an `Entity`.
 pub trait TryIntoEntity {
     fn try_into_entity(self) -> Result<Entity, Error>;

graph/src/util/lfu_cache.rs

@@ -87,7 +87,7 @@ impl<K: Clone + Ord + Eq + Hash + Debug + CacheWeight, V: CacheWeight + Default>
     /// Updates and bumps freceny if already present.
     pub fn insert(&mut self, key: K, value: V) {
         let weight = CacheEntry::weight(&key, &value);
-        match self.get_mut_entry(key.clone()) {
+        match self.get_mut(key.clone()) {
             None => {
                 self.total_weight += weight;
                 self.queue.push(
@@ -119,7 +119,7 @@ impl<K: Clone + Ord + Eq + Hash + Debug + CacheWeight, V: CacheWeight + Default>
             .unwrap_or(0)
     }
 
-    fn get_mut_entry(&mut self, key: K) -> Option<&mut CacheEntry<K, V>> {
+    fn get_mut(&mut self, key: K) -> Option<&mut CacheEntry<K, V>> {
         // Increment the frequency by 1
         let key_entry = CacheEntry::cache_key(key);
         self.queue
@@ -130,12 +130,8 @@ impl<K: Clone + Ord + Eq + Hash + Debug + CacheWeight, V: CacheWeight + Default>
         })
     }
 
-    pub fn get_mut(&mut self, key: K) -> Option<&mut V> {
-        self.get_mut_entry(key).map(|x| &mut x.value)
-    }
-
     pub fn get(&mut self, key: &K) -> Option<&V> {
-        self.get_mut_entry(key.clone()).map(|x| &x.value)
+        self.get_mut(key.clone()).map(|x| &x.value)
     }
 
     pub fn remove(&mut self, key: &K) -> Option<V> {