Skip to content

Commit 000f3ad

Browse files
michaelpqmichaelpq
committed
Refactor tar method of walmethods.c to rely on the compression method
Since d62bcc8, the directory method of walmethods.c uses the compression method to determine which code path to take. The tar method, used by pg_basebackup --format=t, was inconsistent regarding that, as it relied on the compression level to check if no compression or gzip should be used. This commit makes the code more consistent as a whole in this file, making the tar logic use a compression method rather than assigning COMPRESSION_NONE that would be ignored. The options of pg_basebackup are planned to be reworked but we are not sure yet of the shape they should have as this has some dependency with the integration of the server-side compression for base backups, so this is left out for the moment. This change has as benefit to make easier the future integration of new compression methods for the tar method of walmethods.c, for the client-side compression. Reviewed-by: Georgios Kokolatos Discussion: https://postgr.es/m/Yb3GEgWwcu4wZDuA@paquier.xyz
1 parent 7ead992 commit 000f3ad

File tree

2 files changed

+38
-22
lines changed

2 files changed

+38
-22
lines changed

‎src/bin/pg_basebackup/pg_basebackup.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -524,7 +524,8 @@ LogStreamerMain(logstreamer_param *param)
524524
stream.do_sync);
525525
else
526526
stream.walmethod = CreateWalTarMethod(param->xlog,
527-
COMPRESSION_NONE, /* ignored */
527+
(compresslevel > 0) ?
528+
COMPRESSION_GZIP : COMPRESSION_NONE,
528529
compresslevel,
529530
stream.do_sync);
530531

‎src/bin/pg_basebackup/walmethods.c

Lines changed: 36 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -749,7 +749,7 @@ tar_write(Walfile f, const void *buf, size_t count)
749749
tar_clear_error();
750750

751751
/* Tarfile will always be positioned at the end */
752-
if (!tar_data->compression_level)
752+
if (tar_data->compression_method == COMPRESSION_NONE)
753753
{
754754
errno = 0;
755755
r = write(tar_data->fd, buf, count);
@@ -763,21 +763,20 @@ tar_write(Walfile f, const void *buf, size_t count)
763763
return r;
764764
}
765765
#ifdef HAVE_LIBZ
766-
else
766+
else if (tar_data->compression_method == COMPRESSION_GZIP)
767767
{
768768
if (!tar_write_compressed_data(unconstify(void *, buf), count, false))
769769
return -1;
770770
((TarMethodFile *) f)->currpos += count;
771771
return count;
772772
}
773-
#else
773+
#endif
774774
else
775775
{
776-
/* Can't happen - compression enabled with no libz */
776+
/* Can't happen - compression enabled with no method set */
777777
tar_data->lasterrno = ENOSYS;
778778
return -1;
779779
}
780-
#endif
781780
}
782781

783782
static bool
@@ -833,7 +832,7 @@ tar_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
833832
}
834833

835834
#ifdef HAVE_LIBZ
836-
if (tar_data->compression_level)
835+
if (tar_data->compression_method == COMPRESSION_GZIP)
837836
{
838837
tar_data->zp = (z_streamp) pg_malloc(sizeof(z_stream));
839838
tar_data->zp->zalloc = Z_NULL;
@@ -884,7 +883,7 @@ tar_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
884883
pg_free(tmppath);
885884

886885
#ifdef HAVE_LIBZ
887-
if (tar_data->compression_level)
886+
if (tar_data->compression_method == COMPRESSION_GZIP)
888887
{
889888
/* Flush existing data */
890889
if (!tar_write_compressed_data(NULL, 0, true))
@@ -909,7 +908,7 @@ tar_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
909908
}
910909
tar_data->currentfile->currpos = 0;
911910

912-
if (!tar_data->compression_level)
911+
if (tar_data->compression_method == COMPRESSION_NONE)
913912
{
914913
errno = 0;
915914
if (write(tar_data->fd, tar_data->currentfile->header,
@@ -923,7 +922,7 @@ tar_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
923922
}
924923
}
925924
#ifdef HAVE_LIBZ
926-
else
925+
else if (tar_data->compression_method == COMPRESSION_GZIP)
927926
{
928927
/* Write header through the zlib APIs but with no compression */
929928
if (!tar_write_compressed_data(tar_data->currentfile->header,
@@ -938,6 +937,11 @@ tar_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
938937
}
939938
}
940939
#endif
940+
else
941+
{
942+
/* not reachable */
943+
Assert(false);
944+
}
941945

942946
tar_data->currentfile->pathname = pg_strdup(pathname);
943947

@@ -948,7 +952,7 @@ tar_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
948952
if (pad_to_size)
949953
{
950954
tar_data->currentfile->pad_to_size = pad_to_size;
951-
if (!tar_data->compression_level)
955+
if (tar_data->compression_method == COMPRESSION_NONE)
952956
{
953957
/* Uncompressed, so pad now */
954958
if (!tar_write_padding_data(tar_data->currentfile, pad_to_size))
@@ -1009,7 +1013,7 @@ tar_sync(Walfile f)
10091013
* Always sync the whole tarfile, because that's all we can do. This makes
10101014
* no sense on compressed files, so just ignore those.
10111015
*/
1012-
if (tar_data->compression_level)
1016+
if (tar_data->compression_method != COMPRESSION_NONE)
10131017
return 0;
10141018

10151019
r = fsync(tar_data->fd);
@@ -1030,7 +1034,7 @@ tar_close(Walfile f, WalCloseMethod method)
10301034

10311035
if (method == CLOSE_UNLINK)
10321036
{
1033-
if (tar_data->compression_level)
1037+
if (tar_data->compression_method != COMPRESSION_NONE)
10341038
{
10351039
tar_set_error("unlink not supported with compression");
10361040
return -1;
@@ -1061,7 +1065,7 @@ tar_close(Walfile f, WalCloseMethod method)
10611065
*/
10621066
if (tf->pad_to_size)
10631067
{
1064-
if (tar_data->compression_level)
1068+
if (tar_data->compression_method == COMPRESSION_GZIP)
10651069
{
10661070
/*
10671071
* A compressed tarfile is padded on close since we cannot know
@@ -1102,7 +1106,7 @@ tar_close(Walfile f, WalCloseMethod method)
11021106

11031107

11041108
#ifdef HAVE_LIBZ
1105-
if (tar_data->compression_level)
1109+
if (tar_data->compression_method == COMPRESSION_GZIP)
11061110
{
11071111
/* Flush the current buffer */
11081112
if (!tar_write_compressed_data(NULL, 0, true))
@@ -1131,7 +1135,7 @@ tar_close(Walfile f, WalCloseMethod method)
11311135
tar_data->lasterrno = errno;
11321136
return -1;
11331137
}
1134-
if (!tar_data->compression_level)
1138+
if (tar_data->compression_method == COMPRESSION_NONE)
11351139
{
11361140
errno = 0;
11371141
if (write(tar_data->fd, tf->header, TAR_BLOCK_SIZE) != TAR_BLOCK_SIZE)
@@ -1142,7 +1146,7 @@ tar_close(Walfile f, WalCloseMethod method)
11421146
}
11431147
}
11441148
#ifdef HAVE_LIBZ
1145-
else
1149+
else if (tar_data->compression_method == COMPRESSION_GZIP)
11461150
{
11471151
/* Turn off compression */
11481152
if (deflateParams(tar_data->zp, 0, 0) != Z_OK)
@@ -1164,6 +1168,11 @@ tar_close(Walfile f, WalCloseMethod method)
11641168
}
11651169
}
11661170
#endif
1171+
else
1172+
{
1173+
/* not reachable */
1174+
Assert(false);
1175+
}
11671176

11681177
/* Move file pointer back down to end, so we can write the next file */
11691178
if (lseek(tar_data->fd, 0, SEEK_END) < 0)
@@ -1212,7 +1221,7 @@ tar_finish(void)
12121221

12131222
/* A tarfile always ends with two empty blocks */
12141223
MemSet(zerobuf, 0, sizeof(zerobuf));
1215-
if (!tar_data->compression_level)
1224+
if (tar_data->compression_method == COMPRESSION_NONE)
12161225
{
12171226
errno = 0;
12181227
if (write(tar_data->fd, zerobuf, sizeof(zerobuf)) != sizeof(zerobuf))
@@ -1223,7 +1232,7 @@ tar_finish(void)
12231232
}
12241233
}
12251234
#ifdef HAVE_LIBZ
1226-
else
1235+
else if (tar_data->compression_method == COMPRESSION_GZIP)
12271236
{
12281237
if (!tar_write_compressed_data(zerobuf, sizeof(zerobuf), false))
12291238
return false;
@@ -1268,6 +1277,11 @@ tar_finish(void)
12681277
}
12691278
}
12701279
#endif
1280+
else
1281+
{
1282+
/* not reachable */
1283+
Assert(false);
1284+
}
12711285

12721286
/* sync the empty blocks as well, since they're after the last file */
12731287
if (tar_data->sync)
@@ -1312,7 +1326,8 @@ CreateWalTarMethod(const char *tarbase,
13121326
int compression_level, bool sync)
13131327
{
13141328
WalWriteMethod *method;
1315-
const char *suffix = (compression_level != 0) ? ".tar.gz" : ".tar";
1329+
const char *suffix = (compression_method == COMPRESSION_GZIP) ?
1330+
".tar.gz" : ".tar";
13161331

13171332
method = pg_malloc0(sizeof(WalWriteMethod));
13181333
method->open_for_write = tar_open_for_write;
@@ -1335,7 +1350,7 @@ CreateWalTarMethod(const char *tarbase,
13351350
tar_data->compression_level = compression_level;
13361351
tar_data->sync = sync;
13371352
#ifdef HAVE_LIBZ
1338-
if (compression_level)
1353+
if (compression_method == COMPRESSION_GZIP)
13391354
tar_data->zlibOut = (char *) pg_malloc(ZLIB_OUT_SIZE + 1);
13401355
#endif
13411356

@@ -1347,7 +1362,7 @@ FreeWalTarMethod(void)
13471362
{
13481363
pg_free(tar_data->tarfilename);
13491364
#ifdef HAVE_LIBZ
1350-
if (tar_data->compression_level)
1365+
if (tar_data->compression_method == COMPRESSION_GZIP)
13511366
pg_free(tar_data->zlibOut);
13521367
#endif
13531368
pg_free(tar_data);

0 commit comments

Comments
 (0)