Add byte level details to list progress command #503
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
hanefi
force-pushed
the
byte-level-progress
branch
from
5574673
to
f4b70aa
Compare
This patch allows reporting the total number of bytes transferred during
a copy operation. However, it has a limitation that the reported value
is only updated after each table is copied. This is because we do not
update table summary files while the copy is in progress.
Changes include:
- 3 new fields in table summary structures:
- network.bytes: total number of bytes transmitted
- network.bytes-pretty: pretty printed form of network.bytes
- network.transmit-rate: pretty printed bytes transmitted per second
- Some new fields on the `pgcopydb list progress --json --summary`
command:
- steps[].network is a new field that exists for COPY step and has
the following fields:
- bytes: total number of bytes transmitted for all tables
- bytes-pretty: pretty printed form of bytes
- tables[].network is a new json value with the following fields:
- bytes: total number of bytes transmitted for table
- bytes-pretty: pretty printed form of bytes
- transmit-rate: pretty printed bytes transmitted per second
- A new column on top level summary that shows total number of bytes
copied.
hanefi
force-pushed
the
byte-level-progress
branch
from
be253a2
to
4699e6e
Compare
hanefi
commented
Nov 14, 2023
Comment on lines
+37
to
+43
| char bytesPretty[BUFSIZE] = { 0 }; | ||
| pretty_print_bytes(bytesPretty, BUFSIZE, summary->bytesTransmitted); | ||
|
|
||
| char transmitRate[BUFSIZE] = { 0 }; | ||
| pretty_print_bytes_per_second(transmitRate, BUFSIZE, summary->bytesTransmitted, | ||
| summary->durationMs); | ||
|
|
There was a problem hiding this comment.
I did not want to store bytesPretty and transmitRate in CopyTableSummary structs.
- It is inexpensive to have an integer addition in our copy loop, unlike pretty printing said value. I did not want to store bytes and bytesPretty when I can only update the primitive type and not keep the other in sync.
- Calculating the transmitRate in real time is also an expensive operation similar to bytesPretty that was discussed in earlier item. Also, I needed to have the duration information that is not always there when the command is running.
- If resuming an earlier operation, reading an integer value for bytes from disk makes sense. All the other values can be generated from bytes and duration information.
docs/ref/pgcopydb_clone.rst
Outdated
Comment on lines
894
to
906
| Step Connection Duration Transfer Concurrency | ||
| -------------------------------------------------- ---------- ---------- ---------- ------------ | ||
| Dump Schema source 225ms 1 | ||
| Catalog Queries (table ordering, filtering, etc) source 842ms 1 | ||
| Prepare Schema target 694ms 1 | ||
| COPY, INDEX, CONSTRAINTS, VACUUM (wall clock) both 1s538 8 + 12 | ||
| COPY (cumulative) both 2s771 2955 kB 8 | ||
| Large Objects (cumulative) both 242ms 1 | ||
| CREATE INDEX, CONSTRAINTS (cumulative) target 3s489 12 | ||
| Finalize Schema target 654ms 1 | ||
| -------------------------------------------------- ---------- ---------- ---------- ------------ | ||
| Total Wall Clock Duration both 3s984 8 + 12 | ||
| -------------------------------------------------- ---------- ---------- ---------- ------------ |
There was a problem hiding this comment.
Note: In this patch, most of the data in the top-level duration tables are made up.
There was a problem hiding this comment.
All the numbers in the docs to this point where obtained by actual runs and copy/pasted. When that make sense, runs from the test suite, ensuring they're easy to replicate, and that we're in the right ballpark. Please consider the same approach here.
There was a problem hiding this comment.
I created a local prototype that can produce reproducible documentation examples. I used a copy of pagila tests as a starting point and I copied commands from our current documentation. Finally I copy pasted command outputs to the docs.
Hopefully, I will open a separate PR with my changes for reproducible documentation example generation after polishing it some more.
dimitri
requested changes
Nov 20, 2023
docs/ref/pgcopydb_clone.rst
Outdated
Comment on lines
894
to
906
| Step Connection Duration Transfer Concurrency | ||
| -------------------------------------------------- ---------- ---------- ---------- ------------ | ||
| Dump Schema source 225ms 1 | ||
| Catalog Queries (table ordering, filtering, etc) source 842ms 1 | ||
| Prepare Schema target 694ms 1 | ||
| COPY, INDEX, CONSTRAINTS, VACUUM (wall clock) both 1s538 8 + 12 | ||
| COPY (cumulative) both 2s771 2955 kB 8 | ||
| Large Objects (cumulative) both 242ms 1 | ||
| CREATE INDEX, CONSTRAINTS (cumulative) target 3s489 12 | ||
| Finalize Schema target 654ms 1 | ||
| -------------------------------------------------- ---------- ---------- ---------- ------------ | ||
| Total Wall Clock Duration both 3s984 8 + 12 | ||
| -------------------------------------------------- ---------- ---------- ---------- ------------ |
There was a problem hiding this comment.
All the numbers in the docs to this point where obtained by actual runs and copy/pasted. When that make sense, runs from the test suite, ensuring they're easy to replicate, and that we're in the right ballpark. Please consider the same approach here.
This commit addresses 2 issues raised in the review: 1. Using a SI standards when reporting data-rate units 2. Updating the relevant documentation with reproducible examples The documentation update is done in a way that allowed me to run the commands in a docker environment. Once I polish my changes in this docker environment, I will update all the documentation with it and share it in a separate PR.
dimitri
approved these changes
Nov 22, 2023
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.
This patch allows reporting the total number of bytes transferred during
a copy operation. However, it has a limitation that the reported value
is only updated after each table is copied. This is because we do not
update table summary files while the copy is in progress.
Changes include:
pgcopydb list progress --json --summarycommand:
the following fields:
copied.