fix issue 149 and 150 #151
Conversation
|
Dear huangfumingyue. Thank you for responding to this issue. Sorry, but could you write the two issues in separate PRs? Best regards. |
| @@ -1091,11 +1091,12 @@ confirm_block_size(const char *name, int blcksz) | |||
| else if (strcmp(name, "wal_block_size") == 0) | |||
| elog(DEBUG, "wal block size is %d", block_size); | |||
|
|
|||
| PQclear(res); | |||
| //PQclear(res); | |||
There was a problem hiding this comment.
I think to remove this line is ok.
| @@ -192,7 +192,7 @@ do_backup_database(parray *backup_list, pgBackupOption bkupopt) | |||
| uint32 xlogid, xrecoff; | |||
|
|
|||
| /* find last completed database backup */ | |||
| prev_backup = catalog_get_last_full_backup(backup_list); | |||
There was a problem hiding this comment.
For #150, I think it's enough to undo "backup.c", "catalog.c" and "pg_rman.h" of this change .
Your patches check if the full backup exists or not, but this can't solve the meaningless incremental backup issue anyway. If my understanding is right, #154 should be handled if to solve the issue. So, I think it's better that your patches focus to solve the differential backup problem only. #154 will solve the meaningless incremental backup issue.
But, adding a test case is necessary to prevent this degradation for the future. pg_rman supports full backup and incremental backup, not differential backup. So I think we need to check it. Although there may be a better way, I came up with the following test case.
- get full backup
- pgbench and store enough data. For example, execute "pg_bench -i -s 50".
- get incremental backup
- get incremental backup
If incremental backup works, 3's backup's data size is big because 2's data is stored, but 4's backup's data is small.
In my environment, the following is the result.
a. differential backup
This is the result when using "REL_13_STABLE" branch.
Because the differential backup is got, the third backup size is big(798MB). I think the reason why the third one(798MB) is smaller than the second one(1435MB) is WAL archive data is not stored in the third one.
> pg_rman show
=====================================================================
StartTime EndTime Mode Size TLI Status
=====================================================================
2021-05-13 09:20:50 2021-05-13 09:20:52 INCR 798MB 1 OK
2021-05-13 09:20:33 2021-05-13 09:20:38 INCR 1435MB 1 OK
2021-05-13 09:19:29 2021-05-13 09:19:31 FULL 49MB 1 OK
b. incremental backup
This is the result when using "REL_13_STABLE" branch and revert 447c19a.
Since this is an incremental backup, the third one is very small(33MB).
> pg_rman show
=====================================================================
StartTime EndTime Mode Size TLI Status
=====================================================================
2021-05-13 09:27:00 2021-05-13 09:27:03 INCR 33MB 1 OK
2021-05-13 09:26:41 2021-05-13 09:26:45 INCR 1435MB 1 OK
2021-05-13 09:26:04 2021-05-13 09:26:06 FULL 49MB 1 OK
Though?
| if ((endp && *endp) || block_size != blcksz) | ||
| ereport(ERROR, | ||
| (errcode(ERROR_PG_INCOMPATIBLE), | ||
| errmsg("%s(%d) is not compatible(%d expected)", name, block_size, blcksz))); | ||
| PQclear(res); |
There was a problem hiding this comment.
I think this patch needs to remove two branks before PQclear(res).
|
Dear all. Best regards. |
#149 #150