From 29b91a46f5196688fe35943f6b368a62e7589cb9 Mon Sep 17 00:00:00 2001 From: Theodore Schnepper Date: Tue, 26 Nov 2024 12:44:13 -0700 Subject: [PATCH] Fix missing time on first block The first block is always missing time at the moment, as the query can only compare the time to the previous row. Since we'll always be missing the previous row on the first entry, we end up missing the `time` for the first entry as well. The fix is simply to pull one more row than we want, and to remove the first entry when the time comes. As a result the `Vec` has been replaced with a `VecDeque`. Additionally the hard-coded values have been replaced with constants with comments. --- .../storage/sql/queries/explorer.rs | 51 +++++++++++++------ src/explorer/query_data.rs | 9 ++-- 2 files changed, 41 insertions(+), 19 deletions(-) diff --git a/src/data_source/storage/sql/queries/explorer.rs b/src/data_source/storage/sql/queries/explorer.rs index d3f936f5..14fb489a 100644 --- a/src/data_source/storage/sql/queries/explorer.rs +++ b/src/data_source/storage/sql/queries/explorer.rs @@ -39,7 +39,7 @@ use futures::stream::{self, StreamExt, TryStreamExt}; use hotshot_types::traits::node_implementation::NodeType; use itertools::Itertools; use sqlx::{types::Json, FromRow, Row}; -use std::num::NonZeroUsize; +use std::{collections::VecDeque, num::NonZeroUsize}; use tagged_base64::{Tagged, TaggedBase64}; impl From for GetExplorerSummaryError { @@ -251,6 +251,18 @@ lazy_static::lazy_static! { }; } +/// [EXPLORER_SUMMARY_HISTOGRAM_NUM_ENTRIES] is the number of entries we want +/// to return in our histogram summary. +const EXPLORER_SUMMARY_HISTOGRAM_NUM_ENTRIES: usize = 50; + +/// [EXPLORER_SUMMARY_NUM_BLOCKS] is the number of blocks we want to return in +/// our explorer summary. +const EXPLORER_SUMMARY_NUM_BLOCKS: usize = 10; + +/// [EXPLORER_SUMMARY_NUM_TRANSACTIONS] is the number of transactions we want +/// to return in our explorer summary. +const EXPLORER_SUMMARY_NUM_TRANSACTIONS: usize = 10; + #[async_trait] impl ExplorerStorage for Transaction where @@ -471,13 +483,14 @@ where JOIN payload AS p ON p.height = h.height WHERE - h.height IN (SELECT height FROM header ORDER BY height DESC LIMIT 50) + h.height IN (SELECT height FROM header ORDER BY height DESC LIMIT $1) ORDER BY h.height ", ) + .bind((EXPLORER_SUMMARY_HISTOGRAM_NUM_ENTRIES + 1) as i64) .fetch(self.as_mut()); - let histograms: Result = histogram_query_result + let mut histograms: ExplorerHistograms = histogram_query_result .map(|row_stream| { row_stream.map(|row| { let height: i64 = row.try_get("height")?; @@ -491,24 +504,32 @@ where }) .try_fold( ExplorerHistograms { - block_time: Vec::with_capacity(50), - block_size: Vec::with_capacity(50), - block_transactions: Vec::with_capacity(50), - block_heights: Vec::with_capacity(50), + block_time: VecDeque::with_capacity(EXPLORER_SUMMARY_HISTOGRAM_NUM_ENTRIES), + block_size: VecDeque::with_capacity(EXPLORER_SUMMARY_HISTOGRAM_NUM_ENTRIES), + block_transactions: VecDeque::with_capacity(EXPLORER_SUMMARY_HISTOGRAM_NUM_ENTRIES), + block_heights: VecDeque::with_capacity(EXPLORER_SUMMARY_HISTOGRAM_NUM_ENTRIES), }, |mut histograms: ExplorerHistograms, row: sqlx::Result<(i64, i64, Option, Option, i32)>| async { let (height, _timestamp, time, size, num_transactions) = row?; - histograms.block_time.push(time.map(|i| i as u64)); - histograms.block_size.push(size.map(|i| i as u64)); - histograms.block_transactions.push(num_transactions as u64); - histograms.block_heights.push(height as u64); + + histograms.block_time.push_back(time.map(|i| i as u64)); + histograms.block_size.push_back(size.map(|i| i as u64)); + histograms.block_transactions.push_back(num_transactions as u64); + histograms.block_heights.push_back(height as u64); Ok(histograms) }, ) - .await; + .await?; + + while histograms.block_time.len() > EXPLORER_SUMMARY_HISTOGRAM_NUM_ENTRIES { + histograms.block_time.pop_front(); + histograms.block_size.pop_front(); + histograms.block_transactions.pop_front(); + histograms.block_heights.pop_front(); + } - histograms? + histograms }; let genesis_overview = { @@ -528,7 +549,7 @@ where let latest_blocks: Vec> = self .get_block_summaries(GetBlockSummariesRequest(BlockRange { target: BlockIdentifier::Latest, - num_blocks: NonZeroUsize::new(10).unwrap(), + num_blocks: NonZeroUsize::new(EXPLORER_SUMMARY_NUM_BLOCKS).unwrap(), })) .await?; @@ -536,7 +557,7 @@ where .get_transaction_summaries(GetTransactionSummariesRequest { range: TransactionRange { target: TransactionIdentifier::Latest, - num_transactions: NonZeroUsize::new(10).unwrap(), + num_transactions: NonZeroUsize::new(EXPLORER_SUMMARY_NUM_TRANSACTIONS).unwrap(), }, filter: TransactionSummaryFilter::None, }) diff --git a/src/explorer/query_data.rs b/src/explorer/query_data.rs index 9b1a3eef..f4469e96 100644 --- a/src/explorer/query_data.rs +++ b/src/explorer/query_data.rs @@ -23,6 +23,7 @@ use crate::{node::BlockHash, types::HeightIndexed}; use hotshot_types::traits::node_implementation::NodeType; use serde::{Deserialize, Serialize}; use std::{ + collections::VecDeque, fmt::{Debug, Display}, num::{NonZeroUsize, TryFromIntError}, }; @@ -469,10 +470,10 @@ pub struct GenesisOverview { /// `block_transactions` for those `block_heights`. #[derive(Debug, Serialize, Deserialize)] pub struct ExplorerHistograms { - pub block_time: Vec>, - pub block_size: Vec>, - pub block_transactions: Vec, - pub block_heights: Vec, + pub block_time: VecDeque>, + pub block_size: VecDeque>, + pub block_transactions: VecDeque, + pub block_heights: VecDeque, } /// [ExplorerSummary] is a struct that represents an at-a-glance snapshot of