Conversation
Added support for parallel compression and decompression using pigz if available.
|
There's an issue with decompressor quotes in remoteCommands (as it's already a quoted string). |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for using pigz (parallel gzip) for faster compression and decompression during Coolify instance migration. When pigz is available on the system, the script will automatically use it for parallel processing; otherwise, it falls back to standard gzip.
- Detects
pigzavailability and uses it with parallel processing (nprocthreads) for compression - Detects
pigzavailability on the destination server for parallel decompression - Updates tar commands to use the
-Iflag with custom compressor/decompressor commands
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
migrate.sh
Outdated
| fi | ||
|
|
||
| # run extraction | ||
| if ! $decompressor | tar -Px -C /; then |
There was a problem hiding this comment.
The $decompressor variable is unquoted, which could lead to word splitting and glob expansion issues. Although the variable is set internally in the script, it should be quoted for safety and consistency with the compression command at line 97. Change to if ! "$decompressor" | tar -Px -C /; then.
| if ! $decompressor | tar -Px -C /; then | |
| if ! "$decompressor" | tar -Px -C /; then |
| # Choose compressor | ||
| if command -v pigz >/dev/null 2>&1; then | ||
| echo "✅ Using pigz for parallel gzip" | ||
| compressor="pigz -p$(nproc)" |
There was a problem hiding this comment.
[nitpick] The -p$(nproc) flag uses all available CPU cores, which could overwhelm the system during migration. Consider limiting the number of threads (e.g., pigz -p$(($(nproc) - 1))) to leave at least one core available for other processes, or make it configurable.
migrate.sh
Outdated
| echo 'ℹ️ pigz not found, using gzip'; | ||
| decompressor="gzip -dc" | ||
| fi | ||
|
|
There was a problem hiding this comment.
This line contains trailing whitespace. Remove the trailing spaces for consistency with the rest of the script.
|
Ok, I think this one should work now, parallel compression and decompression, for 100GB of volumes this results in like 5mins vs 50mins bacup time, a 10x speed-up |
|
did you test it? you said you think lol |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Choose compressor | ||
| if command -v pigz >/dev/null 2>&1; then | ||
| echo "✅ Using pigz for parallel gzip" | ||
| compressor="pigz -p$(nproc)" |
There was a problem hiding this comment.
The -p$(nproc) option may fail on systems where nproc is not available (e.g., some BSD systems or minimal environments). Consider adding a fallback:
compressor="pigz -p${NPROC:-$(nproc 2>/dev/null || echo 4)}"Or simply use pigz without the -p flag, as it will automatically use all available cores by default.
| compressor="pigz -p$(nproc)" | |
| compressor="pigz -p${NPROC:-$(nproc 2>/dev/null || echo 4)}" |
| echo '❌ Backup file extraction failed' | ||
| exit 1 |
There was a problem hiding this comment.
Missing semicolons at the end of statements. The script's existing style (visible in the surrounding remote commands) consistently uses semicolons. Add them for consistency:
else
if ! tar -Pzxf - -C /; then
echo '❌ Backup file extraction failed';
exit 1;
fi
fi| echo '❌ Backup file extraction failed' | |
| exit 1 | |
| echo '❌ Backup file extraction failed'; | |
| exit 1; |
| exit 1; | ||
| echo '🚸 Extracting backup file...' | ||
| if command -v pigz >/dev/null 2>&1; then | ||
| echo '✅ Using pigz for parallel decompression' |
There was a problem hiding this comment.
Missing semicolon at the end of the statement on line 173. The script's existing style consistently uses semicolons. Add one for consistency:
echo '✅ Using pigz for parallel decompression';| echo '✅ Using pigz for parallel decompression' | |
| echo '✅ Using pigz for parallel decompression'; |
| echo '❌ Backup file extraction failed' | ||
| exit 1 | ||
| fi | ||
| else | ||
| if ! tar -Pzxf - -C /; then | ||
| echo '❌ Backup file extraction failed' | ||
| exit 1 |
There was a problem hiding this comment.
Missing semicolons at the end of statements on lines 175, 176, and 181. While bash doesn't strictly require semicolons before fi, the script's existing style (visible in the surrounding remote commands) consistently uses semicolons. This inconsistency could lead to maintainability issues.
Add semicolons for consistency:
if ! tar -I pigz -Pxf - -C /; then
echo '❌ Backup file extraction failed';
exit 1;
fi| echo '❌ Backup file extraction failed' | |
| exit 1 | |
| fi | |
| else | |
| if ! tar -Pzxf - -C /; then | |
| echo '❌ Backup file extraction failed' | |
| exit 1 | |
| echo '❌ Backup file extraction failed'; | |
| exit 1; | |
| fi | |
| else | |
| if ! tar -Pzxf - -C /; then | |
| echo '❌ Backup file extraction failed'; | |
| exit 1; |
| fi | ||
| fi | ||
| echo '✅ Backup file extracted'; | ||
| echo '✅ Backup file extracted' |
There was a problem hiding this comment.
Missing semicolon at the end of the statement. The script's existing style consistently uses semicolons. Add one for consistency:
echo '✅ Backup file extracted';| echo '✅ Backup file extracted' | |
| echo '✅ Backup file extracted'; |
| if ! tar -Pxzf - -C /; then | ||
| echo '❌ Backup file extraction failed'; | ||
| exit 1; | ||
| echo '🚸 Extracting backup file...' |
There was a problem hiding this comment.
Missing semicolon at the end of the statement on line 171. The script's existing style consistently uses semicolons. Add one for consistency:
echo '🚸 Extracting backup file...';| echo '🚸 Extracting backup file...' | |
| echo '🚸 Extracting backup file...'; |
I tested it, and successfully migrated a few days ago (there were still some issues with some services, where some ENV variables were wrong, or some other small configuration issues, but those are not related to this change). I said I think this one should work, because I made a few extra small changes after, to fix some warnings. Maybe the code can be improved, but currently it does seem to workk: use pigz and all threads (maybe the AI suggesiton is good to use threads N-1 instead). |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if command -v pigz >/dev/null 2>&1; then | ||
| echo '✅ Using pigz for parallel decompression' | ||
| if ! tar -I pigz -Pxf - -C /; then | ||
| echo '❌ Backup file extraction failed' | ||
| exit 1 | ||
| fi | ||
| else | ||
| if ! tar -Pzxf - -C /; then | ||
| echo '❌ Backup file extraction failed' | ||
| exit 1 | ||
| fi | ||
| fi |
There was a problem hiding this comment.
The extraction logic duplicates error handling code across both branches (pigz and gzip). The conditional could be simplified by determining the decompressor flags first, then calling tar once. This would reduce code duplication and make the logic easier to maintain.
| if command -v pigz >/dev/null 2>&1; then | |
| echo '✅ Using pigz for parallel decompression' | |
| if ! tar -I pigz -Pxf - -C /; then | |
| echo '❌ Backup file extraction failed' | |
| exit 1 | |
| fi | |
| else | |
| if ! tar -Pzxf - -C /; then | |
| echo '❌ Backup file extraction failed' | |
| exit 1 | |
| fi | |
| fi | |
| tar_opts="-Pzxf" | |
| if command -v pigz >/dev/null 2>&1; then | |
| echo '✅ Using pigz for parallel decompression' | |
| tar_opts="-I pigz -Pxf" | |
| fi | |
| if ! tar \$tar_opts - -C /; then | |
| echo '❌ Backup file extraction failed' | |
| exit 1 | |
| fi |
| echo '🚸 Extracting backup file...' | ||
| if command -v pigz >/dev/null 2>&1; then | ||
| echo '✅ Using pigz for parallel decompression' | ||
| if ! tar -I pigz -Pxf - -C /; then |
There was a problem hiding this comment.
The pigz compression command includes the number of processors with -p$(nproc), but during decompression, pigz is used without the -p flag. While pigz will auto-detect the number of cores by default, it would be more consistent and explicit to also specify the thread count during decompression, especially since it was explicitly set during compression.
| if ! tar -I pigz -Pxf - -C /; then | |
| if ! tar -I "pigz -p\$(nproc)" -Pxf - -C /; then |
| if ! tar --exclude='*.sock' -Pczf $backupFileName -C / $backupSourceDir $HOME/.ssh/authorized_keys $volumePaths; then | ||
| tar --exclude='*.sock' --warning=no-file-changed -I "$compressor" -Pcf "$backupFileName" \ | ||
| -C / $backupSourceDir $HOME/.ssh/authorized_keys $volumePaths | ||
| rc=$? |
There was a problem hiding this comment.
The tar exit code handling allows exit code 1 to pass (which indicates files changed during archival). While this is likely intentional given the --warning=no-file-changed flag, the condition should be "if [ $rc -ne 0 ] && [ $rc -ne 1 ]; then" or "if [ $rc -gt 1 ]; then" to be explicit about which codes are acceptable. The current implementation correctly uses -gt 1, but a comment explaining that exit code 1 is acceptable (files changed during read) would improve clarity.
| rc=$? | |
| rc=$? | |
| # tar exit code 0 = success, 1 = files changed during read (acceptable with --warning=no-file-changed) |
Added support for parallel compression and decompression using pigz if available.
Note this is not fully tested.
Related: #8