Use gzip in tidy instead of bzip2 for compatibility #45
Conversation
|
Adam made the script to start with and I think he used bzip due to increased compression over gzip. But hopefully he'll chime in. |
|
Thanks for pointing this out. I did not know there were any cases where bzip2 was not available. Nick is correct, I used bzip2 due to its improved compression. It looks as though only the CentOS 7 minimal install does not include it. I do not know how common it is for a customer to both use the minimal installation option and never install bzip2 on their own. So the question becomes, which is more important. A 5% improvement in compression ratios on all systems, or covering for the (potentially) rare case where bzip2 is not installed. I think, ideally, we would put a conditional in there checking for bzip2. Use it if present and fall back to gzip if not. With that method, we could even use xz and gain another 3% compression. |
|
We have a pretty standard CentOS installation, so I think the case where I have a hard time seeing the extra complexity being worth a 5 percentage point improvement in compression, even ignoring the CPU hit (generally 5× or more). Especially since the compressed output is less than 1 MB per day on our system. |
|
Also, as a sysadmin, I very rarely use My network connections are fast enough and my disks big enough that it's just noise. |
|
@abottchen any further thoughts? |
|
Even if a customer has 1 GB of data, we are only talking about a savings of 50MB with bzip2. There isn't enough gain to justify the risk regardless of the percentage of affected systems. I agree with Daniel, let's fall back to gzip. |
templates/tidy_cron.epp
Outdated
| xargs -a "$DIR.tmp" tar -jcf "$DIR/$METRICS_TYPE-$(date +%Y.%m.%d.%H.%M.%S).tar.bz2" 2>/dev/null; | ||
| xargs -a "$DIR.tmp" rm ; | ||
| rm "$DIR.tmp"; | ||
| find . -type f -not -name "*.gz" > "$DIR.tmp" |
There was a problem hiding this comment.
I haven't tested this, but I suspect this line will need something along the lines of -o -name "*.bz2". Otherwise customers that have this already in place with bzip2 will end up compressing their existing bz2 files into their new gzip files.
There was a problem hiding this comment.
Good point! I'm on it.
There was a problem hiding this comment.
Fixed. Turns out it has to be -a -not
|
@jarretlavallee @abottchen I'm about to merge this. Can someone update the metrics viewer to deal with unzipping the gzips instead of bzips? |
templates/tidy_cron.epp
Outdated
| xargs -a "$DIR.tmp" tar -jcf "$DIR/$METRICS_TYPE-$(date +%Y.%m.%d.%H.%M.%S).tar.bz2" 2>/dev/null; | ||
| xargs -a "$DIR.tmp" rm ; | ||
| rm "$DIR.tmp"; | ||
| find manifests -type f -not -name "*.gz" -a -not -name "*.bz2" > "$DIR.tmp" |
There was a problem hiding this comment.
how did this find get changed from . to manifests? @danielparks
In my testing it still needs to be .
There was a problem hiding this comment.
…
That's bizarre. There's nothing called manifests. Must have changed it accidentally somehow? I did test this…
|
@npwalker puppetlabs/puppet-metrics-viewer#19 should take care of it |
bzip2 isn't installed by default on CentOS 7. (!) This also removes the error suppresion on tar, which improves debuggability. Finally, "!" is a shell special character. This switches to "-not", though it's academic here since it's non interactive.
|
Fixed the weird |
|
No worries, that's why we test things :) |
bzip2 isn't installed by default on CentOS 7. (!)
This also removes the error suppresion on tar, which improves debuggability.
Also, "!" is a shell special character. This switches to "-not", though it probably doesn't matter since it's non interactive.