Skip to content

Commit

Permalink
compute_ctl: database_schema should keep process::Child as part of re…
Browse files Browse the repository at this point in the history
…turned value (#10273)

## Problem

/database_schema endpoint returns incomplete output from `pg_dump`

## Summary of changes

The Tokio process was not used properly. The returned stream does not
include `process::Child`, and the process is scheduled to be killed
immediately after the `get_database_schema` call when `cmd` goes out of
scope.

The solution in this PR is to return a special Stream implementation
that retains `process::Child`.
  • Loading branch information
prepor authored Feb 11, 2025
1 parent 98883e4 commit 4ab1844
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 2 deletions.
31 changes: 30 additions & 1 deletion compute_tools/src/catalog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,5 +140,34 @@ pub async fn get_database_schema(
warn!("pg_dump stderr: {}", line)
}
});
Ok(initial_stream.chain(stdout_reader.map(|res| res.map(|b| b.freeze()))))

#[allow(dead_code)]
struct SchemaStream<S> {
// We keep a reference to the child process to ensure it stays alive
// while the stream is being consumed. When SchemaStream is dropped,
// cmd will be dropped, which triggers kill_on_drop and terminates pg_dump
cmd: tokio::process::Child,
stream: S,
}

impl<S> Stream for SchemaStream<S>
where
S: Stream<Item = Result<bytes::Bytes, std::io::Error>> + Unpin,
{
type Item = Result<bytes::Bytes, std::io::Error>;

fn poll_next(
mut self: std::pin::Pin<&mut Self>,
cx: &mut std::task::Context<'_>,
) -> std::task::Poll<Option<Self::Item>> {
Stream::poll_next(std::pin::Pin::new(&mut self.stream), cx)
}
}

let schema_stream = SchemaStream {
cmd,
stream: initial_stream.chain(stdout_reader.map(|res| res.map(|b| b.freeze()))),
};

Ok(schema_stream)
}
2 changes: 1 addition & 1 deletion test_runner/regress/test_compute_catalog.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def test_compute_catalog(neon_simple_env: NeonEnv):
ddl = client.database_schema(database=test_db["name"])

# Check that it looks like a valid PostgreSQL dump
assert "-- PostgreSQL database dump" in ddl
assert "-- PostgreSQL database dump complete" in ddl

# Check that it doesn't contain health_check and migration traces.
# They are only created in system `postgres` database, so by checking
Expand Down

1 comment on commit 4ab1844

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7576 tests run: 7184 passed, 0 failed, 392 skipped (full report)


Flaky tests (2)

Postgres 17

Code coverage* (full report)

  • functions: 33.2% (8585 of 25846 functions)
  • lines: 49.1% (72279 of 147332 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
4ab1844 at 2025-02-11T09:25:05.735Z :recycle:

Please sign in to comment.