search

TritonDataCenter / smartos-live

For more information, please see http://smartos.org/ For any questions that aren't answered there, please join the SmartOS discussion list: https://smartos.topicbox.com/groups/smartos-discuss
1.57k stars 246 forks source link

OS-6185 use pigz (parallel gunzip uncompression) to speed up imgadm import Reviewed by: Trent Mick <trentm@gmail.com> #866

Closed joyent-automation closed 2 years ago

joyent-automation commented 4 years ago

OS-6185 use pigz (parallel gunzip uncompression) to speed up imgadm import Reviewed by: Trent Mick trentm@gmail.com

This PR was migrated-from-gerrit, https://cr.joyent.us/#/c/2127/. The raw archive of this CR is here. See MANTA-4594 for info on Joyent Eng's migration from Gerrit.

CR discussion

@twhiteman commented at 2017-06-19T22:32:34

Patch Set 1:

New commits:
commit a91cd39157c531eba555a616f3c287441bea13e2
OS-6185 use pigz (parallel gunzip uncompression) to speed up imgadm import

@twhiteman commented at 2017-06-19T22:39:17

Patch Set 2:

New commits:
commit cb6e5929641a20126d96d471d5a31e6af9706b98
OS-6185 use pigz (parallel gunzip uncompression) to speed up imgadm import

@trentm commented at 2017-06-27T22:19:09

Patch Set 2:

(2 comments)

@twhiteman commented at 2017-06-28T23:26:29

Patch Set 2:

(1 comment)

Patch Set 2 code comments
src/img/sbin/chroot-gtar#108 @trentm

Where does pigz come from? Are there missing plat changes to add that?

src/img/sbin/chroot-gtar#108 @twhiteman

Yeah, it's waiting on https://cr.joyent.us/#/c/2126/

src/img/sbin/gtar-unlink-dir#77 @trentm

Could (I'm not fussed) just use uncompresscat="pigz -dc -p 2" and not bother with a separate 'uncompresscatopts' var.

@twhiteman commented at 2017-07-17T21:54:43

Patch Set 3:

New commits:
commit ec833de7eb635e9ddcad7843ff884a5d97ce44f8
OS-6185 pigz (parallel gzip) support in the platform

  • don't remove /usr/img directory, as pigz is already installed there

    commit 25394ba4ab49afd2cbb61a948875f3636ba00540
    OS-6185 use pigz (parallel gunzip uncompression) to speed up imgadm import

@trentm commented at 2017-07-17T22:11:24

Patch Set 3: Code-Review+1

(1 comment)

What kind of testing have you done so far here?

Patch Set 3 code comments
src/img/package.json#4 @trentm

I don't care strongly, but I'd classify this as a feature rather than a bug fix, hence "3.8.0" instead. But really it isn't a biggie.

src/img/package.json#4 @twhiteman

Done

@twhiteman commented at 2017-07-18T18:19:33

Patch Set 4:

New commits:
commit 9a7013d889a907689fff5658499a4a523bd41c27
add imgadm 'usePigz' config option, so that pigz decompression can be disabled

@twhiteman commented at 2017-07-18T18:24:40

Patch Set 3:

(1 comment)

What kind of testing have you done so far here?

Manual testing of common docker images (and on some large docker images).

I've tested the new 'usePigz' config option, using dtrace to snoop on the executed processes, to ensure the imgadm config setting matches the expected result.

@trentm commented at 2017-07-20T21:28:09

Patch Set 4: Code-Review+1

Can we a note on 'usePigz' in the top-comment in src/img/lib/configuration.js please?

@twhiteman commented at 2017-07-20T21:59:12

Patch Set 5:

New commits:
commit cfc71e04c43bee236c7e069d064522502ae3fbb4
add comment for usePigz configuration

@trentm commented at 2017-07-20T22:17:15

Patch Set 5: Code-Review+1 Integration-Approval+1

@twhiteman commented at 2017-08-03T18:48:49

Patch Set 6: Patch Set 5 was rebased

twhiteman commented 2 years ago

Expired.