Skip to content
This repository was archived by the owner on Nov 7, 2019. It is now read-only.

7252 compressed zfs send / receive #192

Closed

Conversation

dankimmel
Copy link

@dankimmel dankimmel commented Sep 21, 2016

7628 create long versions of ZFS send / receive options

Reviewed by: George Wilson george.wilson@delphix.com ( @grwilson )
Reviewed by: Matthew Ahrens mahrens@delphix.com ( @ahrens )
Reviewed by: Paul Dagnelie pcd@delphix.com ( @pcd1193182 )
Reviewed by: John Kennedy john.kennedy@delphix.com ( @jwk404 )
Reviewed by: Sebastien Roy sebastien.roy@delphix.com ( @sebroy )
Reviewed by: Pavel Zakharov pavel.zakharov@delphix.com ( @pzakha )

To help ZFS send large amounts of data faster using the same network
throughput, this adds a new --compressed option to the zfs send command.
This allows send streams to simply leave data blocks in whatever compressed
format they are in on-disk, reducing the total amount of data to send by a
factor of roughly compressratio without any additional CPU usage. For
receive-side code simplicity, we still decompress metadata blocks before
sending them.

This diff also refactors some areas of the ARC for clarity and adds longopt
versions of the existing zfs send options.

Upstream bugs: DLPX-45723, DLPX-44733, DLPX-44733, DLPX-40252, QA-5900, DLPX-47346, DLPX-46082, DLPX-46316, QA-6101, DLPX-44361, DLPX-48655
Tests were added by @jwk404

@dankimmel dankimmel force-pushed the openzfs.7252-compressed-send-recv branch from 8f5937d to 0cc5281 Compare September 21, 2016 22:52
@dankimmel
Copy link
Author

The ztest failure is in dmu_tx_willuse_space which is a known (unrelated) issue. The zfs-test failure looks like I accidentally introduced a problem in the rsend tests during my merge -- I'll take a look at that.

@ahrens
Copy link
Member

ahrens commented Nov 2, 2016

@zettabot go

@dankimmel dankimmel force-pushed the openzfs.7252-compressed-send-recv branch from 0cc5281 to ac41920 Compare November 8, 2016 02:55
@dankimmel
Copy link
Author

dankimmel commented Nov 8, 2016

This now depends on #224 integrating first.

@dankimmel dankimmel force-pushed the openzfs.7252-compressed-send-recv branch 4 times, most recently from f5d619c to 68b079f Compare November 14, 2016 08:44
@dankimmel dankimmel force-pushed the openzfs.7252-compressed-send-recv branch 3 times, most recently from 7b32293 to 1d9327e Compare November 16, 2016 18:39
@dankimmel
Copy link
Author

@zettabot go

@dankimmel
Copy link
Author

The failure in delegate/zfs_allow_007_pos is unrelated, but I don't see a bug filed for it yet so I'm filing a new one against our downstream fork now.

@dankimmel dankimmel force-pushed the openzfs.7252-compressed-send-recv branch from 1d9327e to 86b3a44 Compare November 18, 2016 17:19
Copy link

@dpquigl dpquigl left a comment

Choose a reason for hiding this comment

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

Reviewing the current state of the patch it looks to be in good shape. The only differences I found in this patch and what we merged into ZFS on Linux were based on us not having certain send/recv features currently in illumos.

@tcaputi
Copy link

tcaputi commented Nov 21, 2016

LGTM. I'll just say again here (as I said for the Linux PR) it would probably be a good idea to have send flags instead of booleans for embed_ok, compress_ok, etc.

@dankimmel dankimmel force-pushed the openzfs.7252-compressed-send-recv branch from 86b3a44 to eeb9fe9 Compare November 21, 2016 21:41
@dankimmel
Copy link
Author

Just rebased and added Tom & David as reviewers. Kicking off one last test run.

@dankimmel
Copy link
Author

@zettabot go

@dankimmel
Copy link
Author

The panic in zfs-test is caused by a stack trace that looks identical to https://www.illumos.org/issues/6292 (maybe we re-introduced that somehow?). I'm kicking off another run to make sure that the test changes in this diff are working as expected.

@dankimmel dankimmel force-pushed the openzfs.7252-compressed-send-recv branch 2 times, most recently from 04df970 to edae176 Compare November 23, 2016 14:26
@dankimmel
Copy link
Author

@zettabot go

@dankimmel dankimmel force-pushed the openzfs.7252-compressed-send-recv branch from edae176 to 9af7ce7 Compare November 28, 2016 17:02
Copy link

@danmcd danmcd left a comment

Choose a reason for hiding this comment

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

Just the '--' one...

@@ -2464,7 +2464,7 @@ to a different system
.Pc .
By default, a full stream is generated.
.Bl -tag -width "-D"
.It Fl D
.It Fl D, -dedup
Copy link

Choose a reason for hiding this comment

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

One '-' or two '--'?

Copy link
Member

Choose a reason for hiding this comment

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

One - is correct here, it renders as --

'rsend_021_pos', 'rsend_022_pos', 'rsend_024_pos',
'send-c_verify_ratio', 'send-c_verify_contents', 'send-c_props',
'send-c_incremental', 'send-c_volume', 'send-c_zstreamdump',
'send-c_lz4_disabled', 'send-c_recv_lz4_disabled',
Copy link

Choose a reason for hiding this comment

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

What about *.run for not-Delphix? (I thought we got rid of those as well.)

Copy link
Author

Choose a reason for hiding this comment

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

Good catch, I'm making the same update to omnios.run and openindiana.run now. Apparently we got rid of them for the os-tests, but not for the zfs-tests (yet).

Copy link
Author

Choose a reason for hiding this comment

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

This is now fixed in the most recent diff.

7628 create long versions of ZFS send / receive options

Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: John Kennedy <john.kennedy@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Paul Dagnelie <pcd@delphix.com>
Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Reviewed by: Sebastien Roy <sebastien.roy@delphix.com>
Reviewed by: David Quigley <dpquigl@davequigley.com>
Reviewed by: Thomas Caputi <tcaputi@datto.com>

To help ZFS send large amounts of data faster using the same network
throughput, this adds a new --compressed option to the zfs send command.
This allows send streams to simply leave data blocks in whatever
compressed format they are in on-disk, reducing the total amount of data
to send by a factor of roughly "compressratio" without any additional
CPU usage. For receive-side code simplicity, we still decompress
metadata blocks before sending them.

This diff also refactors some areas of the ARC for clarity and adds
longopt versions of the existing zfs send options.
@dankimmel dankimmel force-pushed the openzfs.7252-compressed-send-recv branch from 9af7ce7 to 7f694a8 Compare November 28, 2016 20:16
@ahrens ahrens closed this in 9896f2b Nov 29, 2016
depend fmri=system/file-system/zfs/tests type=require
depend fmri=system/test/fio type=require
Copy link

Choose a reason for hiding this comment

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

Sorry I didn't catch this earlier. Change this to type=optional.

Copy link
Member

Choose a reason for hiding this comment

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

@danmcd did you see this PR: #261 ? It looks like the OI folks have said it's no longer needed for them, but I still wanted to make sure you're OK without this change since you requested it here.

Copy link

Choose a reason for hiding this comment

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

Yeah I saw 261, and you should keep it as optional (to indicate that it'd be nice to have it).

jwk404 pushed a commit to jwk404/openzfs that referenced this pull request Dec 19, 2016
7628 create long versions of ZFS send / receive options

Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: John Kennedy <john.kennedy@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Paul Dagnelie <pcd@delphix.com>
Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com>
Reviewed by: Sebastien Roy <sebastien.roy@delphix.com>
Reviewed by: David Quigley <dpquigl@davequigley.com>
Reviewed by: Thomas Caputi <tcaputi@datto.com>

To help ZFS send large amounts of data faster using the same network
throughput, this adds a new --compressed option to the zfs send command.
This allows send streams to simply leave data blocks in whatever
compressed format they are in on-disk, reducing the total amount of data
to send by a factor of roughly "compressratio" without any additional
CPU usage. For receive-side code simplicity, we still decompress
metadata blocks before sending them.

This diff also refactors some areas of the ARC for clarity and adds
longopt versions of the existing zfs send options.

Closes openzfs#192
@dankimmel dankimmel deleted the openzfs.7252-compressed-send-recv branch March 31, 2017 17:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants