[State Sync] Reduce logging for storage server responses.

666lcz · Dec 22, 2021 · f2481f6 · f2481f6
1 parent 372e031
commit f2481f6
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 5 deletions.
diff --git a/state-sync/storage-service/server/src/lib.rs b/state-sync/storage-service/server/src/lib.rs
@@ -19,7 +19,7 @@ use diem_types::{
 };
 use futures::stream::StreamExt;
 use serde::{Deserialize, Serialize};
-use std::sync::Arc;
+use std::{sync::Arc, time::Duration};
 use storage_interface::DbReader;
 use storage_service_types::{
     AccountStatesChunkWithProofRequest, CompleteDataRange, DataSummary,
@@ -39,6 +39,7 @@ mod tests;
 
 /// Storage server constants.
 pub const STORAGE_SERVER_VERSION: u64 = 1;
+const SUMMARY_LOG_FREQUENCY_SECS: u64 = 5;
 
 #[derive(Clone, Debug, Deserialize, Error, PartialEq, Serialize)]
 pub enum Error {
@@ -108,7 +109,7 @@ impl<T: StorageReaderInterface> StorageServiceServer<T> {
             self.bounded_executor
                 .spawn_blocking(move || {
                     let response = Handler::new(config, storage).call(protocol, request);
-                    debug!(LogSchema::new(LogEntry::SentStorageResponse).response(&response));
+                    log_storage_response(&response);
                     response_sender.send(response);
                 })
                 .await;
@@ -565,3 +566,31 @@ fn inclusive_range_len(start: u64, end: u64) -> Result<u64, Error> {
         .ok_or_else(|| Error::InvalidRequest(format!("end ({}) must not be u64::MAX", end)))?;
     Ok(len)
 }
+
+/// Logs the response sent by storage for a peer request
+fn log_storage_response(storage_response: &Result<StorageServiceResponse, StorageServiceError>) {
+    match storage_response {
+        Ok(storage_response) => {
+            let response = format!("{}", storage_response); // Use display formatting
+            if matches!(
+                storage_response,
+                StorageServiceResponse::StorageServerSummary(_)
+            ) {
+                // We expect peers to be polling our storage server summary frequently,
+                // so only log this response periodically.
+                sample!(
+                    SampleRate::Duration(Duration::from_secs(SUMMARY_LOG_FREQUENCY_SECS)),
+                    {
+                        debug!(LogSchema::new(LogEntry::SentStorageResponse).response(&response));
+                    }
+                );
+            } else {
+                debug!(LogSchema::new(LogEntry::SentStorageResponse).response(&response));
+            }
+        }
+        Err(storage_error) => {
+            let storage_error = format!("{:?}", storage_error); // Use debug formatting
+            debug!(LogSchema::new(LogEntry::SentStorageResponse).response(&storage_error));
+        }
+    };
+}
diff --git a/state-sync/storage-service/server/src/logging.rs b/state-sync/storage-service/server/src/logging.rs
@@ -4,14 +4,14 @@
 use crate::Error;
 use diem_logger::Schema;
 use serde::Serialize;
-use storage_service_types::{StorageServiceError, StorageServiceRequest, StorageServiceResponse};
+use storage_service_types::StorageServiceRequest;
 
 #[derive(Schema)]
 pub struct LogSchema<'a> {
     name: LogEntry,
     error: Option<&'a Error>,
     message: Option<&'a str>,
-    response: Option<&'a Result<StorageServiceResponse, StorageServiceError>>,
+    response: Option<&'a str>,
     request: Option<&'a StorageServiceRequest>,
 }
 

diff --git a/state-sync/storage-service/types/src/lib.rs b/state-sync/storage-service/types/src/lib.rs
@@ -17,7 +17,10 @@ use proptest::{
     strategy::{BoxedStrategy, Strategy},
 };
 use serde::{de, Deserialize, Serialize};
-use std::convert::TryFrom;
+use std::{
+    convert::TryFrom,
+    fmt::{Display, Formatter},
+};
 use thiserror::Error;
 
 /// A type alias for different epochs.
@@ -106,6 +109,24 @@ impl StorageServiceResponse {
     }
 }
 
+impl Display for StorageServiceResponse {
+    fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
+        // To prevent log spamming, we only display storage response data for summaries
+        let data = match self {
+            StorageServiceResponse::StorageServerSummary(storage_summary) => {
+                format!("{:?}", storage_summary)
+            }
+            _ => "...".into(),
+        };
+        write!(
+            f,
+            "Storage service response: {}, data: {}",
+            self.get_label(),
+            data
+        )
+    }
+}
+
 #[derive(Clone, Debug, Error)]
 #[error("unexpected response variant: {0}")]
 pub struct UnexpectedResponseError(pub String);