Re: block-level incremental backup

On Fri, Aug 16, 2019 at 3:54 PM Jeevan Chalke
<jeevan(dot)chalke(at)enterprisedb(dot)com> wrote:
>
0003:
+/*
+ * When to send the whole file, % blocks modified (90%)
+ */
+#define WHOLE_FILE_THRESHOLD 0.9

How this threshold is selected. Is it by some test?

- magic number, currently 0 (4 bytes)
I think in the patch we are using (#define INCREMENTAL_BACKUP_MAGIC
0x494E4352) as a magic number, not 0

+ Assert(statbuf->st_size <= (RELSEG_SIZE * BLCKSZ));
+
+ buf = (char *) malloc(statbuf->st_size);
+ if (buf == NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("out of memory")));
+
+ if ((cnt = fread(buf, 1, statbuf->st_size, fp)) > 0)
+ {
+ Bitmapset *mod_blocks = NULL;
+ int nmodblocks = 0;
+
+ if (cnt % BLCKSZ != 0)
+ {

It will be good to add some comments for the if block and also for the
assert. Actully, it's not very clear from the code.

0004:
+#include <time.h>
+#include <sys/stat.h>
+#include <unistd.h>
Header file include order (sys/state.h should be before time.h)

+ printf(_("%s combines full backup with incremental backup.\n\n"), progname);
/backup/backups

+ * scan_file
+ *
+ * Checks whether given file is partial file or not. If partial, then combines
+ * it into a full backup file, else copies as is to the output directory.
+ */

/If partial, then combines/ If partial, then combine

+static void
+combine_partial_files(const char *fn, char **IncrDirs, int nIncrDir,
+ const char *subdirpath, const char *outfn)
+ /*
+ * Open all files from all incremental backup directories and create a file
+ * map.
+ */
+ basefilefound = false;
+ for (i = (nIncrDir - 1), fmindex = 0; i >= 0; i--, fmindex++)
+ {
+ fm = &filemaps[fmindex];
+
.....
+ }
+
+
+ /* Process all opened files. */
+ lastblkno = 0;
+ modifiedblockfound = false;
+ for (i = 0; i < fmindex; i++)
+ {
+ char *buf;
+ int hsize;
+ int k;
+ int blkstartoffset;
......
+ }
+
+ for (i = 0; i <= lastblkno; i++)
+ {
+ char blkdata[BLCKSZ];
+ FILE *infp;
+ int offset;
...
+ }
}

Can we breakdown this function in 2-3 functions. At least creating a
file map can directly go to a separate function.

I have read 0003 and 0004 patch and there are few cosmetic comments.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com