Scrape mz_introspection data as CSVs per cluster replica #31951
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SangJunBak
force-pushed
the
jun/#8908/dump-catalog-3
branch
from
e82b8ed to
6313548
Compare
SangJunBak
force-pushed
the
jun/#8908/dump-catalog-2
branch
from
d01849a to
fdd5ec2
Compare
- Convert queries to transactions to account for different cluster session variables per introspection query - Introduce separate timeouts for connection and query execution. - Extend relation dumping logic for introspection relatoins
- Create a future instead of spawning a task - Separate closures and functions into separate functions - Make the docs link conditional based on the cluster replica - Cancel queries when not responsive via cancel_token - Set the max retry duration to 20s until dump_relation gets cancelled - Sets the max number of errors for cluster replicas to 3 until dump_all_relations skips the cluster replica entirely
SangJunBak
force-pushed
the
jun/#8908/dump-catalog-3
branch
from
82c6483 to
e783750
Compare
SangJunBak
changed the title
This was referenced Mar 20, 2025
ParkMyCar
approved these changes
Mar 21, 2025
| @@ -615,57 +615,231 @@ const RELATIONS: &[Relation] = &[ | |||
| }, | |||
| ]; | |||
|
|
|||
| const TIMEOUT: Duration = Duration::from_secs(60); | |||
| static PG_CONNECTION_TIMEOUT: Duration = Duration::from_secs(60); | |||
| /// Timeout for a query. We use 6 minutes since it's a good | |||
There was a problem hiding this comment.
The comment says 6 minutes but the timeout is 20 seconds?
| anyhow::Error, | ||
| > { | ||
| let mut pg_config = PgConfig::from_str(connection_string)?; | ||
| pg_config.connect_timeout(PG_CONNECTION_TIMEOUT); |
There was a problem hiding this comment.
It might be nice to have the timeout be configurable via a CLI arg? Doesn't need to be fixed in this PR though!
| let copy_stream = std::pin::pin!(copy_stream); | ||
| let mut reader = StreamReader::new(copy_stream); | ||
| tokio::io::copy(&mut reader, &mut file).await?; | ||
|
|
There was a problem hiding this comment.
Here we should call file.sync_all().await? so we make sure the data is fsync-ed to disk
| } | ||
|
|
||
| // We query the column names to write the header row of the CSV file. | ||
| // TODO (SangJunBak): Use `WITH (HEADER TRUE)` once database-issues#2846 is implemented. |
There was a problem hiding this comment.
I think this comment is stale? Or it should maybe be moved to copy_relation_to_csv(...)?
There was a problem hiding this comment.
Ah good callout on how it should be closer to copy_relation_to_csv.
| let pg_client = Arc::clone(&pg_client); | ||
| let relation_name = relation_name.clone(); | ||
| let relation_category = relation_category.clone(); | ||
| let mut pg_client_lock = pg_client.lock().await; |
There was a problem hiding this comment.
Can you document somewhere in this crate why we don't want to run these queries in parallel, and we'd prefer serial execution?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on PR #31859
Chain of upstream PRs as of 2025-03-20
PR Add system catalog dumper for self-managed debug tool #31859:
main←jun/#8908/dump-catalog-2jun/#8908/dump-catalog-2←jun/#8908/dump-catalog-3See commit messages for details
https://github.com/MaterializeInc/database-issues/issues/8908
Motivation
Tips for reviewer
Checklist
$T ⇔ Proto$Tmapping (possibly in a backwards-incompatible way), then it is tagged with aT-protolabel.