diff --git a/libs/pageserver_api/src/models.rs b/libs/pageserver_api/src/models.rs index 4cab56771b8e..d39ac7570772 100644 --- a/libs/pageserver_api/src/models.rs +++ b/libs/pageserver_api/src/models.rs @@ -1063,7 +1063,7 @@ impl TryFrom for PagestreamBeMessageTag { } } -// In the V2 protocol version, a GetPage request contains two LSN values: +// A GetPage request contains two LSN values: // // request_lsn: Get the page version at this point in time. Lsn::Max is a special value that means // "get the latest version present". It's used by the primary server, which knows that no one else @@ -1076,7 +1076,7 @@ impl TryFrom for PagestreamBeMessageTag { // passing an earlier LSN can speed up the request, by allowing the pageserver to process the // request without waiting for 'request_lsn' to arrive. // -// The legacy V1 interface contained only one LSN, and a boolean 'latest' flag. The V1 interface was +// The now-defunct V1 interface contained only one LSN, and a boolean 'latest' flag. The V1 interface was // sufficient for the primary; the 'lsn' was equivalent to the 'not_modified_since' value, and // 'latest' was set to true. The V2 interface was added because there was no correct way for a // standby to request a page at a particular non-latest LSN, and also include the @@ -1084,15 +1084,11 @@ impl TryFrom for PagestreamBeMessageTag { // request, if the standby knows that the page hasn't been modified since, and risk getting an error // if that LSN has fallen behind the GC horizon, or requesting the current replay LSN, which could // require the pageserver unnecessarily to wait for the WAL to arrive up to that point. The new V2 -// interface allows sending both LSNs, and let the pageserver do the right thing. There is no +// interface allows sending both LSNs, and let the pageserver do the right thing. There was no // difference in the responses between V1 and V2. // -// The Request structs below reflect the V2 interface. If V1 is used, the parse function -// maps the old format requests to the new format. -// #[derive(Clone, Copy)] pub enum PagestreamProtocolVersion { - V1, V2, } @@ -1231,36 +1227,17 @@ impl PagestreamFeMessage { bytes.into() } - pub fn parse( - body: &mut R, - protocol_version: PagestreamProtocolVersion, - ) -> anyhow::Result { + pub fn parse(body: &mut R) -> anyhow::Result { // these correspond to the NeonMessageTag enum in pagestore_client.h // // TODO: consider using protobuf or serde bincode for less error prone // serialization. let msg_tag = body.read_u8()?; - let (request_lsn, not_modified_since) = match protocol_version { - PagestreamProtocolVersion::V2 => ( - Lsn::from(body.read_u64::()?), - Lsn::from(body.read_u64::()?), - ), - PagestreamProtocolVersion::V1 => { - // In the old protocol, each message starts with a boolean 'latest' flag, - // followed by 'lsn'. Convert that to the two LSNs, 'request_lsn' and - // 'not_modified_since', used in the new protocol version. - let latest = body.read_u8()? != 0; - let request_lsn = Lsn::from(body.read_u64::()?); - if latest { - (Lsn::MAX, request_lsn) // get latest version - } else { - (request_lsn, request_lsn) // get version at specified LSN - } - } - }; + // these two fields are the same for every request type + let request_lsn = Lsn::from(body.read_u64::()?); + let not_modified_since = Lsn::from(body.read_u64::()?); - // The rest of the messages are the same between V1 and V2 match msg_tag { 0 => Ok(PagestreamFeMessage::Exists(PagestreamExistsRequest { request_lsn, @@ -1468,9 +1445,7 @@ mod tests { ]; for msg in messages { let bytes = msg.serialize(); - let reconstructed = - PagestreamFeMessage::parse(&mut bytes.reader(), PagestreamProtocolVersion::V2) - .unwrap(); + let reconstructed = PagestreamFeMessage::parse(&mut bytes.reader()).unwrap(); assert!(msg == reconstructed); } } diff --git a/pageserver/src/metrics.rs b/pageserver/src/metrics.rs index 1f8634df939e..c4011d593c93 100644 --- a/pageserver/src/metrics.rs +++ b/pageserver/src/metrics.rs @@ -1552,7 +1552,6 @@ pub(crate) static LIVE_CONNECTIONS: Lazy = Lazy::new(|| { #[derive(Clone, Copy, enum_map::Enum, IntoStaticStr)] pub(crate) enum ComputeCommandKind { PageStreamV2, - PageStream, Basebackup, Fullbackup, LeaseLsn, diff --git a/pageserver/src/page_service.rs b/pageserver/src/page_service.rs index 81294291a93f..cb1ab70147ab 100644 --- a/pageserver/src/page_service.rs +++ b/pageserver/src/page_service.rs @@ -557,7 +557,7 @@ impl PageServerHandler { pgb: &mut PostgresBackend, tenant_id: TenantId, timeline_id: TimelineId, - protocol_version: PagestreamProtocolVersion, + _protocol_version: PagestreamProtocolVersion, ctx: RequestContext, ) -> Result<(), QueryError> where @@ -601,8 +601,7 @@ impl PageServerHandler { fail::fail_point!("ps::handle-pagerequest-message"); // parse request - let neon_fe_msg = - PagestreamFeMessage::parse(&mut copy_data_bytes.reader(), protocol_version)?; + let neon_fe_msg = PagestreamFeMessage::parse(&mut copy_data_bytes.reader())?; // invoke handler function let (handler_result, span) = match neon_fe_msg { @@ -1275,35 +1274,6 @@ where ctx, ) .await?; - } else if let Some(params) = parts.strip_prefix(&["pagestream"]) { - if params.len() != 2 { - return Err(QueryError::Other(anyhow::anyhow!( - "invalid param number for pagestream command" - ))); - } - let tenant_id = TenantId::from_str(params[0]) - .with_context(|| format!("Failed to parse tenant id from {}", params[0]))?; - let timeline_id = TimelineId::from_str(params[1]) - .with_context(|| format!("Failed to parse timeline id from {}", params[1]))?; - - tracing::Span::current() - .record("tenant_id", field::display(tenant_id)) - .record("timeline_id", field::display(timeline_id)); - - self.check_permission(Some(tenant_id))?; - - COMPUTE_COMMANDS_COUNTERS - .for_command(ComputeCommandKind::PageStream) - .inc(); - - self.handle_pagerequests( - pgb, - tenant_id, - timeline_id, - PagestreamProtocolVersion::V1, - ctx, - ) - .await?; } else if let Some(params) = parts.strip_prefix(&["basebackup"]) { if params.len() < 2 { return Err(QueryError::Other(anyhow::anyhow!( diff --git a/pgxn/neon/libpagestore.c b/pgxn/neon/libpagestore.c index 73a001b6ba72..5126c26c5d13 100644 --- a/pgxn/neon/libpagestore.c +++ b/pgxn/neon/libpagestore.c @@ -550,9 +550,6 @@ pageserver_connect(shardno_t shard_no, int elevel) case 2: pagestream_query = psprintf("pagestream_v2 %s %s", neon_tenant, neon_timeline); break; - case 1: - pagestream_query = psprintf("pagestream %s %s", neon_tenant, neon_timeline); - break; default: elog(ERROR, "unexpected neon_protocol_version %d", neon_protocol_version); } @@ -1063,7 +1060,7 @@ pg_init_libpagestore(void) NULL, &neon_protocol_version, 2, /* use protocol version 2 */ - 1, /* min */ + 2, /* min */ 2, /* max */ PGC_SU_BACKEND, 0, /* no flags required */ diff --git a/pgxn/neon/pagestore_client.h b/pgxn/neon/pagestore_client.h index 8951e6607b24..1f196d016c3f 100644 --- a/pgxn/neon/pagestore_client.h +++ b/pgxn/neon/pagestore_client.h @@ -87,9 +87,8 @@ typedef enum { * can skip traversing through recent layers which we know to not contain any * versions for the requested page. * - * These structs describe the V2 of these requests. The old V1 protocol contained - * just one LSN and a boolean 'latest' flag. If the neon_protocol_version GUC is - * set to 1, we will convert these to the V1 requests before sending. + * These structs describe the V2 of these requests. (The old now-defunct V1 + * protocol contained just one LSN and a boolean 'latest' flag.) */ typedef struct { diff --git a/pgxn/neon/pagestore_smgr.c b/pgxn/neon/pagestore_smgr.c index 8edaf65639f7..7f39c7d02603 100644 --- a/pgxn/neon/pagestore_smgr.c +++ b/pgxn/neon/pagestore_smgr.c @@ -1001,51 +1001,10 @@ nm_pack_request(NeonRequest *msg) initStringInfo(&s); - if (neon_protocol_version >= 2) - { - pq_sendbyte(&s, msg->tag); - pq_sendint64(&s, msg->lsn); - pq_sendint64(&s, msg->not_modified_since); - } - else - { - bool latest; - XLogRecPtr lsn; - - /* - * In primary, we always request the latest page version. - */ - if (!RecoveryInProgress()) - { - latest = true; - lsn = msg->not_modified_since; - } - else - { - /* - * In the protocol V1, we cannot represent that we want to read - * page at LSN X, and we know that it hasn't been modified since - * Y. We can either use 'not_modified_lsn' as the request LSN, and - * risk getting an error if that LSN is too old and has already - * fallen out of the pageserver's GC horizon, or we can send - * 'request_lsn', causing the pageserver to possibly wait for the - * recent WAL to arrive unnecessarily. Or something in between. We - * choose to use the old LSN and risk GC errors, because that's - * what we've done historically. - */ - latest = false; - lsn = msg->not_modified_since; - } + pq_sendbyte(&s, msg->tag); + pq_sendint64(&s, msg->lsn); + pq_sendint64(&s, msg->not_modified_since); - pq_sendbyte(&s, msg->tag); - pq_sendbyte(&s, latest); - pq_sendint64(&s, lsn); - } - - /* - * The rest of the request messages are the same between protocol V1 and - * V2 - */ switch (messageTag(msg)) { /* pagestore_client -> pagestore */ diff --git a/test_runner/regress/test_auth.py b/test_runner/regress/test_auth.py index 7cb85e3dd1b2..780c0e1602ac 100644 --- a/test_runner/regress/test_auth.py +++ b/test_runner/regress/test_auth.py @@ -211,7 +211,7 @@ def op(): def check_pageserver(expect_success: bool, **conn_kwargs): check_connection( env.pageserver, - f"pagestream {env.initial_tenant} {env.initial_timeline}", + f"pagestream_v2 {env.initial_tenant} {env.initial_timeline}", expect_success, **conn_kwargs, ) diff --git a/test_runner/regress/test_read_validation.py b/test_runner/regress/test_read_validation.py index d128c60a99c8..1ac881553fbc 100644 --- a/test_runner/regress/test_read_validation.py +++ b/test_runner/regress/test_read_validation.py @@ -19,11 +19,6 @@ def test_read_validation(neon_simple_env: NeonEnv): endpoint = env.endpoints.create_start( "test_read_validation", - # Use protocol version 2, because the code that constructs the V1 messages - # assumes that a primary always wants to read the latest version of a page, - # and therefore doesn't work with the test functions below to read an older - # page version. - config_lines=["neon.protocol_version=2"], ) with closing(endpoint.connect()) as con: @@ -142,11 +137,6 @@ def test_read_validation_neg(neon_simple_env: NeonEnv): endpoint = env.endpoints.create_start( "test_read_validation_neg", - # Use protocol version 2, because the code that constructs the V1 messages - # assumes that a primary always wants to read the latest version of a page, - # and therefore doesn't work with the test functions below to read an older - # page version. - config_lines=["neon.protocol_version=2"], ) with closing(endpoint.connect()) as con: