From 0f770967b4c8bbea5f62492d4dca874695376923 Mon Sep 17 00:00:00 2001 From: Max Sharnoff Date: Fri, 24 Sep 2021 10:18:36 -0700 Subject: [PATCH] Revert "Allow `LeSer`/`BeSer` impls missing either `Serialize` or `Deserialize` (#655) This reverts commit bd9f4794d9e36be920f6172001e920e9b43e55fe. --- pageserver/src/walredo.rs | 4 +- proxy/src/mgmt.rs | 6 +-- walkeeper/src/safekeeper.rs | 14 +++--- zenith_utils/src/bin_ser.rs | 81 +++++++----------------------- zenith_utils/tests/bin_ser_test.rs | 4 +- 5 files changed, 33 insertions(+), 76 deletions(-) diff --git a/pageserver/src/walredo.rs b/pageserver/src/walredo.rs index d1c2b9ae5a..fbb1984859 100644 --- a/pageserver/src/walredo.rs +++ b/pageserver/src/walredo.rs @@ -22,7 +22,7 @@ use byteorder::{ByteOrder, LittleEndian}; use bytes::{Buf, BufMut, Bytes, BytesMut}; use lazy_static::lazy_static; use log::*; -use serde::Serialize; +use serde::{Deserialize, Serialize}; use std::fs; use std::fs::OpenOptions; use std::io::prelude::*; @@ -59,7 +59,7 @@ use postgres_ffi::XLogRecord; /// In Postgres `BufferTag` structure is used for exactly the same purpose. /// [See more related comments here](https://github.com/postgres/postgres/blob/99c5852e20a0987eca1c38ba0c09329d4076b6a0/src/include/storage/buf_internals.h#L91). /// -#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Serialize)] +#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy, Serialize, Deserialize)] pub struct BufferTag { pub rel: RelTag, pub blknum: u32, diff --git a/proxy/src/mgmt.rs b/proxy/src/mgmt.rs index 90fccd770d..0d3c114421 100644 --- a/proxy/src/mgmt.rs +++ b/proxy/src/mgmt.rs @@ -5,7 +5,7 @@ use std::{ use anyhow::bail; use bytes::Bytes; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; use zenith_utils::{ postgres_backend::{self, query_from_cstring, AuthType, PostgresBackend}, pq_proto::{BeMessage, SINGLE_COL_ROWDESC}, @@ -60,13 +60,13 @@ struct MgmtHandler { // "Failure": "oops" // } // } -#[derive(Deserialize)] +#[derive(Serialize, Deserialize)] pub struct PsqlSessionResponse { session_id: String, result: PsqlSessionResult, } -#[derive(Deserialize)] +#[derive(Serialize, Deserialize)] pub enum PsqlSessionResult { Success(DatabaseInfo), Failure(String), diff --git a/walkeeper/src/safekeeper.rs b/walkeeper/src/safekeeper.rs index 0b5c13bb84..bffc329278 100644 --- a/walkeeper/src/safekeeper.rs +++ b/walkeeper/src/safekeeper.rs @@ -111,7 +111,7 @@ impl Default for SafeKeeperState { // protocol messages /// Initial Proposer -> Acceptor message -#[derive(Debug, Deserialize)] +#[derive(Debug, Serialize, Deserialize)] pub struct ProposerGreeting { /// proposer-acceptor protocol version pub protocol_version: u32, @@ -128,19 +128,19 @@ pub struct ProposerGreeting { /// Acceptor -> Proposer initial response: the highest term known to me /// (acceptor voted for). -#[derive(Debug, Serialize)] +#[derive(Debug, Serialize, Deserialize)] pub struct AcceptorGreeting { term: u64, } /// Vote request sent from proposer to safekeepers -#[derive(Debug, Deserialize)] +#[derive(Debug, Serialize, Deserialize)] pub struct VoteRequest { term: Term, } /// Vote itself, sent from safekeeper to proposer -#[derive(Debug, Serialize)] +#[derive(Debug, Serialize, Deserialize)] pub struct VoteResponse { term: Term, // not really needed, just a sanity check vote_given: u64, // fixme u64 due to padding @@ -152,12 +152,12 @@ pub struct VoteResponse { /// Request with WAL message sent from proposer to safekeeper. Along the way it /// announces 1) successful election (with epoch_start_lsn); 2) commit_lsn. -#[derive(Debug)] +#[derive(Debug, Serialize, Deserialize)] pub struct AppendRequest { pub h: AppendRequestHeader, pub wal_data: Bytes, } -#[derive(Debug, Clone, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize)] pub struct AppendRequestHeader { pub term: Term, // LSN since the proposer appends WAL; determines epoch switch point. @@ -175,7 +175,7 @@ pub struct AppendRequestHeader { } /// Report safekeeper state to proposer -#[derive(Debug, PartialEq, Serialize)] +#[derive(Debug, PartialEq, Serialize, Deserialize)] pub struct AppendResponse { // Current term of the safekeeper; if it is higher than proposer's, the // compute is out of date. diff --git a/zenith_utils/src/bin_ser.rs b/zenith_utils/src/bin_ser.rs index 063d69557d..0b78265dfc 100644 --- a/zenith_utils/src/bin_ser.rs +++ b/zenith_utils/src/bin_ser.rs @@ -94,12 +94,9 @@ pub fn le_coder() -> impl Options { /// Binary serialize/deserialize helper functions (Big Endian) /// -pub trait BeSer { +pub trait BeSer: Serialize + DeserializeOwned { /// Serialize into a byte slice - fn ser_into_slice(&self, mut b: &mut [u8]) -> Result<(), SerializeError> - where - Self: Serialize, - { + fn ser_into_slice(&self, mut b: &mut [u8]) -> Result<(), SerializeError> { // &mut [u8] implements Write, but `ser_into` needs a mutable // reference to that. So we need the slightly awkward "mutable // reference to a mutable reference. @@ -110,28 +107,19 @@ pub trait BeSer { /// /// This is useful for most `Write` types except `&mut [u8]`, which /// can more easily use [`ser_into_slice`](Self::ser_into_slice). - fn ser_into(&self, w: &mut W) -> Result<(), SerializeError> - where - Self: Serialize, - { + fn ser_into(&self, w: &mut W) -> Result<(), SerializeError> { be_coder().serialize_into(w, &self).map_err(|e| e.into()) } /// Serialize into a new heap-allocated buffer - fn ser(&self) -> Result, SerializeError> - where - Self: Serialize, - { + fn ser(&self) -> Result, SerializeError> { be_coder().serialize(&self).map_err(|e| e.into()) } /// Deserialize from the full contents of a byte slice /// /// See also: [`BeSer::des_prefix`] - fn des(buf: &[u8]) -> Result - where - Self: DeserializeOwned, - { + fn des(buf: &[u8]) -> Result { be_coder() .deserialize(buf) .or(Err(DeserializeError::BadInput)) @@ -143,10 +131,7 @@ pub trait BeSer { /// type, but does not guarantee that the entire slice is used. /// /// See also: [`BeSer::des`] - fn des_prefix(buf: &[u8]) -> Result - where - Self: DeserializeOwned, - { + fn des_prefix(buf: &[u8]) -> Result { be_coder() .allow_trailing_bytes() .deserialize(buf) @@ -154,10 +139,7 @@ pub trait BeSer { } /// Deserialize from a reader - fn des_from(r: &mut R) -> Result - where - Self: DeserializeOwned, - { + fn des_from(r: &mut R) -> Result { be_coder().deserialize_from(r).map_err(|e| e.into()) } @@ -165,22 +147,16 @@ pub trait BeSer { /// /// Note: it may be faster to serialize to a buffer and then measure the /// buffer length, than to call `serialized_size` and then `ser_into`. - fn serialized_size(&self) -> Result - where - Self: Serialize, - { + fn serialized_size(&self) -> Result { be_coder().serialized_size(self).map_err(|e| e.into()) } } /// Binary serialize/deserialize helper functions (Little Endian) /// -pub trait LeSer { +pub trait LeSer: Serialize + DeserializeOwned { /// Serialize into a byte slice - fn ser_into_slice(&self, mut b: &mut [u8]) -> Result<(), SerializeError> - where - Self: Serialize, - { + fn ser_into_slice(&self, mut b: &mut [u8]) -> Result<(), SerializeError> { // &mut [u8] implements Write, but `ser_into` needs a mutable // reference to that. So we need the slightly awkward "mutable // reference to a mutable reference. @@ -191,28 +167,19 @@ pub trait LeSer { /// /// This is useful for most `Write` types except `&mut [u8]`, which /// can more easily use [`ser_into_slice`](Self::ser_into_slice). - fn ser_into(&self, w: &mut W) -> Result<(), SerializeError> - where - Self: Serialize, - { + fn ser_into(&self, w: &mut W) -> Result<(), SerializeError> { le_coder().serialize_into(w, &self).map_err(|e| e.into()) } /// Serialize into a new heap-allocated buffer - fn ser(&self) -> Result, SerializeError> - where - Self: Serialize, - { + fn ser(&self) -> Result, SerializeError> { le_coder().serialize(&self).map_err(|e| e.into()) } /// Deserialize from the full contents of a byte slice /// /// See also: [`LeSer::des_prefix`] - fn des(buf: &[u8]) -> Result - where - Self: DeserializeOwned, - { + fn des(buf: &[u8]) -> Result { le_coder() .deserialize(buf) .or(Err(DeserializeError::BadInput)) @@ -224,10 +191,7 @@ pub trait LeSer { /// type, but does not guarantee that the entire slice is used. /// /// See also: [`LeSer::des`] - fn des_prefix(buf: &[u8]) -> Result - where - Self: DeserializeOwned, - { + fn des_prefix(buf: &[u8]) -> Result { le_coder() .allow_trailing_bytes() .deserialize(buf) @@ -235,10 +199,7 @@ pub trait LeSer { } /// Deserialize from a reader - fn des_from(r: &mut R) -> Result - where - Self: DeserializeOwned, - { + fn des_from(r: &mut R) -> Result { le_coder().deserialize_from(r).map_err(|e| e.into()) } @@ -246,18 +207,14 @@ pub trait LeSer { /// /// Note: it may be faster to serialize to a buffer and then measure the /// buffer length, than to call `serialized_size` and then `ser_into`. - fn serialized_size(&self) -> Result - where - Self: Serialize, - { + fn serialized_size(&self) -> Result { le_coder().serialized_size(self).map_err(|e| e.into()) } } -// Because usage of `BeSer` or `LeSer` can be done with *either* a Serialize or -// DeserializeOwned implementation, the blanket implementation has to be for every type. -impl BeSer for T {} -impl LeSer for T {} +impl BeSer for T where T: Serialize + DeserializeOwned {} + +impl LeSer for T where T: Serialize + DeserializeOwned {} #[cfg(test)] mod tests { diff --git a/zenith_utils/tests/bin_ser_test.rs b/zenith_utils/tests/bin_ser_test.rs index ada43a1189..d920b430f3 100644 --- a/zenith_utils/tests/bin_ser_test.rs +++ b/zenith_utils/tests/bin_ser_test.rs @@ -1,10 +1,10 @@ use bytes::{Buf, BytesMut}; use hex_literal::hex; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; use std::io::Read; use zenith_utils::bin_ser::LeSer; -#[derive(Debug, PartialEq, Deserialize)] +#[derive(Debug, PartialEq, Serialize, Deserialize)] pub struct HeaderData { magic: u16, info: u16,