On Fri, Aug 16, 2019 at 3:54 PM Jeevan Chalke <jeevan.cha...@enterprisedb.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